Skip to content

Commit

Permalink
feat: Format strings for prints (#1952)
Browse files Browse the repository at this point in the history
* initial stdlib methods to start refactoring logign

* foreign call enum

* working println and println_format w/ brillig oracles

* fix up brillig_oracle test

* uncomment regression test for slice return from foreign calls in brillig

* cargo clippy

* got structs serialized correctly without aos_to_soa

* remove dbg

* working println_format

* cargo clippy

* rename enable_slices to experimental_ssa

* remove dbg and fix format_field_string

* initial work towards FmtStr literal

* working format strins with one unified println method, still have some cleanup to-do, use Display/Debug for pretty printing

* remove old comment

* moved resolution of string to fmt string only when passing literals to functions

* delete temp intrinsic for println new

* remove unnecessary subtype

* remove debugging code w/ def id as part of mono pass Ident

* cleanup formatting stuff

* cargo clippy

* resolver test for fmt string

* remove TODO comment

* cargo clippy

* pr comments

* expose full fmtstr type to the user

* add back fmt string resolver test

* don't allow comparison of format strings

* use JsonType Display trait

* add issue for printing func params

* remove Token::F variant

* remove old append_abi_arg func

* add comments to append_abi-arg

* fix: format printing function parameters, store exprs rather than idents as part of HirLiteral::FmtStr

* remove ve old comment about not being able to use witness values in fmt strings

* push fix for asfs{x}{x} case and more specific regex for idents

* Update crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* remove is_match check

* breakout literal fmt str case in resolver to its own func

* update resolve_fmt_strings test

* switch to_owned placement in resolve_fmt_str_literal

* Update crates/noirc_frontend/src/ast/mod.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* fix find_numeric_generics_in_type

* add case of fmt string in if statement

* add contains_numeric_typevar cases for string and fmtstring

* add unify and subtype checks and fix resolver fmt string test

* working generic fmtstr types

* separate fmtstr codegen into variables

* Update crates/noirc_frontend/src/parser/parser.rs

* Update crates/noirc_abi/src/input_parser/json.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update crates/noirc_frontend/src/ast/mod.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update crates/noirc_frontend/src/monomorphization/mod.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update crates/noirc_frontend/src/monomorphization/mod.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update crates/noirc_frontend/src/monomorphization/mod.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* Update crates/noirc_frontend/src/parser/parser.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* keep the size of fmtrstr type as mandatory

* print original fmt string in monomorphization printer

* print literal update for fmtstr

* add parens to f-string literal printer

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>
  • Loading branch information
vezenovm and jfecher authored Aug 1, 2023
1 parent 5cb8166 commit 3c82721
Show file tree
Hide file tree
Showing 21 changed files with 414 additions and 42 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ serde.workspace = true
serde_json.workspace = true
thiserror.workspace = true
noirc_errors.workspace = true
base64.workspace = true
base64.workspace = true
regex = "1.9.1"
89 changes: 74 additions & 15 deletions crates/nargo/src/ops/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use acvm::{
};
use iter_extended::vecmap;
use noirc_abi::{decode_string_value, input_parser::InputValueDisplay, AbiType};
use regex::{Captures, Regex};

use crate::errors::ForeignCallError;

Expand Down Expand Up @@ -63,31 +64,89 @@ impl ForeignCall {
}

fn execute_println(foreign_call_inputs: &[Vec<Value>]) -> Result<(), ForeignCallError> {
let (abi_type, input_values) = fetch_abi_type(foreign_call_inputs)?;
let (is_fmt_str, foreign_call_inputs) =
foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?;

// 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 input_value_display =
InputValueDisplay::try_from_fields(&mut input_values_as_fields, abi_type)?;

println!("{input_value_display}");
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}");
Ok(())
}
}

