From 4aff219a951f9bd11124139649294691418efe0f Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 20 Jul 2023 20:35:55 +0000 Subject: [PATCH 1/2] use Display impl for InputValue --- crates/nargo/src/ops/foreign_calls.rs | 8 ++--- crates/noirc_abi/src/input_parser/json.rs | 4 +-- crates/noirc_abi/src/input_parser/mod.rs | 35 +++++++++++++++++-- crates/noirc_abi/src/lib.rs | 2 +- crates/noirc_frontend/src/hir/def_map/mod.rs | 8 ++--- .../src/monomorphization/mod.rs | 2 +- 6 files changed, 44 insertions(+), 15 deletions(-) diff --git a/crates/nargo/src/ops/foreign_calls.rs b/crates/nargo/src/ops/foreign_calls.rs index ea7f9be21b..4bbd4eb58b 100644 --- a/crates/nargo/src/ops/foreign_calls.rs +++ b/crates/nargo/src/ops/foreign_calls.rs @@ -3,7 +3,7 @@ use acvm::{ pwg::ForeignCallWaitInfo, }; use iter_extended::vecmap; -use noirc_abi::{decode_string_value, decode_value, input_parser::json::JsonTypes, AbiType}; +use noirc_abi::{decode_string_value, input_parser::InputValueDisplay, AbiType}; use crate::errors::ForeignCallError; @@ -68,11 +68,11 @@ impl ForeignCall { // 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| values.iter().map(|value| value.to_field())); - let decoded_value = decode_value(&mut input_values_as_fields, &abi_type)?; - let json_value = JsonTypes::try_from_input_value(&decoded_value, &abi_type)?; + let input_value_display = + InputValueDisplay::try_from_fields(&mut input_values_as_fields, abi_type)?; - println!("{json_value}"); + println!("{input_value_display}"); Ok(()) } } diff --git a/crates/noirc_abi/src/input_parser/json.rs b/crates/noirc_abi/src/input_parser/json.rs index 7a0cd76698..eb95de9a21 100644 --- a/crates/noirc_abi/src/input_parser/json.rs +++ b/crates/noirc_abi/src/input_parser/json.rs @@ -59,7 +59,7 @@ pub(crate) fn serialize_to_json( #[derive(Debug, Deserialize, Serialize, Clone)] #[serde(untagged)] -pub enum JsonTypes { +pub(crate) enum JsonTypes { // This is most likely going to be a hex string // But it is possible to support UTF-8 String(String), @@ -78,7 +78,7 @@ pub enum JsonTypes { } impl JsonTypes { - pub fn try_from_input_value( + pub(crate) fn try_from_input_value( value: &InputValue, abi_type: &AbiType, ) -> Result { diff --git a/crates/noirc_abi/src/input_parser/mod.rs b/crates/noirc_abi/src/input_parser/mod.rs index 6818f40786..e4adbb3d8c 100644 --- a/crates/noirc_abi/src/input_parser/mod.rs +++ b/crates/noirc_abi/src/input_parser/mod.rs @@ -1,4 +1,4 @@ -pub mod json; +mod json; mod toml; use std::collections::BTreeMap; @@ -6,8 +6,8 @@ use std::collections::BTreeMap; use acvm::FieldElement; use serde::Serialize; -use crate::errors::InputParserError; -use crate::{Abi, AbiType}; +use crate::errors::{AbiError, InputParserError}; +use crate::{decode_value, 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,6 +67,35 @@ 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 86f9edc73b..cb9456af65 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -368,7 +368,7 @@ impl Abi { } } -pub fn decode_value( +pub(crate) fn decode_value( field_iterator: &mut impl Iterator, value_type: &AbiType, ) -> Result { diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index 8bb88abd1e..cc16746dae 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -93,12 +93,12 @@ impl CrateDefMap { .to_str() .expect("expected std path to be convertible to str"); assert_eq!(path_as_str, "std/lib"); - // There are 2 printlns in the stdlib. If we are using the experimental SSA, we want to keep - // only the unconstrained one. Otherwise we want to keep only the constrained one. - ast.functions.retain(|func| { + // There are 2 printlns in the stdlib. If we are using the experimental SSA, we want to keep + // only the unconstrained one. Otherwise we want to keep only the constrained one. + ast.functions.retain(|func| { func.def.name.0.contents.as_str() != "println" || func.def.is_unconstrained == context.def_interner.experimental_ssa - }); + }); if !context.def_interner.experimental_ssa { ast.module_decls.retain(|ident| { diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index a3188eaa14..ed14f79f95 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -778,7 +778,7 @@ impl<'interner> Monomorphizer<'interner> { if let Definition::Oracle(name) = &ident.definition { 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 + // Thus, the only argument to the `println` oracle is expected to always be an ident self.append_abi_arg(&hir_arguments[0], &mut arguments); } } From cfbf7eb565322133c84e72e911bb0eeaebf6938c Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 31 Jul 2023 08:37:37 +0000 Subject: [PATCH 2/2] chore: clean up visibilities --- crates/noirc_abi/src/input_parser/json.rs | 4 ++-- crates/noirc_abi/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/noirc_abi/src/input_parser/json.rs b/crates/noirc_abi/src/input_parser/json.rs index eb95de9a21..6468b48c85 100644 --- a/crates/noirc_abi/src/input_parser/json.rs +++ b/crates/noirc_abi/src/input_parser/json.rs @@ -59,7 +59,7 @@ pub(crate) fn serialize_to_json( #[derive(Debug, Deserialize, Serialize, Clone)] #[serde(untagged)] -pub(crate) enum JsonTypes { +pub(super) enum JsonTypes { // This is most likely going to be a hex string // But it is possible to support UTF-8 String(String), @@ -78,7 +78,7 @@ pub(crate) enum JsonTypes { } impl JsonTypes { - pub(crate) fn try_from_input_value( + pub(super) fn try_from_input_value( value: &InputValue, abi_type: &AbiType, ) -> Result { diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index cb9456af65..5f8c22a665 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -368,7 +368,7 @@ impl Abi { } } -pub(crate) fn decode_value( +fn decode_value( field_iterator: &mut impl Iterator, value_type: &AbiType, ) -> Result {