Skip to content

Commit

Permalink
feat: Support printing more types (#4071)
Browse files Browse the repository at this point in the history
# 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 `<<function>>` tag for now
- Mutable references: printed as an opaque `<<mutable ref>>` 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)

<<mutable ref>>
0xbbbb # c = <<mutable ref>> # 0xeeee

<<function>>
0xbbbb # d = <<function>> # 0xeeee

<<function>>
0xbbbb # f = <<function>> # 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 <sy@nthia.dev>
  • Loading branch information
ggiraldez and nthiad committed Jan 18, 2024
1 parent 17e951c commit f5c4632
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 50 deletions.
16 changes: 10 additions & 6 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()) }
}
Expand All @@ -1638,21 +1638,25 @@ 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();
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::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!(),
}
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,11 +1029,16 @@ impl<'interner> Monomorphizer<'interner> {
}

fn append_printable_type_info_inner(typ: &Type, arguments: &mut Vec<ast::Expression>) {
// 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");
Expand Down
113 changes: 69 additions & 44 deletions compiler/noirc_printable_type/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ use thiserror::Error;
pub enum PrintableType {
Field,
Array {
length: u64,
length: Option<u64>,
#[serde(rename = "type")]
typ: Box<PrintableType>,
},
Tuple {
types: Vec<PrintableType>,
},
SignedInteger {
width: u32,
},
Expand All @@ -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<PrintableType>,
},
MutableReference {
typ: Box<PrintableType>,
},
Unit,
}

/// This is what all formats eventually transform into
Expand Down Expand Up @@ -114,43 +107,26 @@ fn convert_string_inputs(
fn convert_fmt_string_inputs(
foreign_call_inputs: &[ForeignCallParam],
) -> Result<PrintableValueDisplay, ForeignCallError> {
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));
}
Expand Down Expand Up @@ -196,6 +172,12 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option<String> {
output.push_str("false");
}
}
(PrintableValue::Field(_), PrintableType::Function { .. }) => {
output.push_str("<<function>>");
}
(_, PrintableType::MutableReference { .. }) => {
output.push_str("<<mutable ref>>");
}
(PrintableValue::Vec(vector), PrintableType::Array { typ, .. }) => {
output.push('[');
let mut values = vector.iter().peekable();
Expand Down Expand Up @@ -233,6 +215,22 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option<String> {
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,
};

Expand Down Expand Up @@ -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 {
Expand All @@ -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<FieldElement> = field_iterator.take(*length as usize).collect();

Expand All @@ -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()),
}
}

Expand Down
20 changes: 20 additions & 0 deletions test_programs/execution_success/debug_logs/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -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)> {
Expand Down Expand Up @@ -79,3 +98,4 @@ fn regression_2906() {

dep::std::println(f"array_five_vals: {array_five_vals}, label_five_vals: {label_five_vals}");
}

0 comments on commit f5c4632

Please sign in to comment.