/// Fetch the abi type from the foreign call input
/// The remaining input values should hold the values to be printed
fn fetch_abi_type(
foreign_call_inputs: &[Vec<Value>],
) -> Result<(AbiType, &[Vec<Value>]), ForeignCallError> {
fn convert_string_inputs(foreign_call_inputs: &[Vec<Value>]) -> Result<String, ForeignCallError> {
// 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<Value>],
) -> Result<String, ForeignCallError> {
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<AbiType, ForeignCallError> {
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, input_values))
Ok(abi_type)
}
48 changes: 45 additions & 3 deletions crates/nargo_cli/tests/test_data/debug_logs/src/main.nr
Original file line number Diff line number Diff line change
@@ -1,14 +1,56 @@
use dep::std;

fn main(x : Field, y : pub Field) {
let string = "i: {i}, j: {j}";
std::println(string);

// A `fmtstr` lets you easily perform string interpolation.
let fmt_str: fmtstr<14, (Field, Field)> = f"i: {x}, j: {y}";
let fmt_str = string_identity(fmt_str);
std::println(fmt_str);

let fmt_str_no_type = f"i: {x}, j: {y}";
std::println(fmt_str_no_type);

let fmt_str_generic = string_with_generics(fmt_str_no_type);
std::println(fmt_str_generic);

let s = myStruct { y: x, x: y };
std::println(s);

std::println(f"randomstring{x}{x}");

let fmt_str = string_with_partial_generics(f"i: {x}, s: {s}");
std::println(fmt_str);

std::println("*** println ***");
std::println(x);
std::println([x, y]);

let s = myStruct { y: x, x: y };
let foo = fooStruct { my_struct: s, foo: 15 };
std::println(foo);
std::println(f"s: {s}, foo: {foo}");

std::println(f"x: 0, y: 1");

let s_2 = myStruct { x: 20, y: 30 };
std::println(f"s1: {s}, s2: {s_2}");

let bar = fooStruct { my_struct: s_2, foo: 20 };
std::println(f"foo1: {foo}, foo2: {bar}");

let struct_string = if x != 5 { f"{foo}" } else { f"{bar}" };
std::println(struct_string);
}

fn string_identity(string: fmtstr<14, (Field, Field)>) -> fmtstr<14, (Field, Field)> {
string
}

fn string_with_generics<N, T>(string: fmtstr<N, T>) -> fmtstr<N, T> {
string
}

fn string_with_partial_generics<N, T>(string: fmtstr<N, (Field, T)>) -> fmtstr<N, (Field, T)> {
string
}

