From f5c4632e174beba508e1e31d0e2ae3f6d028ae2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Thu, 18 Jan 2024 11:53:46 -0500 Subject: [PATCH] feat: Support printing more types (#4071) # Description ## Problem\* As part of our debugger implementation (see #3015) we need to extend the supported types that can be printed. We are using `noirc_printable_type` to keep values of variables at runtime for debug inspection. Also resolves #4073 ## Summary\* This PR adds support for converting these new types: - Tuples - Slices: lift the restriction to print them and handle arrays with dynamic size (flagging them with a `None` length in the type) - Functions: printed as an opaque `<>` tag for now - Mutable references: printed as an opaque `<>` tag for now - Unit: this is actually required to fully support function type conversion, for non-closured function references (ie. the environment is `()` in that case) The PR also fixes a preexisting bug when printing multiple values using formatted strings with the first ones being structs. Since structs are expanded into tuples which take up more than one field, the printing code would fail to skip over all the fields of the struct. ## Additional Context I've been using [this program](https://gist.github.com/ggiraldez/e3709def6c26e7585d12002fc8a0a216) to test this functionality. If it makes sense, I can add it as an integration test to `test_programs/execution_success`. The program produces this output: ``` (0x01, 0x02, 0x03) 0xbbbb # a = (0x01, 0x02, 0x03) # 0xeeee (0x01, 0x02, 0x03) == (0x01, 0x02, 0x03) ((0x01, 0x02, 0x03), 0x04, 0x05, 0x06) 0xbbbb # b = ((0x01, 0x02, 0x03), 0x04, 0x05, 0x06) # 0xeeee ((0x01, 0x02, 0x03), 0x04, 0x05, 0x06) == ((0x01, 0x02, 0x03), 0x04, 0x05, 0x06) <> 0xbbbb # c = <> # 0xeeee <> 0xbbbb # d = <> # 0xeeee <> 0xbbbb # f = <> # 0xeeee [0x01, 0x02, 0x03] 0xbbbb # g = [0x01, 0x02, 0x03] # 0xeeee [0x01, 0x02, 0x03] == [0x01, 0x02, 0x03] Foo { x: 555, y: 666 } == Foo { x: 555, y: 666 } Vec { slice: [0x01, 0x02] } 0xbbbb # h = Vec { slice: [0x01, 0x02] } # 0xeeee [0x01, 0x02] 0xbbbb # j = [0x01, 0x02] # 0xeeee [0x01, 0x02] == [0x01, 0x02] [printable_types] Circuit witness successfully solved ``` ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: synthia --- compiler/noirc_frontend/src/hir_def/types.rs | 16 ++- .../src/monomorphization/mod.rs | 5 + compiler/noirc_printable_type/src/lib.rs | 113 +++++++++++------- .../execution_success/debug_logs/src/main.nr | 20 ++++ 4 files changed, 104 insertions(+), 50 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index d63bec27bdc..cc22a91de78 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1617,7 +1617,7 @@ impl From<&Type> for PrintableType { match value { Type::FieldElement => PrintableType::Field, Type::Array(size, typ) => { - let length = size.evaluate_to_u64().expect("Cannot print variable sized arrays"); + let length = size.evaluate_to_u64(); let typ = typ.as_ref(); PrintableType::Array { length, typ: Box::new(typ.into()) } } @@ -1638,7 +1638,7 @@ impl From<&Type> for PrintableType { } Type::FmtString(_, _) => unreachable!("format strings cannot be printed"), Type::Error => unreachable!(), - Type::Unit => unreachable!(), + Type::Unit => PrintableType::Unit, Type::Constant(_) => unreachable!(), Type::Struct(def, ref args) => { let struct_type = def.borrow(); @@ -1646,13 +1646,17 @@ impl From<&Type> for PrintableType { let fields = vecmap(fields, |(name, typ)| (name, typ.into())); PrintableType::Struct { fields, name: struct_type.name.to_string() } } - Type::TraitAsType(..) => unreachable!(), - Type::Tuple(_) => todo!("printing tuple types is not yet implemented"), + Type::TraitAsType(_, _, _) => unreachable!(), + Type::Tuple(types) => PrintableType::Tuple { types: vecmap(types, |typ| typ.into()) }, Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), - Type::Function(_, _, _) => unreachable!(), - Type::MutableReference(_) => unreachable!("cannot print &mut"), + Type::Function(_, _, env) => { + PrintableType::Function { env: Box::new(env.as_ref().into()) } + } + Type::MutableReference(typ) => { + PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } + } Type::NotConstant => unreachable!(), } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 75a5f1c6361..67b246a02ce 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1029,11 +1029,16 @@ impl<'interner> Monomorphizer<'interner> { } fn append_printable_type_info_inner(typ: &Type, arguments: &mut Vec) { + // Disallow printing slices and mutable references for consistency, + // since they cannot be passed from ACIR into Brillig 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"); } + } else if matches!(typ, HirType::MutableReference(_)) { + unreachable!("println does not support mutable references."); } + let printable_type: PrintableType = typ.into(); let abi_as_string = serde_json::to_string(&printable_type) .expect("ICE: expected PrintableType to serialize"); diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 273e2d512ea..18f2fe0a873 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -11,10 +11,13 @@ use thiserror::Error; pub enum PrintableType { Field, Array { - length: u64, + length: Option, #[serde(rename = "type")] typ: Box, }, + Tuple { + types: Vec, + }, SignedInteger { width: u32, }, @@ -29,23 +32,13 @@ pub enum 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, - } - } + Function { + env: Box, + }, + MutableReference { + typ: Box, + }, + Unit, } /// This is what all formats eventually transform into @@ -114,43 +107,26 @@ fn convert_string_inputs( fn convert_fmt_string_inputs( foreign_call_inputs: &[ForeignCallParam], ) -> Result { - let (message, input_and_printable_values) = + let (message, input_and_printable_types) = foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; let message_as_fields = vecmap(message.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 + 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_value().to_field().to_u128() as usize; - for (i, printable_value) in input_and_printable_values + let types_start_at = input_and_printable_types.len() - num_values; + let mut input_iter = input_and_printable_types[0..types_start_at] .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 value = match printable_type { - PrintableType::Array { .. } | PrintableType::String { .. } => { - // Arrays and strings are represented in a single value vector rather than multiple separate input values - let mut input_values_as_fields = input_and_printable_values[i] - .values() - .into_iter() - .map(|value| value.to_field()); - decode_value(&mut input_values_as_fields, &printable_type) - } - _ => { - // 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_and_printable_values[i..(i + type_size)] - .iter() - .flat_map(|param| vecmap(param.values(), |value| value.to_field())); - decode_value(&mut input_values_as_fields, &printable_type) - } - }; + .flat_map(|param| vecmap(param.values(), |value| value.to_field())); + 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)); } @@ -196,6 +172,12 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { output.push_str("false"); } } + (PrintableValue::Field(_), PrintableType::Function { .. }) => { + output.push_str("<>"); + } + (_, PrintableType::MutableReference { .. }) => { + output.push_str("<>"); + } (PrintableValue::Vec(vector), PrintableType::Array { typ, .. }) => { output.push('['); let mut values = vector.iter().peekable(); @@ -233,6 +215,22 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { output.push_str(" }"); } + (PrintableValue::Vec(values), PrintableType::Tuple { types }) => { + output.push('('); + let mut elems = values.iter().zip(types).peekable(); + while let Some((value, typ)) = elems.next() { + output.push_str( + &PrintableValueDisplay::Plain(value.clone(), typ.clone()).to_string(), + ); + if elems.peek().is_some() { + output.push_str(", "); + } + } + output.push(')'); + } + + (_, PrintableType::Unit) => output.push_str("()"), + _ => return None, }; @@ -308,7 +306,19 @@ fn decode_value( PrintableValue::Field(field_element) } - PrintableType::Array { length, typ } => { + PrintableType::Array { length: None, 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) + } + PrintableType::Array { length: Some(length), typ } => { let length = *length as usize; let mut array_elements = Vec::with_capacity(length); for _ in 0..length { @@ -317,6 +327,9 @@ fn decode_value( PrintableValue::Vec(array_elements) } + PrintableType::Tuple { types } => { + PrintableValue::Vec(vecmap(types, |typ| decode_value(field_iterator, typ))) + } PrintableType::String { length } => { let field_elements: Vec = field_iterator.take(*length as usize).collect(); @@ -333,6 +346,18 @@ fn decode_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(FieldElement::zero()), } } diff --git a/test_programs/execution_success/debug_logs/src/main.nr b/test_programs/execution_success/debug_logs/src/main.nr index 6accdf725d9..52c910065c1 100644 --- a/test_programs/execution_success/debug_logs/src/main.nr +++ b/test_programs/execution_success/debug_logs/src/main.nr @@ -39,7 +39,26 @@ fn main(x: Field, y: pub Field) { let struct_string = if x != 5 { f"{foo}" } else { f"{bar}" }; std::println(struct_string); + let one_tuple = (1, 2, 3); + let another_tuple = (4, 5, 6); + std::println(f"one_tuple: {one_tuple}, another_tuple: {another_tuple}"); + std::println(one_tuple); + + let tuples_nested = (one_tuple, another_tuple); + std::println(f"tuples_nested: {tuples_nested}"); + std::println(tuples_nested); + regression_2906(); + + let free_lambda = |x| x + 1; + let sentinel: u32 = 8888; + std::println(f"free_lambda: {free_lambda}, sentinel: {sentinel}"); + std::println(free_lambda); + + let one = 1; + let closured_lambda = |x| x + one; + std::println(f"closured_lambda: {closured_lambda}, sentinel: {sentinel}"); + std::println(closured_lambda); } fn string_identity(string: fmtstr<14, (Field, Field)>) -> fmtstr<14, (Field, Field)> { @@ -79,3 +98,4 @@ fn regression_2906() { dep::std::println(f"array_five_vals: {array_five_vals}, label_five_vals: {label_five_vals}"); } +