From d57f075d8319bbace3e89ff13ada3bd985ea4ed9 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 17 Jun 2025 10:46:57 -0300 Subject: [PATCH 1/4] fix: when macro parse error happens, discard warnings --- .../comptime/interpreter/builtin/builtin_helpers.rs | 5 +++-- compiler/noirc_frontend/src/hir/comptime/value.rs | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index 85625135ed5..b151c099c46 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -529,8 +529,9 @@ pub(super) fn parse_tokens<'a, T, F>( where F: FnOnce(&mut Parser<'a>) -> T, { - Parser::for_tokens(quoted).parse_result(parsing_function).map_err(|mut errors| { - let error = Box::new(errors.swap_remove(0)); + Parser::for_tokens(quoted).parse_result(parsing_function).map_err(|errors| { + let error = errors.into_iter().find(|error| !error.is_warning()).unwrap(); + let error = Box::new(error); let tokens = tokens_to_string(&tokens, interner); InterpreterError::FailedToParseMacro { error, tokens, rule, location } }) diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 05b53a783ff..c249843a62f 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -286,8 +286,9 @@ impl Value { Ok(expr) } - Err(mut errors) => { - let error = Box::new(errors.swap_remove(0)); + Err(errors) => { + let error = errors.into_iter().find(|error| !error.is_warning()).unwrap(); + let error = Box::new(error); let rule = "an expression"; let tokens = tokens_to_string(&tokens, elaborator.interner); Err(InterpreterError::FailedToParseMacro { error, tokens, rule, location }) @@ -670,8 +671,9 @@ where } Ok(expr) } - Err(mut errors) => { - let error = Box::new(errors.swap_remove(0)); + Err(errors) => { + let error = errors.into_iter().find(|error| !error.is_warning()).unwrap(); + let error = Box::new(error); let tokens = tokens_to_string(&tokens, elaborator.interner); Err(InterpreterError::FailedToParseMacro { error, tokens, rule, location }) } From 3e82dabf7b775e9a603edd7dc74c529b144cfc21 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 17 Jun 2025 11:13:33 -0300 Subject: [PATCH 2/4] Add a regression test --- .../Nargo.toml | 7 ++ .../src/main.nr | 42 ++++++++++ .../src/other.nr | 3 + .../execute__tests__stderr.snap | 76 +++++++++++++++++++ 4 files changed, 128 insertions(+) create mode 100644 test_programs/compile_failure/comptime_error_on_macro_expansion/Nargo.toml create mode 100644 test_programs/compile_failure/comptime_error_on_macro_expansion/src/main.nr create mode 100644 test_programs/compile_failure/comptime_error_on_macro_expansion/src/other.nr create mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/comptime_error_on_macro_expansion/execute__tests__stderr.snap diff --git a/test_programs/compile_failure/comptime_error_on_macro_expansion/Nargo.toml b/test_programs/compile_failure/comptime_error_on_macro_expansion/Nargo.toml new file mode 100644 index 00000000000..18c6da57012 --- /dev/null +++ b/test_programs/compile_failure/comptime_error_on_macro_expansion/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_error_on_macro_expansion_1" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] diff --git a/test_programs/compile_failure/comptime_error_on_macro_expansion/src/main.nr b/test_programs/compile_failure/comptime_error_on_macro_expansion/src/main.nr new file mode 100644 index 00000000000..1171fc68c82 --- /dev/null +++ b/test_programs/compile_failure/comptime_error_on_macro_expansion/src/main.nr @@ -0,0 +1,42 @@ +mod other; +use other::expr; + +#[foo] +comptime fn foo(_: FunctionDefinition) -> Quoted { + quote { + pub fn generated_by_foo() { + 1 + "a"; + } + } +} + +#[bar] +comptime fn bar(_: FunctionDefinition) -> Quoted { + let expr = expr(); + quote { + pub fn generated_by_bar() { + $expr; + } + } +} + +#[derive_bn254_impl] +pub struct BN254 {} + +comptime fn derive_bn254_impl(s: TypeDefinition) -> Quoted { + let typ = s.as_type(); + quote { + impl BN254 { + fn one() {} + + fn mul(self, scalar: ScalarField) -> Self { + unconstrained_function(); + crate::mul_with_hint($typ, scalar, transcript) + } + } + } +} + +pub unconstrained fn unconstrained_function() {} + +fn main() {} diff --git a/test_programs/compile_failure/comptime_error_on_macro_expansion/src/other.nr b/test_programs/compile_failure/comptime_error_on_macro_expansion/src/other.nr new file mode 100644 index 00000000000..bb1b326663b --- /dev/null +++ b/test_programs/compile_failure/comptime_error_on_macro_expansion/src/other.nr @@ -0,0 +1,3 @@ +pub comptime fn expr() -> Quoted { + quote { 1 + "a" } +} diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_error_on_macro_expansion/execute__tests__stderr.snap b/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_error_on_macro_expansion/execute__tests__stderr.snap new file mode 100644 index 00000000000..7b1716494cf --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_error_on_macro_expansion/execute__tests__stderr.snap @@ -0,0 +1,76 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stderr +--- +warning: Unused expression result of type str<1> + ┌─ src/main.nr:4:1 + │ +4 │ #[foo] + │ ------ While running this function attribute + · +8 │ 1 + "a"; + │ ------- + │ + +warning: Unused expression result of type str<1> + ┌─ src/other.nr:2:13 + │ + 2 │ quote { 1 + "a" } + │ - + │ + ┌─ src/main.nr:13:1 + │ +13 │ #[bar] + │ ------ While running this function attribute + │ + +error: Types in a binary operation should match, but found Field and str<1> + ┌─ src/main.nr:4:1 + │ +4 │ #[foo] + │ ------ While running this function attribute + · +8 │ 1 + "a"; + │ ------- + │ + +error: Types in a binary operation should match, but found Field and str<1> + ┌─ src/other.nr:2:13 + │ + 2 │ quote { 1 + "a" } + │ - + │ + ┌─ src/main.nr:13:1 + │ +13 │ #[bar] + │ ------ While running this function attribute + │ + +error: Expected value, found built-in type `(resolved type)` + ┌─ src/main.nr:23:1 + │ +23 │ #[derive_bn254_impl] + │ -------------------- Failed to parse macro's token stream into top-level item + · +28 │ ╭ quote { +29 │ │ impl BN254 { +30 │ │ fn one() {} +31 │ │ + · │ +36 │ │ } +37 │ │ } + │ ╰─────' + │ + = The resulting token stream was: (stream starts on next line) + impl BN254 { + fn one() { + + } + fn mul < let NScalarSlices: u32 > (self, scalar: ScalarField < NScalarSlices > ) -> Self { + unconstrained_function(); + crate::mul_with_hint(BN254, scalar, transcript) + } + } + = To avoid this error in the future, try adding input validation to your macro. Erroring out early with an `assert` can be a good way to provide a user-friendly error message + +Aborting due to 3 previous errors From e7014282aaa34664153e82af7d8d191896f7d321 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 17 Jun 2025 11:14:31 -0300 Subject: [PATCH 3/4] Preserve unquoted token locations --- .../src/hir/comptime/interpreter.rs | 7 +++---- .../src/hir/comptime/interpreter/unquote.rs | 5 +---- .../execute__tests__stderr.snap | 20 +++++++------------ 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 3f356b7b415..06d902c1b3d 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -554,7 +554,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { HirExpression::Match(match_) => todo!("Evaluate match in comptime code"), HirExpression::Tuple(tuple) => self.evaluate_tuple(tuple), HirExpression::Lambda(lambda) => self.evaluate_lambda(lambda, id), - HirExpression::Quote(tokens) => self.evaluate_quote(tokens, id), + HirExpression::Quote(tokens) => self.evaluate_quote(tokens), HirExpression::Unsafe(block) => self.evaluate_block(block), HirExpression::EnumConstructor(constructor) => { self.evaluate_enum_constructor(constructor, id) @@ -1097,9 +1097,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { Ok(Value::Closure(Box::new(closure))) } - fn evaluate_quote(&mut self, mut tokens: Tokens, expr_id: ExprId) -> IResult { - let location = self.elaborator.interner.expr_location(&expr_id); - let tokens = self.substitute_unquoted_values_into_tokens(tokens, location)?; + fn evaluate_quote(&mut self, mut tokens: Tokens) -> IResult { + let tokens = self.substitute_unquoted_values_into_tokens(tokens)?; Ok(Value::Quoted(Rc::new(tokens))) } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/unquote.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/unquote.rs index fc4daa22edb..4b4a08f03bf 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/unquote.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/unquote.rs @@ -1,5 +1,3 @@ -use noirc_errors::Location; - use crate::{ hir::comptime::errors::IResult, token::{LocatedToken, Token, Tokens}, @@ -14,7 +12,6 @@ impl Interpreter<'_, '_> { pub(super) fn substitute_unquoted_values_into_tokens( &mut self, tokens: Tokens, - location: Location, ) -> IResult> { let mut new_tokens = Vec::with_capacity(tokens.0.len()); @@ -22,7 +19,7 @@ impl Interpreter<'_, '_> { match token.token() { Token::UnquoteMarker(id) => { let value = self.evaluate(*id)?; - let tokens = value.into_tokens(self.elaborator.interner, location)?; + let tokens = value.into_tokens(self.elaborator.interner, token.location())?; new_tokens.extend(tokens); } _ => new_tokens.push(token), diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_error_on_macro_expansion/execute__tests__stderr.snap b/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_error_on_macro_expansion/execute__tests__stderr.snap index 7b1716494cf..6ed3d434df7 100644 --- a/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_error_on_macro_expansion/execute__tests__stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_error_on_macro_expansion/execute__tests__stderr.snap @@ -48,19 +48,13 @@ error: Types in a binary operation should match, but found Field and str<1> error: Expected value, found built-in type `(resolved type)` ┌─ src/main.nr:23:1 - │ -23 │ #[derive_bn254_impl] - │ -------------------- Failed to parse macro's token stream into top-level item - · -28 │ ╭ quote { -29 │ │ impl BN254 { -30 │ │ fn one() {} -31 │ │ - · │ -36 │ │ } -37 │ │ } - │ ╰─────' - │ + │ +23 │ #[derive_bn254_impl] + │ -------------------- Failed to parse macro's token stream into top-level item + · +34 │ crate::mul_with_hint($typ, scalar, transcript) + │ --- + │ = The resulting token stream was: (stream starts on next line) impl BN254 { fn one() { From 089f534cea215ac1ad11ea5707769290c10624a7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 17 Jun 2025 11:51:51 -0300 Subject: [PATCH 4/4] Update snapshot --- .../comptime_parse_all_tokens/execute__tests__stderr.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_parse_all_tokens/execute__tests__stderr.snap b/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_parse_all_tokens/execute__tests__stderr.snap index 7dec81e549d..97defa2ca9c 100644 --- a/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_parse_all_tokens/execute__tests__stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/comptime_parse_all_tokens/execute__tests__stderr.snap @@ -9,7 +9,7 @@ error: Expected an identifier, `crate`, `dep` or `super` but found '(type)' │ ------ Failed to parse macro's token stream into top-level item · 8 │ quote { use $t; } - │ ----------------- + │ - │ = The resulting token stream was: (stream starts on next line) use Field;