From d8767b364f4db9a52c823e7f39f36feac3c90fcd Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 21 Oct 2024 10:27:58 -0400 Subject: [PATCH 1/3] feat(interpreter): Comptime derive generators (#6303) # Description ## Problem\* Request made by the aztec team for their macros work. ## Summary\* This PR adds handling for the `derive_pedersen_generators` compiler builtin. ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** 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: jfecher --- .../noirc_frontend/src/hir/comptime/errors.rs | 11 +++- .../src/hir/comptime/interpreter/builtin.rs | 56 ++++++++++++++++++ .../comptime_derive_generators/Nargo.toml | 7 +++ .../comptime_derive_generators/src/main.nr | 58 +++++++++++++++++++ 4 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 test_programs/compile_success_empty/comptime_derive_generators/Nargo.toml create mode 100644 test_programs/compile_success_empty/comptime_derive_generators/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 5217bbd1e71..dfd328f85f0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -226,6 +226,10 @@ pub enum InterpreterError { location: Location, expression: String, }, + UnknownArrayLength { + length: Type, + location: Location, + }, // These cases are not errors, they are just used to prevent us from running more code // until the loop can be resumed properly. These cases will never be displayed to users. @@ -299,7 +303,8 @@ impl InterpreterError { | InterpreterError::DuplicateGeneric { duplicate_location: location, .. } | InterpreterError::TypeAnnotationsNeededForMethodCall { location } | InterpreterError::CannotResolveExpression { location, .. } - | InterpreterError::CannotSetFunctionBody { location, .. } => *location, + | InterpreterError::CannotSetFunctionBody { location, .. } + | InterpreterError::UnknownArrayLength { location, .. } => *location, InterpreterError::FailedToParseMacro { error, file, .. } => { Location::new(error.span(), *file) @@ -635,6 +640,10 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { let msg = format!("`{expression}` is not a valid function body"); CustomDiagnostic::simple_error(msg, String::new(), location.span) } + InterpreterError::UnknownArrayLength { length, location } => { + let msg = format!("Could not determine array length `{length}`"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index aca0a6dd6ca..1011d71be16 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -63,6 +63,9 @@ impl<'local, 'context> Interpreter<'local, 'context> { "as_slice" => as_slice(interner, arguments, location), "ctstring_eq" => ctstring_eq(arguments, location), "ctstring_hash" => ctstring_hash(arguments, location), + "derive_pedersen_generators" => { + derive_generators(interner, arguments, return_type, location) + } "expr_as_array" => expr_as_array(interner, arguments, return_type, location), "expr_as_assert" => expr_as_assert(interner, arguments, return_type, location), "expr_as_assert_eq" => expr_as_assert_eq(interner, arguments, return_type, location), @@ -2770,3 +2773,56 @@ fn ctstring_eq(arguments: Vec<(Value, Location)>, location: Location) -> IResult fn ctstring_hash(arguments: Vec<(Value, Location)>, location: Location) -> IResult { hash_item(arguments, location, get_ctstring) } + +fn derive_generators( + interner: &mut NodeInterner, + arguments: Vec<(Value, Location)>, + return_type: Type, + location: Location, +) -> IResult { + let (domain_separator_string, starting_index) = check_two_arguments(arguments, location)?; + + let domain_separator_location = domain_separator_string.1; + let (domain_separator_string, _) = get_array(interner, domain_separator_string)?; + let starting_index = get_u32(starting_index)?; + + let domain_separator_string = + try_vecmap(domain_separator_string, |byte| get_u8((byte, domain_separator_location)))?; + + let (size, elements) = match return_type.clone() { + Type::Array(size, elements) => (size, elements), + _ => panic!("ICE: Should only have an array return type"), + }; + + let Some(num_generators) = size.evaluate_to_u32() else { + return Err(InterpreterError::UnknownArrayLength { length: *size, location }); + }; + + let generators = bn254_blackbox_solver::derive_generators( + &domain_separator_string, + num_generators, + starting_index, + ); + + let is_infinite = FieldElement::zero(); + let x_field_name: Rc = Rc::new("x".to_owned()); + let y_field_name: Rc = Rc::new("y".to_owned()); + let is_infinite_field_name: Rc = Rc::new("is_infinite".to_owned()); + let mut results = Vector::new(); + for gen in generators { + let x_big: BigUint = gen.x.into(); + let x = FieldElement::from_be_bytes_reduce(&x_big.to_bytes_be()); + let y_big: BigUint = gen.y.into(); + let y = FieldElement::from_be_bytes_reduce(&y_big.to_bytes_be()); + let mut embedded_curve_point_fields = HashMap::default(); + embedded_curve_point_fields.insert(x_field_name.clone(), Value::Field(x)); + embedded_curve_point_fields.insert(y_field_name.clone(), Value::Field(y)); + embedded_curve_point_fields + .insert(is_infinite_field_name.clone(), Value::Field(is_infinite)); + let embedded_curve_point_struct = + Value::Struct(embedded_curve_point_fields, *elements.clone()); + results.push_back(embedded_curve_point_struct); + } + + Ok(Value::Array(results, return_type)) +} diff --git a/test_programs/compile_success_empty/comptime_derive_generators/Nargo.toml b/test_programs/compile_success_empty/comptime_derive_generators/Nargo.toml new file mode 100644 index 00000000000..0bf7ca9d0f2 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_derive_generators/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_derive_generators" +type = "bin" +authors = [""] +compiler_version = ">=0.35.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/comptime_derive_generators/src/main.nr b/test_programs/compile_success_empty/comptime_derive_generators/src/main.nr new file mode 100644 index 00000000000..c42fac8d187 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_derive_generators/src/main.nr @@ -0,0 +1,58 @@ +use std::embedded_curve_ops::EmbeddedCurvePoint; + +fn main() { + comptime + { + // Result computed from executing `derive_generators` with non-comptime Noir + let result = [ + EmbeddedCurvePoint { + x: 0x0224a8abc6c8b8d50373d64cd2a1ab1567bf372b3b1f7b861d7f01257052d383, + y: 0x2358629b90eafb299d6650a311e79914b0215eb0a790810b26da5a826726d711, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x0f106f6d46bc904a5290542490b2f238775ff3c445b2f8f704c466655f460a2a, + y: 0x29ab84d472f1d33f42fe09c47b8f7710f01920d6155250126731e486877bcf27, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x0298f2e42249f0519c8a8abd91567ebe016e480f219b8c19461d6a595cc33696, + y: 0x035bec4b8520a4ece27bd5aafabee3dfe1390d7439c419a8c55aceb207aac83b, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x2c9628479de4181ea77e7b0913ccf41d2a74155b1d9c82eaa220c218781f6f3b, + y: 0x278f86b8fd95520b5da23bee1a5e354dc5dcb0cb43d6b76e628ddbffb101d776, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x0be1916f382e3532aa53a766fe74b1a983784caab90290aea7bf616bc371fb41, + y: 0x0f65545005e896f14249956344faf9addd762b7573a487b58f805a361d920a20, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x29ff8437ae5bec89981441b23036a22b7fd5bee9eff0e83c0dd5b87bfb5bd60e, + y: 0x1fd247352b77e2676b22db23cf7cd482474f543e3480b5a39c42f839a306be10, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x2f3bd4e98f8c8458cd58888749f0f5e582a43565767398e08e50e94b9b19a4d9, + y: 0x1f534906d1aa8b4ba74ad9e3f85ae3f8295e51eaafd15b5d116801b96360205b, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x27759098f425b76447c2c52728576803a1ac5de37bba875ac47cdcff539ab931, + y: 0x0aa47ee64d12d856cfb81b595c1d60ceecb693f0fdae644746ff333e39f61db7, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x015ca8d68616fde86c9108e3db04f588e0f308e60d367e963b7d460fe9a65e6c, + y: 0x2cf918009dda942ac9d59903cd2d0294d8738f938b1394170d892a027d0f347b, + is_infinite: false + }, EmbeddedCurvePoint { + x: 0x0d1783d5b256765515f3c9988df9f1ba7e6f5fb0248c8971fbc503ffd5187714, + y: 0x2ebb434ff4857fc3621f3bc3c6b8002b17d02d9c204e75f19b8f0b99ea68402c, + is_infinite: false + } + ]; + + let generators: [EmbeddedCurvePoint; 10] = std::hash::derive_generators("DEFAULT_DOMAIN_SEPARATOR".as_bytes(), 5); + + for i in 0..10 { + assert(generators[i].x == result[i].x); + assert(generators[i].y == result[i].y); + } + } +} From 33a1e7d2246bdea48dd6fe925d427c7be8c4659d Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 21 Oct 2024 16:45:57 +0100 Subject: [PATCH 2/3] fix: Display function name and body when inlining recursion limit hit (#6291) # Description ## Problem\* Resolves #4828 ## Summary\* Given a program like this: ``` fn main() { main(); } ``` Changed this panic message: ``` Attempted to recur more than 1000 times during function inlining. ``` Into this: ``` Attempted to recur more than 1000 times during inlining function 'main': acir(inline) fn main f0 { b0(): call f0() return } ``` I included the ACIR to help figure out which function is the culprit, in case the name is not enough. ## Additional Context I added a test to show that there is no compilation error or warning for the example program. It would be nice to issue a warning about unconditional recursion like Rust does. I'll try to do that in a followup PR. ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../noirc_evaluator/src/ssa/opt/inlining.rs | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 7843c55da65..0c11a8bee9a 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -291,13 +291,14 @@ impl InlineContext { ) -> Vec { self.recursion_level += 1; + let source_function = &ssa.functions[&id]; + if self.recursion_level > RECURSION_LIMIT { panic!( - "Attempted to recur more than {RECURSION_LIMIT} times during function inlining." + "Attempted to recur more than {RECURSION_LIMIT} times during inlining function '{}': {}", source_function.name(), source_function ); } - let source_function = &ssa.functions[&id]; let mut context = PerFunctionContext::new(self, source_function); let parameters = source_function.parameters(); @@ -967,4 +968,28 @@ mod test { let main = ssa.main(); assert_eq!(main.reachable_blocks().len(), 4); } + + #[test] + #[should_panic( + expected = "Attempted to recur more than 1000 times during inlining function 'main': acir(inline) fn main f0 {" + )] + fn unconditional_recursion() { + // fn main f1 { + // b0(): + // call f1() + // return + // } + let main_id = Id::test_new(0); + let mut builder = FunctionBuilder::new("main".into(), main_id); + + let main = builder.import_function(main_id); + let results = builder.insert_call(main, Vec::new(), vec![]).to_vec(); + builder.terminate_with_return(results); + + let ssa = builder.finish(); + assert_eq!(ssa.functions.len(), 1); + + let inlined = ssa.inline_functions(); + assert_eq!(inlined.functions.len(), 0); + } } From 51ae1b324cd73fdb4fe3695b5d483a44b4aff4a9 Mon Sep 17 00:00:00 2001 From: jfecher Date: Mon, 21 Oct 2024 14:19:38 -0500 Subject: [PATCH 3/3] fix: Allow array map on empty arrays (#6305) # Description ## Problem\* Resolves a private issue sent to me on slack ## Summary\* Array map is a fairly old method written before we had `std::mem::zeroed`. Now that we have zeroed, we can allow mapping empty arrays since we can use `zeroed` to get the filler `U` value for the starting elements. ## Additional Context While I was at it I included a small clarification to `reduce`'s docs that it requires a non-empty array ## Documentation\* Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** 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. --- docs/docs/noir/concepts/data_types/arrays.md | 2 ++ noir_stdlib/src/array/mod.nr | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/docs/docs/noir/concepts/data_types/arrays.md b/docs/docs/noir/concepts/data_types/arrays.md index bb68e60fe53..289145a8c4d 100644 --- a/docs/docs/noir/concepts/data_types/arrays.md +++ b/docs/docs/noir/concepts/data_types/arrays.md @@ -203,6 +203,8 @@ fn main() { Same as fold, but uses the first element as the starting element. +Requires `self` to be non-empty. + ```rust fn reduce(self, f: fn(T, T) -> T) -> T ``` diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index 46acf619dd2..f5089de1877 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -43,10 +43,10 @@ impl [T; N] { /// assert_eq(b, [2, 4, 6]); /// ``` pub fn map(self, f: fn[Env](T) -> U) -> [U; N] { - let first_elem = f(self[0]); - let mut ret = [first_elem; N]; + let uninitialized = crate::mem::zeroed(); + let mut ret = [uninitialized; N]; - for i in 1..self.len() { + for i in 0..self.len() { ret[i] = f(self[i]); } @@ -79,6 +79,8 @@ impl [T; N] { } /// Same as fold, but uses the first element as the starting element. + /// + /// Requires the input array to be non-empty. /// /// Example: /// @@ -217,3 +219,10 @@ impl From> for [u8; N] { s.as_bytes() } } + +mod test { + #[test] + fn map_empty() { + assert_eq([].map(|x| x + 1), []); + } +}