struct myStruct {
Expand Down
12 changes: 12 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,15 @@ impl<'a> FunctionContext<'a> {
ast::Type::MutableReference(element) => {
Self::map_type_helper(element, &mut |_| f(Type::Reference))
}
ast::Type::FmtString(len, fields) => {
// A format string is represented by multiple values
// The message string, the number of fields to be formatted, and
// then the encapsulated fields themselves
let final_fmt_str_fields =
vec![ast::Type::String(*len), ast::Type::Field, *fields.clone()];
let fmt_str_tuple = ast::Type::Tuple(final_fmt_str_fields);
Self::map_type_helper(&fmt_str_tuple, f)
}
other => Tree::Leaf(f(Self::convert_non_tuple_type(other))),
}
}
Expand Down Expand Up @@ -204,6 +213,9 @@ impl<'a> FunctionContext<'a> {
ast::Type::Integer(Signedness::Unsigned, bits) => Type::unsigned(*bits),
ast::Type::Bool => Type::unsigned(1),
ast::Type::String(len) => Type::Array(Rc::new(vec![Type::char()]), *len as usize),
ast::Type::FmtString(_, _) => {
panic!("convert_non_tuple_type called on a fmt string: {typ}")
}
ast::Type::Unit => panic!("convert_non_tuple_type called on a unit type"),
ast::Type::Tuple(_) => panic!("convert_non_tuple_type called on a tuple: {typ}"),
ast::Type::Function(_, _) => Type::Function,
Expand Down
12 changes: 12 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,18 @@ impl<'a> FunctionContext<'a> {
let typ = Self::convert_non_tuple_type(&ast::Type::String(elements.len() as u64));
self.codegen_array(elements, typ)
}
ast::Literal::FmtStr(string, number_of_fields, fields) => {
// A caller needs multiple pieces of information to make use of a format string
// The message string, the number of fields to be formatted, and the fields themselves
let string = Expression::Literal(ast::Literal::Str(string.clone()));
let number_of_fields = Expression::Literal(ast::Literal::Integer(
(*number_of_fields as u128).into(),
ast::Type::Field,
));
let fields = *fields.clone();
let fmt_str_tuple = &[string, number_of_fields, fields];
self.codegen_tuple(fmt_str_tuple)
}
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/noirc_frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ serde.workspace = true
serde_json.workspace = true
rustc-hash = "1.1.0"
small-ord-set = "0.1.3"
regex = "1.9.1"

[dev-dependencies]
strum = "0.24"
Expand Down
6 changes: 6 additions & 0 deletions crates/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ impl ExpressionKind {
ExpressionKind::Literal(Literal::Str(contents))
}

pub fn format_string(contents: String) -> ExpressionKind {
ExpressionKind::Literal(Literal::FmtStr(contents))
}

pub fn constructor((type_name, fields): (Path, Vec<(Ident, Expression)>)) -> ExpressionKind {
ExpressionKind::Constructor(Box::new(ConstructorExpression { type_name, fields }))
}
Expand Down Expand Up @@ -298,6 +302,7 @@ pub enum Literal {
Bool(bool),
Integer(FieldElement),
Str(String),
FmtStr(String),
Unit,
}

Expand Down Expand Up @@ -473,6 +478,7 @@ impl Display for Literal {
Literal::Bool(boolean) => write!(f, "{}", if *boolean { "true" } else { "false" }),
Literal::Integer(integer) => write!(f, "{}", integer.to_u128()),
Literal::Str(string) => write!(f, "\"{string}\""),
Literal::FmtStr(string) => write!(f, "f\"{string}\""),
Literal::Unit => write!(f, "()"),
}
}
Expand Down
6 changes: 4 additions & 2 deletions crates/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub enum UnresolvedType {
Bool(CompTime),
Expression(UnresolvedTypeExpression),
String(Option<UnresolvedTypeExpression>),
FormatString(UnresolvedTypeExpression, Box<UnresolvedType>),
Unit,

/// A Named UnresolvedType can be a struct type or a type variable
Expand Down Expand Up @@ -102,9 +103,10 @@ impl std::fmt::Display for UnresolvedType {
Expression(expression) => expression.fmt(f),
Bool(is_const) => write!(f, "{is_const}bool"),
String(len) => match len {
None => write!(f, "str[]"),
Some(len) => write!(f, "str[{len}]"),
None => write!(f, "str<_>"),
Some(len) => write!(f, "str<{len}>"),
},
FormatString(len, elements) => write!(f, "fmt<{len}, {elements}"),
Function(args, ret) => {
let args = vecmap(args, ToString::to_string);
write!(f, "fn({}) -> {ret}", args.join(", "))
Expand Down
7 changes: 7 additions & 0 deletions crates/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ pub enum ResolverError {
MutableReferenceToArrayElement { span: Span },
#[error("Function is not defined in a contract yet sets is_internal")]
ContractFunctionInternalInNormalFunction { span: Span },
#[error("Numeric constants should be printed without formatting braces")]
NumericConstantInFormatString { name: String, span: Span },
}

impl ResolverError {
Expand Down Expand Up @@ -283,6 +285,11 @@ impl From<ResolverError> for Diagnostic {
"Non-contract functions cannot be 'internal'".into(),
span,
),
ResolverError::NumericConstantInFormatString { name, span } => Diagnostic::simple_error(
format!("cannot find `{name}` in this scope "),
"Numeric constants should be printed without formatting braces".to_string(),
span,
),
}
}
}
Loading

0 comments on commit 3c82721

Please sign in to comment.