From 87b2cef8752590699812a2b3b898b68ba55033c0 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 19 Dec 2024 12:00:21 +0000 Subject: [PATCH 1/3] chore: move implementation of print foreign call into `nargo` --- Cargo.lock | 6 +- compiler/noirc_printable_type/Cargo.toml | 5 +- compiler/noirc_printable_type/src/lib.rs | 286 +++------------------- tooling/debugger/src/foreign_calls.rs | 3 +- tooling/nargo/Cargo.toml | 1 + tooling/nargo/src/errors.rs | 3 +- tooling/nargo/src/foreign_calls/mocker.rs | 4 +- tooling/nargo/src/foreign_calls/mod.rs | 20 +- tooling/nargo/src/foreign_calls/print.rs | 84 ++++++- tooling/nargo/src/foreign_calls/rpc.rs | 3 +- tooling/nargo/src/ops/test.rs | 3 +- tooling/noirc_abi/src/lib.rs | 16 +- tooling/noirc_abi/src/printable_type.rs | 78 ++++++ tooling/noirc_artifacts/src/debug_vars.rs | 7 +- 14 files changed, 240 insertions(+), 279 deletions(-) create mode 100644 tooling/noirc_abi/src/printable_type.rs diff --git a/Cargo.lock b/Cargo.lock index 4907de7ae62..324e5f3350d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2793,6 +2793,7 @@ dependencies = [ "rand 0.8.5", "rayon", "serde", + "serde_json", "thiserror", "tracing", "walkdir", @@ -3226,11 +3227,8 @@ name = "noirc_printable_type" version = "1.0.0-beta.0" dependencies = [ "acvm", - "iter-extended", - "jsonrpc", + "regex", "serde", - "serde_json", - "thiserror", ] [[package]] diff --git a/compiler/noirc_printable_type/Cargo.toml b/compiler/noirc_printable_type/Cargo.toml index 8d0574aad64..73cf104ae44 100644 --- a/compiler/noirc_printable_type/Cargo.toml +++ b/compiler/noirc_printable_type/Cargo.toml @@ -13,10 +13,7 @@ workspace = true [dependencies] acvm.workspace = true -iter-extended.workspace = true serde.workspace = true -serde_json.workspace = true -thiserror.workspace = true -jsonrpc.workspace = true +regex = "1.9.1" [dev-dependencies] diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 6ae187da27f..6c3dbfe6c65 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -1,10 +1,14 @@ +#![forbid(unsafe_code)] +#![warn(unused_crate_dependencies, unused_extern_crates)] +#![warn(unreachable_pub)] +#![warn(clippy::semicolon_if_nothing_returned)] + use std::{collections::BTreeMap, str}; -use acvm::{acir::AcirField, brillig_vm::brillig::ForeignCallParam}; -use iter_extended::vecmap; +use acvm::AcirField; +use regex::{Captures, Regex}; use serde::{Deserialize, Serialize}; -use thiserror::Error; #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "lowercase")] @@ -67,95 +71,28 @@ pub enum PrintableValueDisplay { FmtString(String, Vec<(PrintableValue, PrintableType)>), } -#[derive(Debug, Error)] -pub enum ForeignCallError { - #[error("No handler could be found for foreign call `{0}`")] - NoHandler(String), - - #[error("Foreign call inputs needed for execution are missing")] - MissingForeignCallInputs, - - #[error("Could not parse PrintableType argument. {0}")] - ParsingError(#[from] serde_json::Error), - - #[error("Failed calling external resolver. {0}")] - ExternalResolverError(#[from] jsonrpc::Error), - - #[error("Assert message resolved after an unsatisified constrain. {0}")] - ResolvedAssertMessage(String), -} - -impl TryFrom<&[ForeignCallParam]> for PrintableValueDisplay { - type Error = ForeignCallError; +impl std::fmt::Display for PrintableValueDisplay { + fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Plain(value, typ) => { + let output_string = to_string(value, typ).ok_or(std::fmt::Error)?; + write!(fmt, "{output_string}") + } + Self::FmtString(template, values) => { + let mut display_iter = values.iter(); + let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}").map_err(|_| std::fmt::Error)?; - fn try_from(foreign_call_inputs: &[ForeignCallParam]) -> Result { - let (is_fmt_str, foreign_call_inputs) = - foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; + let formatted_str = replace_all(&re, template, |_: &Captures| { + let (value, typ) = display_iter.next().ok_or(std::fmt::Error)?; + to_string(value, typ).ok_or(std::fmt::Error) + })?; - if is_fmt_str.unwrap_field().is_one() { - convert_fmt_string_inputs(foreign_call_inputs) - } else { - convert_string_inputs(foreign_call_inputs) + write!(fmt, "{formatted_str}") + } } } } -fn convert_string_inputs( - foreign_call_inputs: &[ForeignCallParam], -) -> Result, ForeignCallError> { - // Fetch the PrintableType from the foreign call input - // The remaining input values should hold what is to be printed - let (printable_type_as_values, input_values) = - foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; - let printable_type = fetch_printable_type(printable_type_as_values)?; - - // We must use a flat map here as each value in a struct will be in a separate input value - let mut input_values_as_fields = input_values.iter().flat_map(|param| param.fields()); - - let value = decode_value(&mut input_values_as_fields, &printable_type); - - Ok(PrintableValueDisplay::Plain(value, printable_type)) -} - -fn convert_fmt_string_inputs( - foreign_call_inputs: &[ForeignCallParam], -) -> Result, ForeignCallError> { - let (message, input_and_printable_types) = - foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; - - let message_as_fields = message.fields(); - let message_as_string = decode_string_value(&message_as_fields); - - let (num_values, input_and_printable_types) = input_and_printable_types - .split_first() - .ok_or(ForeignCallError::MissingForeignCallInputs)?; - - let mut output = Vec::new(); - let num_values = num_values.unwrap_field().to_u128() as usize; - - let types_start_at = input_and_printable_types.len() - num_values; - let mut input_iter = - input_and_printable_types[0..types_start_at].iter().flat_map(|param| param.fields()); - for printable_type in input_and_printable_types.iter().skip(types_start_at) { - let printable_type = fetch_printable_type(printable_type)?; - let value = decode_value(&mut input_iter, &printable_type); - - output.push((value, printable_type)); - } - - Ok(PrintableValueDisplay::FmtString(message_as_string, output)) -} - -fn fetch_printable_type( - printable_type: &ForeignCallParam, -) -> Result { - let printable_type_as_fields = printable_type.fields(); - let printable_type_as_string = decode_string_value(&printable_type_as_fields); - let printable_type: PrintableType = serde_json::from_str(&printable_type_as_string)?; - - Ok(printable_type) -} - fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { let mut output = String::new(); match (value, typ) { @@ -193,7 +130,7 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Array { typ, .. }) | (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Slice { typ }) => { if *is_slice { - output.push('&') + output.push('&'); } output.push('['); let mut values = array_elements.iter().peekable(); @@ -253,82 +190,22 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op Some(output) } -impl std::fmt::Display for PrintableValueDisplay { - fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Plain(value, typ) => { - let output_string = to_string(value, typ).ok_or(std::fmt::Error)?; - write!(fmt, "{output_string}") - } - Self::FmtString(template, values) => { - let mut values_iter = values.iter(); - write_template_replacing_interpolations(template, fmt, || { - values_iter.next().and_then(|(value, typ)| to_string(value, typ)) - }) - } - } +// Taken from Regex docs directly +fn replace_all( + re: &Regex, + haystack: &str, + mut replacement: impl FnMut(&Captures) -> Result, +) -> Result { + let mut new = String::with_capacity(haystack.len()); + let mut last_match = 0; + for caps in re.captures_iter(haystack) { + let m = caps.get(0).unwrap(); + new.push_str(&haystack[last_match..m.start()]); + new.push_str(&replacement(&caps)?); + last_match = m.end(); } -} - -fn write_template_replacing_interpolations( - template: &str, - fmt: &mut std::fmt::Formatter<'_>, - mut replacement: impl FnMut() -> Option, -) -> std::fmt::Result { - let mut last_index = 0; // How far we've written from the template - let mut char_indices = template.char_indices().peekable(); - while let Some((char_index, char)) = char_indices.next() { - // If we see a '}' it must be "}}" because the ones for interpolation are handled - // when we see '{' - if char == '}' { - // Write what we've seen so far in the template, including this '}' - write!(fmt, "{}", &template[last_index..=char_index])?; - - // Skip the second '}' - let (_, closing_curly) = char_indices.next().unwrap(); - assert_eq!(closing_curly, '}'); - - last_index = char_indices.peek().map(|(index, _)| *index).unwrap_or(template.len()); - continue; - } - - // Keep going forward until we find a '{' - if char != '{' { - continue; - } - - // We'll either have to write an interpolation or '{{' if it's an escape, - // so let's write what we've seen so far in the template. - write!(fmt, "{}", &template[last_index..char_index])?; - - // If it's '{{', write '{' and keep going - if char_indices.peek().map(|(_, char)| char) == Some(&'{') { - write!(fmt, "{{")?; - - // Skip the second '{' - char_indices.next().unwrap(); - - last_index = char_indices.peek().map(|(index, _)| *index).unwrap_or(template.len()); - continue; - } - - // Write the interpolation - if let Some(string) = replacement() { - write!(fmt, "{}", string)?; - } else { - return Err(std::fmt::Error); - } - - // Whatever was inside '{...}' doesn't matter, so skip until we find '}' - while let Some((_, char)) = char_indices.next() { - if char == '}' { - last_index = char_indices.peek().map(|(index, _)| *index).unwrap_or(template.len()); - break; - } - } - } - - write!(fmt, "{}", &template[last_index..]) + new.push_str(&haystack[last_match..]); + Ok(new) } /// This trims any leading zeroes. @@ -346,94 +223,11 @@ fn format_field_string(field: F) -> String { "0x".to_owned() + &trimmed_field } -/// Assumes that `field_iterator` contains enough field elements in order to decode the [PrintableType] -pub fn decode_value( - field_iterator: &mut impl Iterator, - typ: &PrintableType, -) -> PrintableValue { - match typ { - PrintableType::Field - | PrintableType::SignedInteger { .. } - | PrintableType::UnsignedInteger { .. } - | PrintableType::Boolean => { - let field_element = field_iterator.next().unwrap(); - - PrintableValue::Field(field_element) - } - PrintableType::Array { length, typ } => { - let length = *length as usize; - let mut array_elements = Vec::with_capacity(length); - for _ in 0..length { - array_elements.push(decode_value(field_iterator, typ)); - } - - PrintableValue::Vec { array_elements, is_slice: false } - } - PrintableType::Slice { typ } => { - let length = field_iterator - .next() - .expect("not enough data to decode variable array length") - .to_u128() as usize; - let mut array_elements = Vec::with_capacity(length); - for _ in 0..length { - array_elements.push(decode_value(field_iterator, typ)); - } - - PrintableValue::Vec { array_elements, is_slice: true } - } - PrintableType::Tuple { types } => PrintableValue::Vec { - array_elements: vecmap(types, |typ| decode_value(field_iterator, typ)), - is_slice: false, - }, - PrintableType::String { length } => { - let field_elements: Vec = field_iterator.take(*length as usize).collect(); - - PrintableValue::String(decode_string_value(&field_elements)) - } - PrintableType::Struct { fields, .. } => { - let mut struct_map = BTreeMap::new(); - - for (field_key, param_type) in fields { - let field_value = decode_value(field_iterator, param_type); - - struct_map.insert(field_key.to_owned(), field_value); - } - - PrintableValue::Struct(struct_map) - } - PrintableType::Function { env, .. } => { - let field_element = field_iterator.next().unwrap(); - let func_ref = PrintableValue::Field(field_element); - // we want to consume the fields from the environment, but for now they are not actually printed - decode_value(field_iterator, env); - func_ref - } - PrintableType::MutableReference { typ } => { - // we decode the reference, but it's not really used for printing - decode_value(field_iterator, typ) - } - PrintableType::Unit => PrintableValue::Field(F::zero()), - } -} - -pub fn decode_string_value(field_elements: &[F]) -> String { - // TODO: Replace with `into` when Char is supported - let string_as_slice = vecmap(field_elements, |e| { - let mut field_as_bytes = e.to_be_bytes(); - let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element - assert!(field_as_bytes.into_iter().all(|b| b == 0)); // Assert that the rest of the field element's bytes are empty - char_byte - }); - - let final_string = str::from_utf8(&string_as_slice).unwrap(); - final_string.to_owned() -} - #[cfg(test)] mod tests { use acvm::FieldElement; - use crate::{PrintableType, PrintableValue, PrintableValueDisplay}; + use super::{PrintableType, PrintableValue, PrintableValueDisplay}; #[test] fn printable_value_display_to_string_without_interpolations() { diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index 899ba892d8f..49c7134c657 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -4,12 +4,11 @@ use acvm::{ AcirField, FieldElement, }; use nargo::{ - foreign_calls::{DefaultForeignCallExecutor, ForeignCallExecutor}, + foreign_calls::{DefaultForeignCallExecutor, ForeignCallError, ForeignCallExecutor}, PrintOutput, }; use noirc_artifacts::debug::{DebugArtifact, DebugVars, StackFrame}; use noirc_errors::debug_info::{DebugFnId, DebugVarId}; -use noirc_printable_type::ForeignCallError; pub(crate) enum DebugForeignCall { VarAssign, diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index 1dbb9978b0b..a8e0d3ceea6 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -27,6 +27,7 @@ rayon.workspace = true jsonrpc.workspace = true rand.workspace = true serde.workspace = true +serde_json.workspace = true walkdir = "2.5.0" [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/tooling/nargo/src/errors.rs b/tooling/nargo/src/errors.rs index 5256f28e36c..00c411bf7e4 100644 --- a/tooling/nargo/src/errors.rs +++ b/tooling/nargo/src/errors.rs @@ -16,9 +16,10 @@ use noirc_errors::{ pub use noirc_errors::Location; use noirc_driver::CrateName; -use noirc_printable_type::ForeignCallError; use thiserror::Error; +use crate::foreign_calls::ForeignCallError; + /// Errors covering situations where a package cannot be compiled. #[derive(Debug, Error)] pub enum CompileError { diff --git a/tooling/nargo/src/foreign_calls/mocker.rs b/tooling/nargo/src/foreign_calls/mocker.rs index c93d16bbaf6..c2035193c64 100644 --- a/tooling/nargo/src/foreign_calls/mocker.rs +++ b/tooling/nargo/src/foreign_calls/mocker.rs @@ -3,10 +3,10 @@ use acvm::{ pwg::ForeignCallWaitInfo, AcirField, }; -use noirc_printable_type::{decode_string_value, ForeignCallError}; +use noirc_abi::decode_string_value; use serde::{Deserialize, Serialize}; -use super::{ForeignCall, ForeignCallExecutor}; +use super::{ForeignCall, ForeignCallError, ForeignCallExecutor}; /// This struct represents an oracle mock. It can be used for testing programs that use oracles. #[derive(Debug, PartialEq, Eq, Clone)] diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs index 65ff051bcbf..4c2ff03cfac 100644 --- a/tooling/nargo/src/foreign_calls/mod.rs +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -2,11 +2,11 @@ use std::path::PathBuf; use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; use mocker::MockForeignCallExecutor; -use noirc_printable_type::ForeignCallError; use print::{PrintForeignCallExecutor, PrintOutput}; use rand::Rng; use rpc::RPCForeignCallExecutor; use serde::{Deserialize, Serialize}; +use thiserror::Error; pub(crate) mod mocker; pub(crate) mod print; @@ -64,6 +64,24 @@ impl ForeignCall { } } +#[derive(Debug, Error)] +pub enum ForeignCallError { + #[error("No handler could be found for foreign call `{0}`")] + NoHandler(String), + + #[error("Foreign call inputs needed for execution are missing")] + MissingForeignCallInputs, + + #[error("Could not parse PrintableType argument. {0}")] + ParsingError(#[from] serde_json::Error), + + #[error("Failed calling external resolver. {0}")] + ExternalResolverError(#[from] jsonrpc::Error), + + #[error("Assert message resolved after an unsatisified constrain. {0}")] + ResolvedAssertMessage(String), +} + #[derive(Debug, Default)] pub struct DefaultForeignCallExecutor<'a, F> { /// The executor for any [`ForeignCall::Print`] calls. diff --git a/tooling/nargo/src/foreign_calls/print.rs b/tooling/nargo/src/foreign_calls/print.rs index 8b2b5efd8b6..4785245200b 100644 --- a/tooling/nargo/src/foreign_calls/print.rs +++ b/tooling/nargo/src/foreign_calls/print.rs @@ -1,7 +1,13 @@ -use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; -use noirc_printable_type::{ForeignCallError, PrintableValueDisplay}; +use acvm::{ + acir::brillig::{ForeignCallParam, ForeignCallResult}, + pwg::ForeignCallWaitInfo, + AcirField, +}; +use jsonrpc::serde_json; +use noirc_abi::{decode_printable_value, decode_string_value}; +use noirc_printable_type::{PrintableType, PrintableValueDisplay}; -use super::{ForeignCall, ForeignCallExecutor}; +use super::{ForeignCall, ForeignCallError, ForeignCallExecutor}; #[derive(Debug, Default)] pub enum PrintOutput<'a> { @@ -32,7 +38,8 @@ impl ForeignCallExecutor for PrintForeignCallExecutor<'_> { .ok_or(ForeignCallError::MissingForeignCallInputs)? .1; - let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; + let display_values: PrintableValueDisplay = + try_from_params(foreign_call_inputs)?; let display_string = format!("{display_values}{}", if skip_newline { "" } else { "\n" }); @@ -50,3 +57,72 @@ impl ForeignCallExecutor for PrintForeignCallExecutor<'_> { } } } + +fn try_from_params( + foreign_call_inputs: &[ForeignCallParam], +) -> Result, ForeignCallError> { + let (is_fmt_str, foreign_call_inputs) = + foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; + + if is_fmt_str.unwrap_field().is_one() { + convert_fmt_string_inputs(foreign_call_inputs) + } else { + convert_string_inputs(foreign_call_inputs) + } +} + +fn convert_string_inputs( + foreign_call_inputs: &[ForeignCallParam], +) -> Result, ForeignCallError> { + // Fetch the PrintableType from the foreign call input + // The remaining input values should hold what is to be printed + let (printable_type_as_values, input_values) = + foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; + let printable_type = fetch_printable_type(printable_type_as_values)?; + + // We must use a flat map here as each value in a struct will be in a separate input value + let mut input_values_as_fields = input_values.iter().flat_map(|param| param.fields()); + + let value = decode_printable_value(&mut input_values_as_fields, &printable_type); + + Ok(PrintableValueDisplay::Plain(value, printable_type)) +} + +fn convert_fmt_string_inputs( + foreign_call_inputs: &[ForeignCallParam], +) -> Result, ForeignCallError> { + let (message, input_and_printable_types) = + foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; + + let message_as_fields = message.fields(); + let message_as_string = decode_string_value(&message_as_fields); + + let (num_values, input_and_printable_types) = input_and_printable_types + .split_first() + .ok_or(ForeignCallError::MissingForeignCallInputs)?; + + let mut output = Vec::new(); + let num_values = num_values.unwrap_field().to_u128() as usize; + + let types_start_at = input_and_printable_types.len() - num_values; + let mut input_iter = + input_and_printable_types[0..types_start_at].iter().flat_map(|param| param.fields()); + for printable_type in input_and_printable_types.iter().skip(types_start_at) { + let printable_type = fetch_printable_type(printable_type)?; + let value = decode_printable_value(&mut input_iter, &printable_type); + + output.push((value, printable_type)); + } + + Ok(PrintableValueDisplay::FmtString(message_as_string, output)) +} + +fn fetch_printable_type( + printable_type: &ForeignCallParam, +) -> Result { + let printable_type_as_fields = printable_type.fields(); + let printable_type_as_string = decode_string_value(&printable_type_as_fields); + let printable_type: PrintableType = serde_json::from_str(&printable_type_as_string)?; + + Ok(printable_type) +} diff --git a/tooling/nargo/src/foreign_calls/rpc.rs b/tooling/nargo/src/foreign_calls/rpc.rs index 0653eb1c7e3..4854754837a 100644 --- a/tooling/nargo/src/foreign_calls/rpc.rs +++ b/tooling/nargo/src/foreign_calls/rpc.rs @@ -2,10 +2,9 @@ use std::path::PathBuf; use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; use jsonrpc::{arg as build_json_rpc_arg, minreq_http::Builder, Client}; -use noirc_printable_type::ForeignCallError; use serde::{Deserialize, Serialize}; -use super::ForeignCallExecutor; +use super::{ForeignCallError, ForeignCallExecutor}; #[derive(Debug)] pub(crate) struct RPCForeignCallExecutor { diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 1306150518d..e7c64f27ab4 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -12,7 +12,6 @@ use noirc_abi::Abi; use noirc_driver::{compile_no_check, CompileError, CompileOptions, DEFAULT_EXPRESSION_WIDTH}; use noirc_errors::{debug_info::DebugInfo, FileDiagnostic}; use noirc_frontend::hir::{def_map::TestFunction, Context}; -use noirc_printable_type::ForeignCallError; use rand::Rng; use serde::{Deserialize, Serialize}; @@ -22,7 +21,7 @@ use crate::{ mocker::MockForeignCallExecutor, print::{PrintForeignCallExecutor, PrintOutput}, rpc::RPCForeignCallExecutor, - ForeignCall, ForeignCallExecutor, + ForeignCall, ForeignCallError, ForeignCallExecutor, }, NargoError, }; diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index bd5674d64f1..5f5f3748bc4 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -13,10 +13,7 @@ use acvm::{ use errors::AbiError; use input_parser::InputValue; use iter_extended::{try_btree_map, try_vecmap, vecmap}; -use noirc_printable_type::{ - decode_value as printable_type_decode_value, PrintableType, PrintableValue, - PrintableValueDisplay, -}; +use noirc_printable_type::{PrintableType, PrintableValue, PrintableValueDisplay}; use serde::{Deserialize, Serialize}; use std::borrow::Borrow; use std::{collections::BTreeMap, str}; @@ -30,8 +27,11 @@ mod arbitrary; pub mod errors; pub mod input_parser; +mod printable_type; mod serialization; +pub use printable_type::decode_value as decode_printable_value; + /// A map from the fields in an TOML/JSON file which correspond to some ABI to their values pub type InputMap = BTreeMap; @@ -417,7 +417,7 @@ pub fn decode_value( Ok(value) } -fn decode_string_value(field_elements: &[FieldElement]) -> String { +pub fn decode_string_value(field_elements: &[F]) -> String { let string_as_slice = vecmap(field_elements, |e| { let mut field_as_bytes = e.to_be_bytes(); let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element @@ -476,21 +476,21 @@ pub fn display_abi_error( AbiErrorType::FmtString { length, item_types } => { let mut fields_iter = fields.iter().copied(); let PrintableValue::String(string) = - printable_type_decode_value(&mut fields_iter, &PrintableType::String { length }) + decode_printable_value(&mut fields_iter, &PrintableType::String { length }) else { unreachable!("Got non-string from string decoding"); }; let _length_of_items = fields_iter.next(); let items = item_types.into_iter().map(|abi_type| { let printable_typ = (&abi_type).into(); - let decoded = printable_type_decode_value(&mut fields_iter, &printable_typ); + let decoded = decode_printable_value(&mut fields_iter, &printable_typ); (decoded, printable_typ) }); PrintableValueDisplay::FmtString(string, items.collect()) } AbiErrorType::Custom(abi_typ) => { let printable_type = (&abi_typ).into(); - let decoded = printable_type_decode_value(&mut fields.iter().copied(), &printable_type); + let decoded = decode_printable_value(&mut fields.iter().copied(), &printable_type); PrintableValueDisplay::Plain(decoded, printable_type) } AbiErrorType::String { string } => { diff --git a/tooling/noirc_abi/src/printable_type.rs b/tooling/noirc_abi/src/printable_type.rs new file mode 100644 index 00000000000..a81eb0ce8f6 --- /dev/null +++ b/tooling/noirc_abi/src/printable_type.rs @@ -0,0 +1,78 @@ +use std::collections::BTreeMap; + +use acvm::acir::AcirField; +use iter_extended::vecmap; + +use noirc_printable_type::{PrintableType, PrintableValue}; + +use crate::decode_string_value; + +/// Assumes that `field_iterator` contains enough field elements in order to decode the [PrintableType] +pub fn decode_value( + field_iterator: &mut impl Iterator, + typ: &PrintableType, +) -> PrintableValue { + match typ { + PrintableType::Field + | PrintableType::SignedInteger { .. } + | PrintableType::UnsignedInteger { .. } + | PrintableType::Boolean => { + let field_element = field_iterator.next().unwrap(); + + PrintableValue::Field(field_element) + } + PrintableType::Array { length, typ } => { + let length = *length as usize; + let mut array_elements = Vec::with_capacity(length); + for _ in 0..length { + array_elements.push(decode_value(field_iterator, typ)); + } + + PrintableValue::Vec { array_elements, is_slice: false } + } + PrintableType::Slice { typ } => { + let length = field_iterator + .next() + .expect("not enough data to decode variable array length") + .to_u128() as usize; + let mut array_elements = Vec::with_capacity(length); + for _ in 0..length { + array_elements.push(decode_value(field_iterator, typ)); + } + + PrintableValue::Vec { array_elements, is_slice: true } + } + PrintableType::Tuple { types } => PrintableValue::Vec { + array_elements: vecmap(types, |typ| decode_value(field_iterator, typ)), + is_slice: false, + }, + PrintableType::String { length } => { + let field_elements: Vec = field_iterator.take(*length as usize).collect(); + + PrintableValue::String(decode_string_value(&field_elements)) + } + PrintableType::Struct { fields, .. } => { + let mut struct_map = BTreeMap::new(); + + for (field_key, param_type) in fields { + let field_value = decode_value(field_iterator, param_type); + + struct_map.insert(field_key.to_owned(), field_value); + } + + PrintableValue::Struct(struct_map) + } + PrintableType::Function { env, .. } => { + let field_element = field_iterator.next().unwrap(); + let func_ref = PrintableValue::Field(field_element); + // we want to consume the fields from the environment, but for now they are not actually printed + decode_value(field_iterator, env); + func_ref + } + PrintableType::MutableReference { typ } => { + // we decode the reference, but it's not really used for printing + decode_value(field_iterator, typ) + } + PrintableType::Unit => PrintableValue::Field(F::zero()), + } +} diff --git a/tooling/noirc_artifacts/src/debug_vars.rs b/tooling/noirc_artifacts/src/debug_vars.rs index aa9328432b8..1e144e3ecc5 100644 --- a/tooling/noirc_artifacts/src/debug_vars.rs +++ b/tooling/noirc_artifacts/src/debug_vars.rs @@ -1,8 +1,9 @@ use acvm::AcirField; +use noirc_abi::decode_printable_value; use noirc_errors::debug_info::{ DebugFnId, DebugFunction, DebugInfo, DebugTypeId, DebugVarId, DebugVariable, }; -use noirc_printable_type::{decode_value, PrintableType, PrintableValue}; +use noirc_printable_type::{PrintableType, PrintableValue}; use std::collections::HashMap; #[derive(Debug, Default, Clone)] @@ -72,7 +73,7 @@ impl DebugVars { .last_mut() .expect("unexpected empty stack frames") .1 - .insert(var_id, decode_value(&mut values.iter().copied(), ptype)); + .insert(var_id, decode_printable_value(&mut values.iter().copied(), ptype)); } pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[F]) { @@ -143,7 +144,7 @@ impl DebugVars { } }; } - *cursor = decode_value(&mut values.iter().copied(), cursor_type); + *cursor = decode_printable_value(&mut values.iter().copied(), cursor_type); } pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[F]) { From 8eb77568f9814f0d54efbdee1487570d8bf076dd Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 19 Dec 2024 12:23:01 +0000 Subject: [PATCH 2/3] Update tooling/nargo/src/foreign_calls/mod.rs --- tooling/nargo/src/foreign_calls/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs index 4c2ff03cfac..1fbfc9458f4 100644 --- a/tooling/nargo/src/foreign_calls/mod.rs +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -78,7 +78,7 @@ pub enum ForeignCallError { #[error("Failed calling external resolver. {0}")] ExternalResolverError(#[from] jsonrpc::Error), - #[error("Assert message resolved after an unsatisified constrain. {0}")] + #[error("Assert message resolved after an unsatisfied constrain. {0}")] ResolvedAssertMessage(String), } From 794adf9f3bed8292542f9eb07d62c5a36e212e88 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 19 Dec 2024 12:57:26 +0000 Subject: [PATCH 3/3] . --- Cargo.lock | 1 - compiler/noirc_printable_type/Cargo.toml | 1 - compiler/noirc_printable_type/src/lib.rs | 88 +++++++++++++++++------- 3 files changed, 62 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 324e5f3350d..88fa65c7e9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3227,7 +3227,6 @@ name = "noirc_printable_type" version = "1.0.0-beta.0" dependencies = [ "acvm", - "regex", "serde", ] diff --git a/compiler/noirc_printable_type/Cargo.toml b/compiler/noirc_printable_type/Cargo.toml index 73cf104ae44..a1eae750b1f 100644 --- a/compiler/noirc_printable_type/Cargo.toml +++ b/compiler/noirc_printable_type/Cargo.toml @@ -14,6 +14,5 @@ workspace = true [dependencies] acvm.workspace = true serde.workspace = true -regex = "1.9.1" [dev-dependencies] diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 6c3dbfe6c65..eb74d2470fb 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -7,7 +7,6 @@ use std::{collections::BTreeMap, str}; use acvm::AcirField; -use regex::{Captures, Regex}; use serde::{Deserialize, Serialize}; #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -70,7 +69,6 @@ pub enum PrintableValueDisplay { Plain(PrintableValue, PrintableType), FmtString(String, Vec<(PrintableValue, PrintableType)>), } - impl std::fmt::Display for PrintableValueDisplay { fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -79,15 +77,10 @@ impl std::fmt::Display for PrintableValueDisplay { write!(fmt, "{output_string}") } Self::FmtString(template, values) => { - let mut display_iter = values.iter(); - let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}").map_err(|_| std::fmt::Error)?; - - let formatted_str = replace_all(&re, template, |_: &Captures| { - let (value, typ) = display_iter.next().ok_or(std::fmt::Error)?; - to_string(value, typ).ok_or(std::fmt::Error) - })?; - - write!(fmt, "{formatted_str}") + let mut values_iter = values.iter(); + write_template_replacing_interpolations(template, fmt, || { + values_iter.next().and_then(|(value, typ)| to_string(value, typ)) + }) } } } @@ -190,22 +183,65 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op Some(output) } -// Taken from Regex docs directly -fn replace_all( - re: &Regex, - haystack: &str, - mut replacement: impl FnMut(&Captures) -> Result, -) -> Result { - let mut new = String::with_capacity(haystack.len()); - let mut last_match = 0; - for caps in re.captures_iter(haystack) { - let m = caps.get(0).unwrap(); - new.push_str(&haystack[last_match..m.start()]); - new.push_str(&replacement(&caps)?); - last_match = m.end(); +fn write_template_replacing_interpolations( + template: &str, + fmt: &mut std::fmt::Formatter<'_>, + mut replacement: impl FnMut() -> Option, +) -> std::fmt::Result { + let mut last_index = 0; // How far we've written from the template + let mut char_indices = template.char_indices().peekable(); + while let Some((char_index, char)) = char_indices.next() { + // If we see a '}' it must be "}}" because the ones for interpolation are handled + // when we see '{' + if char == '}' { + // Write what we've seen so far in the template, including this '}' + write!(fmt, "{}", &template[last_index..=char_index])?; + + // Skip the second '}' + let (_, closing_curly) = char_indices.next().unwrap(); + assert_eq!(closing_curly, '}'); + + last_index = char_indices.peek().map(|(index, _)| *index).unwrap_or(template.len()); + continue; + } + + // Keep going forward until we find a '{' + if char != '{' { + continue; + } + + // We'll either have to write an interpolation or '{{' if it's an escape, + // so let's write what we've seen so far in the template. + write!(fmt, "{}", &template[last_index..char_index])?; + + // If it's '{{', write '{' and keep going + if char_indices.peek().map(|(_, char)| char) == Some(&'{') { + write!(fmt, "{{")?; + + // Skip the second '{' + char_indices.next().unwrap(); + + last_index = char_indices.peek().map(|(index, _)| *index).unwrap_or(template.len()); + continue; + } + + // Write the interpolation + if let Some(string) = replacement() { + write!(fmt, "{}", string)?; + } else { + return Err(std::fmt::Error); + } + + // Whatever was inside '{...}' doesn't matter, so skip until we find '}' + while let Some((_, char)) = char_indices.next() { + if char == '}' { + last_index = char_indices.peek().map(|(index, _)| *index).unwrap_or(template.len()); + break; + } + } } - new.push_str(&haystack[last_match..]); - Ok(new) + + write!(fmt, "{}", &template[last_index..]) } /// This trims any leading zeroes.