From 3d2779a84d1835a769a4a7888cac6cfce4802aa0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Aug 2023 18:58:54 +0000 Subject: [PATCH 1/6] chore(deps): bump rustls-webpki from 0.101.1 to 0.101.4 (#2404) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3b68b5004d8..2fc63b9f6f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3203,9 +3203,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.101.1" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "15f36a6828982f422756984e47912a7a51dcbc2a197aa791158f8ca61cd8204e" +checksum = "7d93931baf2d282fff8d3a532bbfd7653f734643161b87e3e01e59a04439bf0d" dependencies = [ "ring", "untrusted", From 1d3fe5065868a845642b694ba068f5e0d4f85ae9 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 22 Aug 2023 22:32:45 +0100 Subject: [PATCH 2/6] chore: clippy fix (#2408) --- crates/noirc_frontend/src/lexer/lexer.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/noirc_frontend/src/lexer/lexer.rs b/crates/noirc_frontend/src/lexer/lexer.rs index d0fa475426a..fc72f43674d 100644 --- a/crates/noirc_frontend/src/lexer/lexer.rs +++ b/crates/noirc_frontend/src/lexer/lexer.rs @@ -459,10 +459,7 @@ fn custom_attribute() { let mut lexer = Lexer::new(input); let token = lexer.next().unwrap().unwrap(); - assert_eq!( - token.token(), - &Token::Attribute(Attribute::Custom("custom(hello)".to_string().into())) - ); + assert_eq!(token.token(), &Token::Attribute(Attribute::Custom("custom(hello)".to_string()))); } #[test] From 707685cbb339f4ea83f88a1df6d3955acef5a125 Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Tue, 22 Aug 2023 14:51:48 -0700 Subject: [PATCH 3/6] chore: Rework `CrateGraph` to only have one root crate (#2391) --- crates/nargo/src/lib.rs | 4 +- crates/noirc_driver/src/lib.rs | 9 +- crates/noirc_frontend/src/graph/mod.rs | 145 +++++++++++++++++++++---- crates/noirc_frontend/src/hir/mod.rs | 4 + crates/wasm/src/compile.rs | 6 +- 5 files changed, 143 insertions(+), 25 deletions(-) diff --git a/crates/nargo/src/lib.rs b/crates/nargo/src/lib.rs index 44261307886..2bc62f9a96d 100644 --- a/crates/nargo/src/lib.rs +++ b/crates/nargo/src/lib.rs @@ -17,7 +17,7 @@ pub mod workspace; use std::collections::BTreeMap; use fm::FileManager; -use noirc_driver::{add_dep, prepare_crate}; +use noirc_driver::{add_dep, prepare_crate, prepare_dependency}; use noirc_frontend::{ graph::{CrateGraph, CrateId, CrateName}, hir::Context, @@ -34,7 +34,7 @@ pub fn prepare_dependencies( for (dep_name, dep) in dependencies.iter() { match dep { Dependency::Remote { package } | Dependency::Local { package } => { - let crate_id = prepare_crate(context, &package.entry_path); + let crate_id = prepare_dependency(context, &package.entry_path); add_dep(context, parent_crate, crate_id, dep_name.clone()); prepare_dependencies(context, crate_id, &package.dependencies); } diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 1192416b98f..c0966afd454 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -56,13 +56,20 @@ pub fn compile_file( compile_main(context, crate_id, &CompileOptions::default()) } -/// Adds the file from the file system at `Path` to the crate graph +/// Adds the file from the file system at `Path` to the crate graph as a root file pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { let root_file_id = context.file_manager.add_file(file_name).unwrap(); context.crate_graph.add_crate_root(root_file_id) } +// Adds the file from the file system at `Path` to the crate graph +pub fn prepare_dependency(context: &mut Context, file_name: &Path) -> CrateId { + let root_file_id = context.file_manager.add_file(file_name).unwrap(); + + context.crate_graph.add_crate(root_file_id) +} + /// Adds a edge in the crate graph for two crates pub fn add_dep( context: &mut Context, diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs index 90490c47c36..0305854ca32 100644 --- a/crates/noirc_frontend/src/graph/mod.rs +++ b/crates/noirc_frontend/src/graph/mod.rs @@ -12,18 +12,24 @@ use smol_str::SmolStr; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum CrateId { + Root(usize), Crate(usize), Stdlib(usize), + Dummy, } impl CrateId { pub fn dummy_id() -> CrateId { - CrateId::Crate(std::usize::MAX) + CrateId::Dummy } pub fn is_stdlib(&self) -> bool { matches!(self, CrateId::Stdlib(_)) } + + pub fn is_root(&self) -> bool { + matches!(self, CrateId::Root(_)) + } } #[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)] @@ -117,29 +123,73 @@ impl Dependency { } impl CrateGraph { + pub fn root_crate_id(&self) -> &CrateId { + self.arena + .keys() + .find(|crate_id| crate_id.is_root()) + .expect("ICE: A root crate should exist in the CrateGraph") + } + pub fn add_crate_root(&mut self, file_id: FileId) -> CrateId { - let mut roots_with_file_id = - self.arena.iter().filter(|(_, crate_data)| crate_data.root_file_id == file_id); + for (crate_id, crate_data) in self.arena.iter() { + if crate_id.is_root() { + panic!("ICE: Cannot add two crate roots to a graph - use `add_crate` instead"); + } - let next_file_id = roots_with_file_id.next(); - if let Some(file_id) = next_file_id { - return *file_id.0; + if crate_data.root_file_id == file_id { + panic!("ICE: This FileId was already added to the CrateGraph") + } } let data = CrateData { root_file_id: file_id, dependencies: Vec::new() }; - let crate_id = CrateId::Crate(self.arena.len()); + let crate_id = CrateId::Root(self.arena.len()); let prev = self.arena.insert(crate_id, data); assert!(prev.is_none()); crate_id } + pub fn add_crate(&mut self, file_id: FileId) -> CrateId { + let mut crates_with_file_id = self + .arena + .iter() + .filter(|(_, crate_data)| crate_data.root_file_id == file_id) + .peekable(); + + let matching_id = crates_with_file_id.next(); + if crates_with_file_id.peek().is_some() { + panic!("ICE: Found multiple crates with the same FileId"); + } + + match matching_id { + Some((crate_id @ CrateId::Crate(_), _)) => *crate_id, + Some((CrateId::Root(_), _)) => { + panic!("ICE: Tried to re-add the root crate as a regular crate") + } + Some((CrateId::Stdlib(_), _)) => { + panic!("ICE: Tried to re-add the stdlib crate as a regular crate") + } + Some((CrateId::Dummy, _)) => { + panic!("ICE: A dummy CrateId should not exist in the CrateGraph") + } + None => { + let data = CrateData { root_file_id: file_id, dependencies: Vec::new() }; + let crate_id = CrateId::Crate(self.arena.len()); + let prev = self.arena.insert(crate_id, data); + assert!(prev.is_none()); + crate_id + } + } + } + pub fn add_stdlib(&mut self, file_id: FileId) -> CrateId { - let mut roots_with_file_id = - self.arena.iter().filter(|(_, crate_data)| crate_data.root_file_id == file_id); + for (crate_id, crate_data) in self.arena.iter() { + if crate_id.is_stdlib() { + panic!("ICE: Cannot add two stdlib crates to a graph - use `add_crate` instead"); + } - let next_file_id = roots_with_file_id.next(); - if let Some(file_id) = next_file_id { - return *file_id.0; + if crate_data.root_file_id == file_id { + panic!("ICE: This FileId was already added to the CrateGraph") + } } let data = CrateData { root_file_id: file_id, dependencies: Vec::new() }; @@ -263,8 +313,8 @@ mod tests { let mut graph = CrateGraph::default(); let crate1 = graph.add_crate_root(file_ids[0]); - let crate2 = graph.add_crate_root(file_ids[1]); - let crate3 = graph.add_crate_root(file_ids[2]); + let crate2 = graph.add_crate(file_ids[1]); + let crate3 = graph.add_crate(file_ids[2]); assert!(graph.add_dep(crate1, "crate2".parse().unwrap(), crate2).is_ok()); assert!(graph.add_dep(crate2, "crate3".parse().unwrap(), crate3).is_ok()); @@ -279,8 +329,8 @@ mod tests { let file_id_2 = file_ids[2]; let mut graph = CrateGraph::default(); let crate1 = graph.add_crate_root(file_id_0); - let crate2 = graph.add_crate_root(file_id_1); - let crate3 = graph.add_crate_root(file_id_2); + let crate2 = graph.add_crate(file_id_1); + let crate3 = graph.add_crate(file_id_2); assert!(graph.add_dep(crate1, "crate2".parse().unwrap(), crate2).is_ok()); assert!(graph.add_dep(crate2, "crate3".parse().unwrap(), crate3).is_ok()); } @@ -292,11 +342,68 @@ mod tests { let file_id_2 = file_ids[2]; let mut graph = CrateGraph::default(); let _crate1 = graph.add_crate_root(file_id_0); - let _crate2 = graph.add_crate_root(file_id_1); + let _crate2 = graph.add_crate(file_id_1); // Adding the same file, so the crate should be the same. - let crate3 = graph.add_crate_root(file_id_2); - let crate3_2 = graph.add_crate_root(file_id_2); + let crate3 = graph.add_crate(file_id_2); + let crate3_2 = graph.add_crate(file_id_2); assert_eq!(crate3, crate3_2); } + + #[test] + #[should_panic = "ICE: Cannot add two crate roots to a graph - use `add_crate` instead"] + fn panics_if_adding_two_roots() { + let file_ids = dummy_file_ids(2); + let mut graph = CrateGraph::default(); + let _ = graph.add_crate_root(file_ids[0]); + let _ = graph.add_crate_root(file_ids[1]); + } + + #[test] + #[should_panic = "ICE: This FileId was already added to the CrateGraph"] + fn panics_if_adding_existing_file_as_root() { + let file_ids = dummy_file_ids(1); + let mut graph = CrateGraph::default(); + let file_id_0 = file_ids[0]; + let _ = graph.add_crate(file_id_0); + let _ = graph.add_crate_root(file_id_0); + } + + #[test] + #[should_panic = "ICE: Cannot add two stdlib crates to a graph - use `add_crate` instead"] + fn panics_if_adding_two_stdlib() { + let file_ids = dummy_file_ids(2); + let mut graph = CrateGraph::default(); + let _ = graph.add_stdlib(file_ids[0]); + let _ = graph.add_stdlib(file_ids[1]); + } + + #[test] + #[should_panic = "ICE: This FileId was already added to the CrateGraph"] + fn panics_if_adding_existing_file_as_stdlib() { + let file_ids = dummy_file_ids(1); + let mut graph = CrateGraph::default(); + let file_id_0 = file_ids[0]; + let _ = graph.add_crate(file_id_0); + let _ = graph.add_stdlib(file_id_0); + } + + #[test] + #[should_panic = "ICE: Tried to re-add the root crate as a regular crate"] + fn panics_if_adding_root_as_regular() { + let file_ids = dummy_file_ids(1); + let mut graph = CrateGraph::default(); + let file_id_0 = file_ids[0]; + let _ = graph.add_crate_root(file_id_0); + let _ = graph.add_crate(file_id_0); + } + #[test] + #[should_panic = "ICE: Tried to re-add the stdlib crate as a regular crate"] + fn panics_if_adding_stdlib_as_regular() { + let file_ids = dummy_file_ids(1); + let mut graph = CrateGraph::default(); + let file_id_0 = file_ids[0]; + let _ = graph.add_stdlib(file_id_0); + let _ = graph.add_crate(file_id_0); + } } diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs index 497935c8ece..d0b24e90a76 100644 --- a/crates/noirc_frontend/src/hir/mod.rs +++ b/crates/noirc_frontend/src/hir/mod.rs @@ -59,6 +59,10 @@ impl Context { self.crate_graph.iter_keys() } + pub fn root_crate_id(&self) -> &CrateId { + self.crate_graph.root_crate_id() + } + // TODO: Decide if we actually need `function_name` and `fully_qualified_function_name` pub fn function_name(&self, id: &FuncId) -> &str { self.def_interner.function_name(id) diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index 2e923574bbf..01f286f924f 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -3,8 +3,8 @@ use fm::FileManager; use gloo_utils::format::JsValueSerdeExt; use log::debug; use noirc_driver::{ - check_crate, compile_contracts, compile_no_check, prepare_crate, propagate_dep, CompileOptions, - CompiledContract, + check_crate, compile_contracts, compile_no_check, prepare_crate, prepare_dependency, + propagate_dep, CompileOptions, CompiledContract, }; use noirc_frontend::{graph::CrateGraph, hir::Context}; use serde::{Deserialize, Serialize}; @@ -60,7 +60,7 @@ impl Default for WASMCompileOptions { fn add_noir_lib(context: &mut Context, crate_name: &str) { let path_to_lib = Path::new(&crate_name).join("lib.nr"); - let library_crate = prepare_crate(context, &path_to_lib); + let library_crate = prepare_dependency(context, &path_to_lib); propagate_dep(context, library_crate, &crate_name.parse().unwrap()); } From a1d12675a8bc75651d9634776c9d6c7cbf81ff7c Mon Sep 17 00:00:00 2001 From: Ethan-000 Date: Wed, 23 Aug 2023 15:34:44 +0100 Subject: [PATCH 4/6] feat: Report compilation warnings before errors (#2398) Co-authored-by: jfecher --- crates/noirc_errors/src/reporter.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/noirc_errors/src/reporter.rs b/crates/noirc_errors/src/reporter.rs index 8f4cdd9b551..d55189e80ac 100644 --- a/crates/noirc_errors/src/reporter.rs +++ b/crates/noirc_errors/src/reporter.rs @@ -115,6 +115,10 @@ pub fn report_all( diagnostics: &[FileDiagnostic], deny_warnings: bool, ) -> ReportedErrors { + // Report warnings before any errors + let (mut diagnostics, mut errors): (Vec<_>, _) = + diagnostics.iter().partition(|item| item.diagnostic.is_warning()); + diagnostics.append(&mut errors); let error_count = diagnostics.iter().map(|error| error.report(files, deny_warnings) as u32).sum(); From 3771e521110da845a14058b97c5e5037daf599b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Wed, 23 Aug 2023 16:09:06 +0100 Subject: [PATCH 5/6] feat(brillig): Added locations for brillig artifacts (#2415) --- .../src/brillig/brillig_gen/brillig_block.rs | 4 + .../brillig/brillig_gen/brillig_directive.rs | 119 ++++++++++-------- .../noirc_evaluator/src/brillig/brillig_ir.rs | 17 ++- .../src/brillig/brillig_ir/artifact.rs | 33 ++++- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 9 +- .../ssa/acir_gen/acir_ir/generated_acir.rs | 13 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 5 +- 7 files changed, 131 insertions(+), 69 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 38df0374a96..831ad3d5d2a 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1,6 +1,7 @@ use crate::brillig::brillig_ir::{ BrilligBinaryOp, BrilligContext, BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE, }; +use crate::ssa::ir::dfg::CallStack; use crate::ssa::ir::{ basic_block::{BasicBlock, BasicBlockId}, dfg::DataFlowGraph, @@ -202,6 +203,7 @@ impl<'block> BrilligBlock<'block> { /// Converts an SSA instruction into a sequence of Brillig opcodes. fn convert_ssa_instruction(&mut self, instruction_id: InstructionId, dfg: &DataFlowGraph) { let instruction = &dfg[instruction_id]; + self.brillig_context.set_call_stack(dfg.get_call_stack(instruction_id)); match instruction { Instruction::Binary(binary) => { @@ -479,6 +481,8 @@ impl<'block> BrilligBlock<'block> { } _ => todo!("ICE: Instruction not supported {instruction:?}"), }; + + self.brillig_context.set_call_stack(CallStack::new()); } fn convert_ssa_function_call( diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs index 93e760f9737..6b53b7c2069 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs @@ -2,8 +2,10 @@ use acvm::acir::brillig::{ BinaryFieldOp, BinaryIntOp, Opcode as BrilligOpcode, RegisterIndex, Value, }; +use crate::brillig::brillig_ir::artifact::GeneratedBrillig; + /// Generates brillig bytecode which computes the inverse of its input if not null, and zero else. -pub(crate) fn directive_invert() -> Vec { +pub(crate) fn directive_invert() -> GeneratedBrillig { // We generate the following code: // fn invert(x : Field) -> Field { // 1/ x @@ -16,20 +18,23 @@ pub(crate) fn directive_invert() -> Vec { // Location of the stop opcode let stop_location = 3; - vec![ - // If the input is zero, then we jump to the stop opcode - BrilligOpcode::JumpIfNot { condition: input, location: stop_location }, - // Put value one in register (1) - BrilligOpcode::Const { destination: one_const, value: Value::from(1_usize) }, - // Divide 1 by the input, and set the result of the division into register (0) - BrilligOpcode::BinaryFieldOp { - op: BinaryFieldOp::Div, - lhs: one_const, - rhs: input, - destination: input, - }, - BrilligOpcode::Stop, - ] + GeneratedBrillig { + byte_code: vec![ + // If the input is zero, then we jump to the stop opcode + BrilligOpcode::JumpIfNot { condition: input, location: stop_location }, + // Put value one in register (1) + BrilligOpcode::Const { destination: one_const, value: Value::from(1_usize) }, + // Divide 1 by the input, and set the result of the division into register (0) + BrilligOpcode::BinaryFieldOp { + op: BinaryFieldOp::Div, + lhs: one_const, + rhs: input, + destination: input, + }, + BrilligOpcode::Stop, + ], + locations: Default::default(), + } } /// Generates brillig bytecode which computes `a / b` and returns the quotient and remainder. @@ -47,43 +52,55 @@ pub(crate) fn directive_invert() -> Vec { /// } /// } /// ``` -pub(crate) fn directive_quotient(bit_size: u32) -> Vec { +pub(crate) fn directive_quotient(bit_size: u32) -> GeneratedBrillig { // `a` is (0) (i.e register index 0) // `b` is (1) // `predicate` is (2) - vec![ - // If the predicate is zero, we jump to the exit segment - BrilligOpcode::JumpIfNot { condition: RegisterIndex::from(2), location: 6 }, - //q = a/b is set into register (3) - BrilligOpcode::BinaryIntOp { - op: BinaryIntOp::UnsignedDiv, - lhs: RegisterIndex::from(0), - rhs: RegisterIndex::from(1), - destination: RegisterIndex::from(3), - bit_size, - }, - //(1)= q*b - BrilligOpcode::BinaryIntOp { - op: BinaryIntOp::Mul, - lhs: RegisterIndex::from(3), - rhs: RegisterIndex::from(1), - destination: RegisterIndex::from(1), - bit_size, - }, - //(1) = a-q*b - BrilligOpcode::BinaryIntOp { - op: BinaryIntOp::Sub, - lhs: RegisterIndex::from(0), - rhs: RegisterIndex::from(1), - destination: RegisterIndex::from(1), - bit_size, - }, - //(0) = q - BrilligOpcode::Mov { destination: RegisterIndex::from(0), source: RegisterIndex::from(3) }, - BrilligOpcode::Stop, - // Exit segment: we return 0,0 - BrilligOpcode::Const { destination: RegisterIndex::from(0), value: Value::from(0_usize) }, - BrilligOpcode::Const { destination: RegisterIndex::from(1), value: Value::from(0_usize) }, - BrilligOpcode::Stop, - ] + GeneratedBrillig { + byte_code: vec![ + // If the predicate is zero, we jump to the exit segment + BrilligOpcode::JumpIfNot { condition: RegisterIndex::from(2), location: 6 }, + //q = a/b is set into register (3) + BrilligOpcode::BinaryIntOp { + op: BinaryIntOp::UnsignedDiv, + lhs: RegisterIndex::from(0), + rhs: RegisterIndex::from(1), + destination: RegisterIndex::from(3), + bit_size, + }, + //(1)= q*b + BrilligOpcode::BinaryIntOp { + op: BinaryIntOp::Mul, + lhs: RegisterIndex::from(3), + rhs: RegisterIndex::from(1), + destination: RegisterIndex::from(1), + bit_size, + }, + //(1) = a-q*b + BrilligOpcode::BinaryIntOp { + op: BinaryIntOp::Sub, + lhs: RegisterIndex::from(0), + rhs: RegisterIndex::from(1), + destination: RegisterIndex::from(1), + bit_size, + }, + //(0) = q + BrilligOpcode::Mov { + destination: RegisterIndex::from(0), + source: RegisterIndex::from(3), + }, + BrilligOpcode::Stop, + // Exit segment: we return 0,0 + BrilligOpcode::Const { + destination: RegisterIndex::from(0), + value: Value::from(0_usize), + }, + BrilligOpcode::Const { + destination: RegisterIndex::from(1), + value: Value::from(0_usize), + }, + BrilligOpcode::Stop, + ], + locations: Default::default(), + } } diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir.rs b/crates/noirc_evaluator/src/brillig/brillig_ir.rs index 047e8b7edf8..41e52434009 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir.rs @@ -10,6 +10,8 @@ pub(crate) mod registers; mod entry_point; +use crate::ssa::ir::dfg::CallStack; + use self::{ artifact::{BrilligArtifact, UnresolvedJumpLocation}, registers::BrilligRegistersContext, @@ -104,7 +106,7 @@ impl BrilligContext { /// Adds a brillig instruction to the brillig byte code pub(crate) fn push_opcode(&mut self, opcode: BrilligOpcode) { - self.obj.byte_code.push(opcode); + self.obj.push_opcode(opcode); } /// Returns the artifact @@ -945,6 +947,11 @@ impl BrilligContext { _ => unreachable!("ICE: Expected vector, got {variable:?}"), } } + + /// Sets a current call stack that the next pushed opcodes will be associated with. + pub(crate) fn set_call_stack(&mut self, call_stack: CallStack) { + self.obj.set_call_stack(call_stack); + } } /// Type to encapsulate the binary operation types in Brillig @@ -970,7 +977,7 @@ pub(crate) mod tests { use crate::brillig::brillig_ir::BrilligContext; - use super::artifact::BrilligParameter; + use super::artifact::{BrilligParameter, GeneratedBrillig}; use super::{BrilligOpcode, ReservedRegisters}; pub(crate) struct DummyBlackBoxSolver; @@ -1010,7 +1017,7 @@ pub(crate) mod tests { context: BrilligContext, arguments: Vec, returns: Vec, - ) -> Vec { + ) -> GeneratedBrillig { let artifact = context.artifact(); let mut entry_point_artifact = BrilligContext::new_entry_point_artifact(arguments, returns, "test".to_string()); @@ -1028,7 +1035,7 @@ pub(crate) mod tests { let mut vm = VM::new( Registers { inner: param_registers }, memory, - create_entry_point_bytecode(context, arguments, returns), + create_entry_point_bytecode(context, arguments, returns).byte_code, vec![], &DummyBlackBoxSolver, ); @@ -1079,7 +1086,7 @@ pub(crate) mod tests { context.stop_instruction(); - let bytecode = context.artifact().byte_code; + let bytecode = context.artifact().finish().byte_code; let number_sequence: Vec = (0_usize..12_usize).map(Value::from).collect(); let mut vm = VM::new( Registers { inner: vec![] }, diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 46e3df2e465..627e096bfc9 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -1,5 +1,7 @@ use acvm::acir::brillig::Opcode as BrilligOpcode; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; + +use crate::ssa::ir::dfg::CallStack; /// Represents a parameter or a return value of a function. #[derive(Debug, Clone)] @@ -9,11 +11,19 @@ pub(crate) enum BrilligParameter { Slice(Vec), } +/// The result of compiling and linking brillig artifacts. +/// This is ready to run bytecode with attached metadata. +#[derive(Debug)] +pub(crate) struct GeneratedBrillig { + pub(crate) byte_code: Vec, + pub(crate) locations: BTreeMap, +} + #[derive(Default, Debug, Clone)] /// Artifacts resulting from the compilation of a function into brillig byte code. /// Currently it is just the brillig bytecode of the function. pub(crate) struct BrilligArtifact { - pub(crate) byte_code: Vec, + byte_code: Vec, /// The set of jumps that need to have their locations /// resolved. unresolved_jumps: Vec<(JumpInstructionPosition, UnresolvedJumpLocation)>, @@ -26,6 +36,10 @@ pub(crate) struct BrilligArtifact { /// TODO: perhaps we should combine this with the `unresolved_jumps` field /// TODO: and have an enum which indicates whether the jump is internal or external unresolved_external_call_labels: Vec<(JumpInstructionPosition, UnresolvedJumpLocation)>, + /// Maps the opcodes that are associated with a callstack to it. + locations: BTreeMap, + /// The current call stack. All opcodes that are pushed will be associated with this call stack. + call_stack: CallStack, } /// A pointer to a location in the opcode. @@ -52,9 +66,9 @@ pub(crate) type UnresolvedJumpLocation = Label; impl BrilligArtifact { /// Resolves all jumps and generates the final bytecode - pub(crate) fn finish(mut self) -> Vec { + pub(crate) fn finish(mut self) -> GeneratedBrillig { self.resolve_jumps(); - self.byte_code + GeneratedBrillig { byte_code: self.byte_code, locations: self.locations } } /// Gets the first unresolved function call of this artifact. @@ -116,10 +130,17 @@ impl BrilligArtifact { self.unresolved_external_call_labels .push((position_in_bytecode + offset, label_id.clone())); } + + for (position_in_bytecode, call_stack) in obj.locations.iter() { + self.locations.insert(position_in_bytecode + offset, call_stack.clone()); + } } /// Adds a brillig instruction to the brillig byte code pub(crate) fn push_opcode(&mut self, opcode: BrilligOpcode) { + if !self.call_stack.is_empty() { + self.locations.insert(self.index_of_next_opcode(), self.call_stack.clone()); + } self.byte_code.push(opcode); } @@ -217,4 +238,8 @@ impl BrilligArtifact { } } } + + pub(crate) fn set_call_stack(&mut self, call_stack: CallStack) { + self.call_stack = call_stack; + } } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index b28a4a90ada..fe31608db8d 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1,5 +1,6 @@ use super::generated_acir::GeneratedAcir; use crate::brillig::brillig_gen::brillig_directive; +use crate::brillig::brillig_ir::artifact::GeneratedBrillig; use crate::errors::{InternalError, RuntimeError}; use crate::ssa::acir_gen::{AcirDynamicArray, AcirValue}; use crate::ssa::ir::dfg::CallStack; @@ -910,7 +911,7 @@ impl AcirContext { pub(crate) fn brillig( &mut self, predicate: AcirVar, - code: Vec, + generated_brillig: GeneratedBrillig, inputs: Vec, outputs: Vec, ) -> Result, InternalError> { @@ -932,7 +933,9 @@ impl AcirContext { // Optimistically try executing the brillig now, if we can complete execution they just return the results. // This is a temporary measure pending SSA optimizations being applied to Brillig which would remove constant-input opcodes (See #2066) - if let Some(brillig_outputs) = self.execute_brillig(code.clone(), &b_inputs, &outputs) { + if let Some(brillig_outputs) = + self.execute_brillig(generated_brillig.byte_code.clone(), &b_inputs, &outputs) + { return Ok(brillig_outputs); } @@ -953,7 +956,7 @@ impl AcirContext { } }); let predicate = self.var_to_expression(predicate)?; - self.acir_ir.brillig(Some(predicate), code, b_inputs, b_outputs); + self.acir_ir.brillig(Some(predicate), generated_brillig, b_inputs, b_outputs); Ok(outputs_var) } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 9201179803b..29ca4fa3892 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -3,13 +3,12 @@ use std::collections::BTreeMap; use crate::{ - brillig::brillig_gen::brillig_directive, + brillig::{brillig_gen::brillig_directive, brillig_ir::artifact::GeneratedBrillig}, errors::{InternalError, RuntimeError}, ssa::ir::dfg::CallStack, }; use acvm::acir::{ - brillig::Opcode as BrilligOpcode, circuit::{ brillig::{Brillig as AcvmBrillig, BrilligInputs, BrilligOutputs}, opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode}, @@ -788,7 +787,7 @@ impl GeneratedAcir { pub(crate) fn brillig( &mut self, predicate: Option, - code: Vec, + generated_brillig: GeneratedBrillig, inputs: Vec, outputs: Vec, ) { @@ -796,10 +795,16 @@ impl GeneratedAcir { inputs, outputs, foreign_call_results: Vec::new(), - bytecode: code, + bytecode: generated_brillig.byte_code, predicate, }); self.push_opcode(opcode); + for (brillig_index, call_stack) in generated_brillig.locations { + self.locations.insert( + OpcodeLocation::Brillig { acir_index: self.opcodes.len() - 1, brillig_index }, + call_stack, + ); + } } /// Generate gates and control bits witnesses which ensure that out_expr is a permutation of in_expr diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs index 4fd828f05d0..92711701c76 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -20,12 +20,13 @@ use super::{ }, ssa_gen::Ssa, }; +use crate::brillig::brillig_ir::artifact::GeneratedBrillig; use crate::brillig::brillig_ir::BrilligContext; use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig}; use crate::errors::{InternalError, RuntimeError}; pub(crate) use acir_ir::generated_acir::GeneratedAcir; use acvm::{ - acir::{brillig::Opcode, circuit::opcodes::BlockId, native_types::Expression}, + acir::{circuit::opcodes::BlockId, native_types::Expression}, FieldElement, }; use iter_extended::{try_vecmap, vecmap}; @@ -435,7 +436,7 @@ impl Context { &self, func: &Function, brillig: &Brillig, - ) -> Result, InternalError> { + ) -> Result { // Create the entry point artifact let mut entry_point = BrilligContext::new_entry_point_artifact( BrilligFunctionContext::parameters(func), From 182d017fe8f4fff1ccdd9fd55be6a21856110903 Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Wed, 23 Aug 2023 08:09:11 -0700 Subject: [PATCH 6/6] chore: Decouple `noirc_abi` from frontend by introducing `PrintableType` (#2373) Co-authored-by: Maxim Vezenov --- Cargo.lock | 18 +- Cargo.toml | 2 + crates/nargo/Cargo.toml | 3 +- crates/nargo/src/errors.rs | 16 +- crates/nargo/src/ops/foreign_calls.rs | 94 +---- crates/noirc_abi/Cargo.toml | 1 + crates/noirc_abi/src/errors.rs | 4 - crates/noirc_abi/src/input_parser/json.rs | 10 - crates/noirc_abi/src/input_parser/mod.rs | 33 +- crates/noirc_abi/src/lib.rs | 75 +++- crates/noirc_driver/src/lib.rs | 2 +- crates/noirc_evaluator/src/ssa/abi_gen/mod.rs | 6 +- crates/noirc_frontend/Cargo.toml | 2 +- crates/noirc_frontend/src/ast/mod.rs | 21 -- crates/noirc_frontend/src/hir_def/types.rs | 107 +++--- .../src/monomorphization/mod.rs | 24 +- crates/noirc_printable_type/Cargo.toml | 17 + crates/noirc_printable_type/src/lib.rs | 321 ++++++++++++++++++ 18 files changed, 511 insertions(+), 245 deletions(-) create mode 100644 crates/noirc_printable_type/Cargo.toml create mode 100644 crates/noirc_printable_type/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 2fc63b9f6f7..647acb20e05 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2234,10 +2234,9 @@ dependencies = [ "noirc_driver", "noirc_errors", "noirc_frontend", - "regex", + "noirc_printable_type", "rustc_version", "serde", - "serde_json", "thiserror", ] @@ -2354,6 +2353,7 @@ version = "0.10.3" dependencies = [ "acvm", "iter-extended", + "noirc_frontend", "serde", "serde_json", "strum", @@ -2413,8 +2413,8 @@ dependencies = [ "chumsky", "fm", "iter-extended", - "noirc_abi", "noirc_errors", + "noirc_printable_type", "regex", "rustc-hash", "serde_json", @@ -2425,6 +2425,18 @@ dependencies = [ "thiserror", ] +[[package]] +name = "noirc_printable_type" +version = "0.10.3" +dependencies = [ + "acvm", + "iter-extended", + "regex", + "serde", + "serde_json", + "thiserror", +] + [[package]] name = "nom" version = "7.1.3" diff --git a/Cargo.toml b/Cargo.toml index 76ec9edfa0d..998eb2bb65b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,6 +5,7 @@ members = [ "crates/noirc_frontend", "crates/noirc_errors", "crates/noirc_driver", + "crates/noirc_printable_type", "crates/nargo", "crates/nargo_cli", "crates/nargo_toml", @@ -38,6 +39,7 @@ noirc_driver = { path = "crates/noirc_driver" } noirc_errors = { path = "crates/noirc_errors" } noirc_evaluator = { path = "crates/noirc_evaluator" } noirc_frontend = { path = "crates/noirc_frontend" } +noirc_printable_type = { path = "crates/noirc_printable_type" } noir_wasm = { path = "crates/wasm" } cfg-if = "1.0.0" clap = { version = "4.3.19", features = ["derive"] } diff --git a/crates/nargo/Cargo.toml b/crates/nargo/Cargo.toml index 32ca04ad34f..25c73a3c025 100644 --- a/crates/nargo/Cargo.toml +++ b/crates/nargo/Cargo.toml @@ -16,10 +16,9 @@ fm.workspace = true noirc_abi.workspace = true noirc_driver.workspace = true noirc_frontend.workspace = true +noirc_printable_type.workspace = true iter-extended.workspace = true serde.workspace = true -serde_json.workspace = true thiserror.workspace = true noirc_errors.workspace = true base64.workspace = true -regex = "1.9.1" diff --git a/crates/nargo/src/errors.rs b/crates/nargo/src/errors.rs index 2878d9f2db7..1c99d27ae42 100644 --- a/crates/nargo/src/errors.rs +++ b/crates/nargo/src/errors.rs @@ -1,5 +1,5 @@ use acvm::pwg::OpcodeResolutionError; -use noirc_abi::errors::{AbiError, InputParserError}; +use noirc_printable_type::ForeignCallError; use thiserror::Error; #[derive(Debug, Error)] @@ -15,17 +15,3 @@ pub enum NargoError { #[error(transparent)] ForeignCallError(#[from] ForeignCallError), } - -#[derive(Debug, Error)] -pub enum ForeignCallError { - #[error("Foreign call inputs needed for execution are missing")] - MissingForeignCallInputs, - - /// ABI encoding/decoding error - #[error(transparent)] - AbiError(#[from] AbiError), - - /// Input parsing error - #[error(transparent)] - InputParserError(#[from] InputParserError), -} diff --git a/crates/nargo/src/ops/foreign_calls.rs b/crates/nargo/src/ops/foreign_calls.rs index e1768d71c44..8eac516a7e9 100644 --- a/crates/nargo/src/ops/foreign_calls.rs +++ b/crates/nargo/src/ops/foreign_calls.rs @@ -3,10 +3,9 @@ use acvm::{ pwg::ForeignCallWaitInfo, }; use iter_extended::vecmap; -use noirc_abi::{decode_string_value, input_parser::InputValueDisplay, AbiType}; -use regex::{Captures, Regex}; +use noirc_printable_type::PrintableValueDisplay; -use crate::errors::ForeignCallError; +use crate::NargoError; /// This enumeration represents the Brillig foreign calls that are natively supported by nargo. /// After resolution of a foreign call, nargo will restart execution of the ACVM @@ -43,7 +42,7 @@ impl ForeignCall { pub(crate) fn execute( foreign_call: &ForeignCallWaitInfo, show_output: bool, - ) -> Result { + ) -> Result { let foreign_call_name = foreign_call.function.as_str(); match Self::lookup(foreign_call_name) { Some(ForeignCall::Println) => { @@ -78,90 +77,9 @@ impl ForeignCall { } } - fn execute_println(foreign_call_inputs: &[Vec]) -> Result<(), ForeignCallError> { - let (is_fmt_str, foreign_call_inputs) = - foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; - - let output_string = if is_fmt_str[0].to_field().is_one() { - convert_fmt_string_inputs(foreign_call_inputs)? - } else { - convert_string_inputs(foreign_call_inputs)? - }; - println!("{output_string}"); + fn execute_println(foreign_call_inputs: &[Vec]) -> Result<(), NargoError> { + let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; + println!("{display_values}"); Ok(()) } } - -fn convert_string_inputs(foreign_call_inputs: &[Vec]) -> Result { - // Fetch the abi type from the foreign call input - // The remaining input values should hold what is to be printed - let (abi_type_as_values, input_values) = - foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; - let abi_type = fetch_abi_type(abi_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(|values| vecmap(values, |value| value.to_field())); - - let input_value_display = - InputValueDisplay::try_from_fields(&mut input_values_as_fields, abi_type)?; - - Ok(input_value_display.to_string()) -} - -fn convert_fmt_string_inputs( - foreign_call_inputs: &[Vec], -) -> Result { - let (message_as_values, input_and_abi_values) = - foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; - - let message_as_fields = vecmap(message_as_values, |value| value.to_field()); - let message_as_string = decode_string_value(&message_as_fields); - - let (num_values, input_and_abi_values) = - input_and_abi_values.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; - - let mut output_strings = Vec::new(); - let num_values = num_values[0].to_field().to_u128() as usize; - - let mut abi_types = Vec::new(); - for abi_values in input_and_abi_values.iter().skip(input_and_abi_values.len() - num_values) { - let abi_type = fetch_abi_type(abi_values)?; - abi_types.push(abi_type); - } - - for i in 0..num_values { - let abi_type = &abi_types[i]; - let type_size = abi_type.field_count() as usize; - - let mut input_values_as_fields = input_and_abi_values[i..(i + type_size)] - .iter() - .flat_map(|values| vecmap(values, |value| value.to_field())); - - let input_value_display = - InputValueDisplay::try_from_fields(&mut input_values_as_fields, abi_type.clone())?; - - output_strings.push(input_value_display.to_string()); - } - - let mut output_strings_iter = output_strings.into_iter(); - let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}") - .expect("ICE: an invalid regex pattern was used for checking format strings"); - - let formatted_str = re.replace_all(&message_as_string, |_: &Captures| { - output_strings_iter - .next() - .expect("ICE: there are more regex matches than fields supplied to the format string") - }); - - Ok(formatted_str.into_owned()) -} - -fn fetch_abi_type(abi_type_as_values: &[Value]) -> Result { - let abi_type_as_fields = vecmap(abi_type_as_values, |value| value.to_field()); - let abi_type_as_string = decode_string_value(&abi_type_as_fields); - let abi_type: AbiType = serde_json::from_str(&abi_type_as_string) - .map_err(|err| ForeignCallError::InputParserError(err.into()))?; - - Ok(abi_type) -} diff --git a/crates/noirc_abi/Cargo.toml b/crates/noirc_abi/Cargo.toml index 6af0cfe78b3..45466061fde 100644 --- a/crates/noirc_abi/Cargo.toml +++ b/crates/noirc_abi/Cargo.toml @@ -9,6 +9,7 @@ edition.workspace = true [dependencies] acvm.workspace = true iter-extended.workspace = true +noirc_frontend.workspace = true toml.workspace = true serde_json = "1.0" serde.workspace = true diff --git a/crates/noirc_abi/src/errors.rs b/crates/noirc_abi/src/errors.rs index 80f9d665dff..687fecfcc1d 100644 --- a/crates/noirc_abi/src/errors.rs +++ b/crates/noirc_abi/src/errors.rs @@ -10,8 +10,6 @@ pub enum InputParserError { ParseStr(String), #[error("Could not parse hex value {0}")] ParseHexStr(String), - #[error("duplicate variable name {0}")] - DuplicateVariableName(String), #[error("cannot parse value into {0:?}")] AbiTypeMismatch(AbiType), #[error("Expected argument `{0}`, but none was found")] @@ -38,8 +36,6 @@ impl From for InputParserError { #[derive(Debug, Error)] pub enum AbiError { - #[error("{0}")] - Generic(String), #[error("Received parameters not expected by ABI: {0:?}")] UnexpectedParams(Vec), #[error("The parameter {} is expected to be a {:?} but found incompatible value {value:?}", .param.name, .param.typ)] diff --git a/crates/noirc_abi/src/input_parser/json.rs b/crates/noirc_abi/src/input_parser/json.rs index c8a421c8353..c6f1e304728 100644 --- a/crates/noirc_abi/src/input_parser/json.rs +++ b/crates/noirc_abi/src/input_parser/json.rs @@ -175,13 +175,3 @@ impl InputValue { Ok(input_value) } } - -impl std::fmt::Display for JsonTypes { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // From the docs: https://doc.rust-lang.org/std/fmt/struct.Error.html - // This type does not support transmission of an error other than that an error - // occurred. Any extra information must be arranged to be transmitted through - // some other means. - write!(f, "{}", serde_json::to_string(&self).map_err(|_| std::fmt::Error)?) - } -} diff --git a/crates/noirc_abi/src/input_parser/mod.rs b/crates/noirc_abi/src/input_parser/mod.rs index 3a317697534..11d40f338d5 100644 --- a/crates/noirc_abi/src/input_parser/mod.rs +++ b/crates/noirc_abi/src/input_parser/mod.rs @@ -6,8 +6,8 @@ use std::collections::BTreeMap; use acvm::FieldElement; use serde::Serialize; -use crate::errors::{AbiError, InputParserError}; -use crate::{decode_value, Abi, AbiType}; +use crate::errors::InputParserError; +use crate::{Abi, AbiType}; /// This is what all formats eventually transform into /// For example, a toml file will parse into TomlTypes /// and those TomlTypes will be mapped to Value @@ -67,35 +67,6 @@ impl InputValue { } } -/// In order to display an `InputValue` we need an `AbiType` to accurately -/// convert the value into a human-readable format. -pub struct InputValueDisplay { - input_value: InputValue, - abi_type: AbiType, -} - -impl InputValueDisplay { - pub fn try_from_fields( - field_iterator: &mut impl Iterator, - abi_type: AbiType, - ) -> Result { - let input_value = decode_value(field_iterator, &abi_type)?; - Ok(InputValueDisplay { input_value, abi_type }) - } -} - -impl std::fmt::Display for InputValueDisplay { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // From the docs: https://doc.rust-lang.org/std/fmt/struct.Error.html - // This type does not support transmission of an error other than that an error - // occurred. Any extra information must be arranged to be transmitted through - // some other means. - let json_value = json::JsonTypes::try_from_input_value(&self.input_value, &self.abi_type) - .map_err(|_| std::fmt::Error)?; - write!(f, "{}", serde_json::to_string(&json_value).map_err(|_| std::fmt::Error)?) - } -} - /// The different formats that are supported when parsing /// the initial witness values #[cfg_attr(test, derive(strum_macros::EnumIter))] diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index b68a03f270d..76ecba9bff2 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -12,6 +12,7 @@ use acvm::{ use errors::AbiError; use input_parser::InputValue; use iter_extended::{try_btree_map, try_vecmap, vecmap}; +use noirc_frontend::{Signedness, Type, TypeBinding, TypeVariableKind, Visibility}; use serde::{Deserialize, Serialize}; // This is the ABI used to bridge the different TOML formats for the initial // witness, the partial witness generator and the interpreter. @@ -74,6 +75,24 @@ pub enum AbiVisibility { Private, } +impl From for AbiVisibility { + fn from(value: Visibility) -> Self { + match value { + Visibility::Public => AbiVisibility::Public, + Visibility::Private => AbiVisibility::Private, + } + } +} + +impl From<&Visibility> for AbiVisibility { + fn from(value: &Visibility) -> Self { + match value { + Visibility::Public => AbiVisibility::Public, + Visibility::Private => AbiVisibility::Private, + } + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] /// Represents whether the return value should compromise of unique witness indices such that no @@ -97,6 +116,60 @@ pub enum Sign { } impl AbiType { + // TODO: Add `Context` argument for resolving fully qualified struct paths + pub fn from_type(typ: &Type) -> Self { + // Note; use strict_eq instead of partial_eq when comparing field types + // in this method, you most likely want to distinguish between public and private + match typ { + Type::FieldElement => Self::Field, + Type::Array(size, typ) => { + let length = size + .evaluate_to_u64() + .expect("Cannot have variable sized arrays as a parameter to main"); + let typ = typ.as_ref(); + Self::Array { length, typ: Box::new(Self::from_type(typ)) } + } + Type::Integer(sign, bit_width) => { + let sign = match sign { + Signedness::Unsigned => Sign::Unsigned, + Signedness::Signed => Sign::Signed, + }; + + Self::Integer { sign, width: *bit_width } + } + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { + match &*binding.borrow() { + TypeBinding::Bound(typ) => Self::from_type(typ), + TypeBinding::Unbound(_) => Self::from_type(&Type::default_int_type()), + } + } + Type::Bool => Self::Boolean, + Type::String(size) => { + let size = size + .evaluate_to_u64() + .expect("Cannot have variable sized strings as a parameter to main"); + Self::String { length: size } + } + Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"), + Type::Error => unreachable!(), + Type::Unit => unreachable!(), + Type::Constant(_) => unreachable!(), + Type::Struct(def, ref args) => { + let struct_type = def.borrow(); + let fields = struct_type.get_fields(args); + let fields = vecmap(fields, |(name, typ)| (name, Self::from_type(&typ))); + Self::Struct { fields, name: struct_type.name.to_string() } + } + Type::Tuple(_) => todo!("AbiType::from_type not yet implemented for tuple types"), + Type::TypeVariable(_, _) => unreachable!(), + Type::NamedGeneric(..) => unreachable!(), + Type::Forall(..) => unreachable!(), + Type::Function(_, _, _) => unreachable!(), + Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"), + Type::NotConstant => unreachable!(), + } + } + /// Returns the number of field elements required to represent the type once encoded. pub fn field_count(&self) -> u32 { match self { @@ -390,7 +463,7 @@ fn decode_value( Ok(value) } -pub fn decode_string_value(field_elements: &[FieldElement]) -> String { +fn decode_string_value(field_elements: &[FieldElement]) -> 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 diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index c0966afd454..3511fbeabb6 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -142,7 +142,7 @@ pub fn compute_function_abi( let (parameters, return_type) = func_meta.into_function_signature(); let parameters = into_abi_params(parameters, &context.def_interner); - let return_type = return_type.map(|typ| typ.as_abi_type()); + let return_type = return_type.map(|typ| AbiType::from_type(&typ)); Some((parameters, return_type)) } diff --git a/crates/noirc_evaluator/src/ssa/abi_gen/mod.rs b/crates/noirc_evaluator/src/ssa/abi_gen/mod.rs index cfefb3c9f73..f2c61715e37 100644 --- a/crates/noirc_evaluator/src/ssa/abi_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa/abi_gen/mod.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use acvm::acir::native_types::Witness; use iter_extended::{btree_map, vecmap}; -use noirc_abi::{Abi, AbiParameter}; +use noirc_abi::{Abi, AbiParameter, AbiType}; use noirc_frontend::{ hir_def::{ function::{FunctionSignature, Param}, @@ -27,7 +27,7 @@ pub fn into_abi_params(params: Vec, interner: &NodeInterner) -> Vec Abi { let (parameters, return_type) = func_sig; let parameters = into_abi_params(parameters, interner); - let return_type = return_type.map(|typ| typ.as_abi_type()); + let return_type = return_type.map(|typ| AbiType::from_type(&typ)); let param_witnesses = param_witnesses_from_abi_param(¶meters, input_witnesses); Abi { parameters, return_type, param_witnesses, return_witnesses } } diff --git a/crates/noirc_frontend/Cargo.toml b/crates/noirc_frontend/Cargo.toml index c2d6e579c2c..636f2e74b2a 100644 --- a/crates/noirc_frontend/Cargo.toml +++ b/crates/noirc_frontend/Cargo.toml @@ -8,8 +8,8 @@ edition.workspace = true [dependencies] acvm.workspace = true -noirc_abi.workspace = true noirc_errors.workspace = true +noirc_printable_type.workspace = true fm.workspace = true arena.workspace = true iter-extended.workspace = true diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index 19e0d5d2e62..0a4e69aa55f 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -14,7 +14,6 @@ mod type_alias; pub use expression::*; pub use function::*; -use noirc_abi::AbiVisibility; use noirc_errors::Span; pub use statement::*; pub use structure::*; @@ -248,26 +247,6 @@ impl std::fmt::Display for Visibility { } } -// TODO: Move this into noirc_abi when it depends upon noirc_frontend (instead of other way around) -impl From for AbiVisibility { - fn from(value: Visibility) -> Self { - match value { - Visibility::Public => AbiVisibility::Public, - Visibility::Private => AbiVisibility::Private, - } - } -} - -// TODO: Move this into noirc_abi when it depends upon noirc_frontend (instead of other way around) -impl From<&Visibility> for AbiVisibility { - fn from(value: &Visibility) -> Self { - match value { - Visibility::Public => AbiVisibility::Public, - Visibility::Private => AbiVisibility::Private, - } - } -} - #[derive(Clone, Copy, Debug, PartialEq, Eq)] /// Represents whether the return value should compromise of unique witness indices such that no /// index occurs within the program's abi more than once. diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index 1f2b9e8be74..7988c20d244 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -9,8 +9,8 @@ use crate::{ node_interner::{ExprId, NodeInterner, TypeAliasId}, }; use iter_extended::vecmap; -use noirc_abi::AbiType; use noirc_errors::Span; +use noirc_printable_type::PrintableType; use crate::{node_interner::StructId, node_interner::TraitId, Ident, Signedness}; @@ -1012,58 +1012,6 @@ impl Type { } } - // Note; use strict_eq instead of partial_eq when comparing field types - // in this method, you most likely want to distinguish between public and private - pub fn as_abi_type(&self) -> AbiType { - match self { - Type::FieldElement => AbiType::Field, - Type::Array(size, typ) => { - let length = size - .evaluate_to_u64() - .expect("Cannot have variable sized arrays as a parameter to main"); - AbiType::Array { length, typ: Box::new(typ.as_abi_type()) } - } - Type::Integer(sign, bit_width) => { - let sign = match sign { - Signedness::Unsigned => noirc_abi::Sign::Unsigned, - Signedness::Signed => noirc_abi::Sign::Signed, - }; - - AbiType::Integer { sign, width: *bit_width } - } - Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { - match &*binding.borrow() { - TypeBinding::Bound(typ) => typ.as_abi_type(), - TypeBinding::Unbound(_) => Type::default_int_type().as_abi_type(), - } - } - Type::Bool => AbiType::Boolean, - Type::String(size) => { - let size = size - .evaluate_to_u64() - .expect("Cannot have variable sized strings as a parameter to main"); - AbiType::String { length: size } - } - Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"), - Type::Error => unreachable!(), - Type::Unit => unreachable!(), - Type::Constant(_) => unreachable!(), - Type::Struct(def, args) => { - let struct_type = def.borrow(); - let fields = struct_type.get_fields(args); - let fields = vecmap(fields, |(name, typ)| (name, typ.as_abi_type())); - AbiType::Struct { fields, name: struct_type.name.to_string() } - } - Type::Tuple(_) => todo!("as_abi_type not yet implemented for tuple types"), - Type::TypeVariable(_, _) => unreachable!(), - Type::NamedGeneric(..) => unreachable!(), - Type::Forall(..) => unreachable!(), - Type::Function(_, _, _) => unreachable!(), - Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"), - Type::NotConstant => unreachable!(), - } - } - /// Iterate over the fields of this type. /// Panics if the type is not a struct or tuple. pub fn iter_fields(&self) -> impl Iterator { @@ -1334,3 +1282,56 @@ impl TypeVariableKind { } } } + +impl From for PrintableType { + fn from(value: Type) -> Self { + Self::from(&value) + } +} + +impl From<&Type> for PrintableType { + fn from(value: &Type) -> Self { + // Note; use strict_eq instead of partial_eq when comparing field types + // in this method, you most likely want to distinguish between public and private + match value { + Type::FieldElement => PrintableType::Field, + Type::Array(size, typ) => { + let length = size.evaluate_to_u64().expect("Cannot print variable sized arrays"); + let typ = typ.as_ref(); + PrintableType::Array { length, typ: Box::new(typ.into()) } + } + Type::Integer(sign, bit_width) => match sign { + Signedness::Unsigned => PrintableType::UnsignedInteger { width: *bit_width }, + Signedness::Signed => PrintableType::SignedInteger { width: *bit_width }, + }, + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { + match &*binding.borrow() { + TypeBinding::Bound(typ) => typ.into(), + TypeBinding::Unbound(_) => Type::default_int_type().into(), + } + } + Type::Bool => PrintableType::Boolean, + Type::String(size) => { + let size = size.evaluate_to_u64().expect("Cannot print variable sized strings"); + PrintableType::String { length: size } + } + Type::FmtString(_, _) => unreachable!("format strings cannot be printed"), + Type::Error => unreachable!(), + Type::Unit => unreachable!(), + Type::Constant(_) => unreachable!(), + Type::Struct(def, ref args) => { + let struct_type = def.borrow(); + let fields = struct_type.get_fields(args); + let fields = vecmap(fields, |(name, typ)| (name, typ.into())); + PrintableType::Struct { fields, name: struct_type.name.to_string() } + } + Type::Tuple(_) => todo!("printing tuple types is not yet implemented"), + Type::TypeVariable(_, _) => unreachable!(), + Type::NamedGeneric(..) => unreachable!(), + Type::Forall(..) => unreachable!(), + Type::Function(_, _, _) => unreachable!(), + Type::MutableReference(_) => unreachable!("cannot print &mut"), + Type::NotConstant => unreachable!(), + } + } +} diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index 0d3297bf8a4..2ef980176d3 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -10,6 +10,7 @@ //! function, will monomorphize the entire reachable program. use acvm::FieldElement; use iter_extended::{btree_map, vecmap}; +use noirc_printable_type::PrintableType; use std::collections::{BTreeMap, HashMap, VecDeque}; use crate::{ @@ -775,7 +776,7 @@ impl<'interner> Monomorphizer<'interner> { if name.as_str() == "println" { // Oracle calls are required to be wrapped in an unconstrained function // Thus, the only argument to the `println` oracle is expected to always be an ident - self.append_abi_arg(&hir_arguments[0], &mut arguments); + self.append_printable_type_info(&hir_arguments[0], &mut arguments); } } } @@ -828,17 +829,16 @@ impl<'interner> Monomorphizer<'interner> { } /// Adds a function argument that contains type metadata that is required to tell - /// a caller (such as nargo) how to convert values passed to an foreign call - /// back to a human-readable string. + /// `println` how to convert values passed to an foreign call back to a human-readable string. /// The values passed to an foreign call will be a simple list of field elements, /// thus requiring extra metadata to correctly decode this list of elements. /// - /// The Noir compiler has an `AbiType` that handles encoding/decoding a list + /// The Noir compiler has a `PrintableType` that handles encoding/decoding a list /// of field elements to/from JSON. The type metadata attached in this method - /// is the serialized `AbiType` for the argument passed to the function. - /// The caller that is running a Noir program should then deserialize the `AbiType`, + /// is the serialized `PrintableType` for the argument passed to the function. + /// The caller that is running a Noir program should then deserialize the `PrintableType`, /// and accurately decode the list of field elements passed to the foreign call. - fn append_abi_arg( + fn append_printable_type_info( &mut self, hir_argument: &HirExpression, arguments: &mut Vec, @@ -854,7 +854,7 @@ impl<'interner> Monomorphizer<'interner> { match *elements { Type::Tuple(element_types) => { for typ in element_types { - Self::append_abi_arg_inner(&typ, arguments); + Self::append_printable_type_info_inner(&typ, arguments); } } _ => unreachable!( @@ -864,7 +864,7 @@ impl<'interner> Monomorphizer<'interner> { true } _ => { - Self::append_abi_arg_inner(&typ, arguments); + Self::append_printable_type_info_inner(&typ, arguments); false } }; @@ -875,15 +875,15 @@ impl<'interner> Monomorphizer<'interner> { } } - fn append_abi_arg_inner(typ: &Type, arguments: &mut Vec) { + fn append_printable_type_info_inner(typ: &Type, arguments: &mut Vec) { if let HirType::Array(size, _) = typ { if let HirType::NotConstant = **size { unreachable!("println does not support slices. Convert the slice to an array before passing it to println"); } } - let abi_type = typ.as_abi_type(); + let printable_type: PrintableType = typ.into(); let abi_as_string = - serde_json::to_string(&abi_type).expect("ICE: expected Abi type to serialize"); + serde_json::to_string(&printable_type).expect("ICE: expected PrintableType to serialize"); arguments.push(ast::Expression::Literal(ast::Literal::Str(abi_as_string))); } diff --git a/crates/noirc_printable_type/Cargo.toml b/crates/noirc_printable_type/Cargo.toml new file mode 100644 index 00000000000..b3de1a63dec --- /dev/null +++ b/crates/noirc_printable_type/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "noirc_printable_type" +version.workspace = true +authors.workspace = true +edition.workspace = true + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +acvm.workspace = true +iter-extended.workspace = true +regex = "1.9.1" +serde.workspace = true +serde_json.workspace = true +thiserror.workspace = true + +[dev-dependencies] diff --git a/crates/noirc_printable_type/src/lib.rs b/crates/noirc_printable_type/src/lib.rs new file mode 100644 index 00000000000..46f59f665a0 --- /dev/null +++ b/crates/noirc_printable_type/src/lib.rs @@ -0,0 +1,321 @@ +use std::{collections::BTreeMap, str}; + +use acvm::{brillig_vm::brillig::Value, FieldElement}; +use iter_extended::vecmap; +use regex::{Captures, Regex}; +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "lowercase")] +pub enum PrintableType { + Field, + Array { + length: u64, + #[serde(rename = "type")] + typ: Box, + }, + SignedInteger { + width: u32, + }, + UnsignedInteger { + width: u32, + }, + Boolean, + Struct { + name: String, + fields: Vec<(String, PrintableType)>, + }, + String { + length: u64, + }, +} + +impl PrintableType { + /// Returns the number of field elements required to represent the type once encoded. + fn field_count(&self) -> u32 { + match self { + Self::Field + | Self::SignedInteger { .. } + | Self::UnsignedInteger { .. } + | Self::Boolean => 1, + Self::Array { length, typ } => typ.field_count() * (*length as u32), + Self::Struct { fields, .. } => { + fields.iter().fold(0, |acc, (_, field_type)| acc + field_type.field_count()) + } + Self::String { length } => *length as u32, + } + } +} + +/// This is what all formats eventually transform into +/// For example, a toml file will parse into TomlTypes +/// and those TomlTypes will be mapped to Value +#[derive(Debug, Clone, Serialize, PartialEq)] +pub enum PrintableValue { + Field(FieldElement), + String(String), + Vec(Vec), + Struct(BTreeMap), +} + +/// In order to display a `PrintableValue` we need a `PrintableType` to accurately +/// convert the value into a human-readable format. +pub enum PrintableValueDisplay { + Plain(PrintableValue, PrintableType), + FmtString(String, Vec<(PrintableValue, PrintableType)>), +} + +#[derive(Debug, Error)] +pub enum ForeignCallError { + #[error("Foreign call inputs needed for execution are missing")] + MissingForeignCallInputs, + + #[error("Could not parse PrintableType argument. {0}")] + ParsingError(#[from] serde_json::Error), +} + +impl TryFrom<&[Vec]> for PrintableValueDisplay { + type Error = ForeignCallError; + + fn try_from(foreign_call_inputs: &[Vec]) -> Result { + let (is_fmt_str, foreign_call_inputs) = + foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; + + if is_fmt_str[0].to_field().is_one() { + convert_fmt_string_inputs(foreign_call_inputs) + } else { + convert_string_inputs(foreign_call_inputs) + } + } +} + +fn convert_string_inputs( + foreign_call_inputs: &[Vec], +) -> Result { + // 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(|values| vecmap(values, |value| value.to_field())); + + 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: &[Vec], +) -> Result { + let (message_as_values, input_and_printable_values) = + foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; + + let message_as_fields = vecmap(message_as_values, |value| value.to_field()); + let message_as_string = decode_string_value(&message_as_fields); + + let (num_values, input_and_printable_values) = input_and_printable_values + .split_first() + .ok_or(ForeignCallError::MissingForeignCallInputs)?; + + let mut output = Vec::new(); + let num_values = num_values[0].to_field().to_u128() as usize; + + for (i, printable_value) in input_and_printable_values + .iter() + .skip(input_and_printable_values.len() - num_values) + .enumerate() + { + let printable_type = fetch_printable_type(printable_value)?; + let type_size = printable_type.field_count() as usize; + + let mut input_values_as_fields = input_and_printable_values[i..(i + type_size)] + .iter() + .flat_map(|values| vecmap(values, |value| value.to_field())); + + let value = decode_value(&mut input_values_as_fields, &printable_type); + + output.push((value, printable_type)); + } + + Ok(PrintableValueDisplay::FmtString(message_as_string, output)) +} + +fn fetch_printable_type( + printable_type_as_values: &[Value], +) -> Result { + let printable_type_as_fields = vecmap(printable_type_as_values, |value| value.to_field()); + 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) { + ( + PrintableValue::Field(f), + PrintableType::Field + // TODO(#2401): We should print the sign for these and probably print normal integers instead of field strings + | PrintableType::SignedInteger { .. } + | PrintableType::UnsignedInteger { .. }, + ) => { + output.push_str(&format_field_string(*f)); + } + (PrintableValue::Field(f), PrintableType::Boolean) => { + if f.is_one() { + output.push_str("true"); + } else { + output.push_str("false"); + } + } + (PrintableValue::Vec(vector), PrintableType::Array { typ, .. }) => { + output.push('['); + let mut values = vector.iter().peekable(); + while let Some(value) = values.next() { + output.push_str(&format!("{}", PrintableValueDisplay::Plain(value.clone(), *typ.clone()))); + if values.peek().is_some() { + output.push_str(", "); + } + } + output.push(']'); + } + + (PrintableValue::String(s), PrintableType::String { .. }) => { + output.push_str(&format!(r#""{s}""#)); + } + + (PrintableValue::Struct(map), PrintableType::Struct { name, fields, .. }) => { + output.push_str(&format!("{name} {{ ")); + + let mut fields = fields.iter().peekable(); + while let Some((key, field_type)) = fields.next() { + let value = &map[key]; + output.push_str(&format!("{key}: {}", PrintableValueDisplay::Plain(value.clone(), field_type.clone()))); + if fields.peek().is_some() { + output.push_str(", "); + } + } + + output.push_str(" }"); + } + + _ => return None + }; + + 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(); + } + new.push_str(&haystack[last_match..]); + Ok(new) +} + +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)?; + + 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}") + } + } + } +} + +/// This trims any leading zeroes. +/// A singular '0' will be prepended as well if the trimmed string has an odd length. +/// A hex string's length needs to be even to decode into bytes, as two digits correspond to +/// one byte. +fn format_field_string(field: FieldElement) -> String { + if field.is_zero() { + return "0x00".to_owned(); + } + let mut trimmed_field = field.to_hex().trim_start_matches('0').to_owned(); + if trimmed_field.len() % 2 != 0 { + trimmed_field = "0".to_owned() + &trimmed_field; + } + "0x".to_owned() + &trimmed_field +} + +/// Assumes that `field_iterator` contains enough [FieldElement] in order to decode the [PrintableType] +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) + } + 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) + } + } +} + +fn decode_string_value(field_elements: &[FieldElement]) -> 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() +}