From af52873dbec9ab980d17d9ba4336181c006a9a53 Mon Sep 17 00:00:00 2001 From: jfecher Date: Wed, 18 Sep 2024 13:08:49 -0500 Subject: [PATCH] fix: Unify macro result type with actual type (#6086) # Description ## Problem\* Resolves an issue from @asterite on slack ## Summary\* When we type check a macro call we don't know the actual type so we return a fresh type variable. When we later execute this code at comptime we do know the type, so we should unify the type variable then, potentially catching any inconsistencies. This also makes the value of the type variable known (during evaluation, not type checking!) as in the new test case. ## Additional Context Also fixed an issue where you couldn't call a method as a macro directly and needed an extra `unquote!` call. ## 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. --- .../src/elaborator/expressions.rs | 12 ++++++++++- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- .../noirc_frontend/src/elaborator/types.rs | 2 +- .../src/hir/comptime/interpreter.rs | 20 +++++++++++++++++++ noir_stdlib/src/meta/mod.nr | 9 +++++++-- .../macro_result_type/Nargo.toml | 7 +++++++ .../macro_result_type/src/main.nr | 13 ++++++++++++ .../macro_result_type/t.rs | 12 +++++++++++ 8 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 test_programs/compile_success_empty/macro_result_type/Nargo.toml create mode 100644 test_programs/compile_success_empty/macro_result_type/src/main.nr create mode 100644 test_programs/compile_success_empty/macro_result_type/t.rs diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 375df4e532a..6f81c889e77 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -478,7 +478,17 @@ impl<'context> Elaborator<'context> { // Type check the new call now that it has been changed from a method call // to a function call. This way we avoid duplicating code. - let typ = self.type_check_call(&function_call, func_type, function_args, span); + let mut typ = self.type_check_call(&function_call, func_type, function_args, span); + if is_macro_call { + if self.in_comptime_context() { + typ = self.interner.next_type_variable(); + } else { + let args = function_call.arguments.clone(); + return self + .call_macro(function_call.func, args, location, typ) + .unwrap_or_else(|| (HirExpression::Error, Type::Error)); + } + } (HirExpression::Call(function_call), typ) } None => (HirExpression::Error, Type::Error), diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 905cdc3df79..f9016b1ca65 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -97,7 +97,7 @@ pub struct Elaborator<'context> { def_maps: &'context mut DefMaps, - file: FileId, + pub(crate) file: FileId, in_unsafe_block: bool, nested_loops: usize, diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 816535ba564..c11fc98fb1c 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -715,7 +715,7 @@ impl<'context> Elaborator<'context> { } } - pub(super) fn unify( + pub fn unify( &mut self, actual: &Type, expected: &Type, diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index fe29262fc33..ceaeb9393b1 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -12,6 +12,7 @@ use crate::ast::{BinaryOpKind, FunctionKind, IntegerBitSize, Signedness}; use crate::elaborator::Elaborator; use crate::graph::CrateId; use crate::hir::def_map::ModuleId; +use crate::hir::type_check::TypeCheckError; use crate::hir_def::expr::ImplKind; use crate::hir_def::function::FunctionBody; use crate::macros_api::UnaryOp; @@ -1298,6 +1299,13 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { elaborator.elaborate_expression(expr).0 }); result = self.evaluate(expr)?; + + // Macro calls are typed as type variables during type checking. + // Now that we know the type we need to further unify it in case there + // are inconsistencies or the type needs to be known. + let expected_type = self.elaborator.interner.id_type(id); + let actual_type = result.get_type(); + self.unify(&actual_type, &expected_type, location); } Ok(result) } @@ -1311,6 +1319,18 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } } + fn unify(&mut self, actual: &Type, expected: &Type, location: Location) { + // We need to swap out the elaborator's file since we may be + // in a different one currently, and it uses that for the error location. + let old_file = std::mem::replace(&mut self.elaborator.file, location.file); + self.elaborator.unify(actual, expected, || TypeCheckError::TypeMismatch { + expected_typ: expected.to_string(), + expr_typ: actual.to_string(), + expr_span: location.span, + }); + self.elaborator.file = old_file; + } + fn evaluate_method_call( &mut self, call: HirMethodCallExpression, diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr index f19ccc523fc..1d787ebcdc1 100644 --- a/noir_stdlib/src/meta/mod.nr +++ b/noir_stdlib/src/meta/mod.nr @@ -130,8 +130,13 @@ mod tests { // let _a: Quoted = quote { 1 }; let _a: Quoted = quote_one(); - // let _b: i32 = 1; - let _b: i32 = quote_one!(); + // let _b: Field = 1; + let _b: Field = quote_one!(); + + // Since integers default to fields, if we + // want a different type we have to explicitly cast + // let _c: i32 = 1 as i32; + let _c: i32 = quote_one!() as i32; } } // docs:end:quote-example diff --git a/test_programs/compile_success_empty/macro_result_type/Nargo.toml b/test_programs/compile_success_empty/macro_result_type/Nargo.toml new file mode 100644 index 00000000000..664e405d5fe --- /dev/null +++ b/test_programs/compile_success_empty/macro_result_type/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "macro_result_type" +type = "bin" +authors = [""] +compiler_version = ">=0.34.0" + +[dependencies] diff --git a/test_programs/compile_success_empty/macro_result_type/src/main.nr b/test_programs/compile_success_empty/macro_result_type/src/main.nr new file mode 100644 index 00000000000..4dc97d1d422 --- /dev/null +++ b/test_programs/compile_success_empty/macro_result_type/src/main.nr @@ -0,0 +1,13 @@ +fn main() { + comptime + { + let signature = "hello".as_ctstring(); + let string = signature.as_quoted_str!(); + let result = half(string); + assert_eq(result, 2); + } +} + +comptime fn half(_s: str) -> u32 { + N / 2 +} diff --git a/test_programs/compile_success_empty/macro_result_type/t.rs b/test_programs/compile_success_empty/macro_result_type/t.rs new file mode 100644 index 00000000000..bcd91d7bf5d --- /dev/null +++ b/test_programs/compile_success_empty/macro_result_type/t.rs @@ -0,0 +1,12 @@ + +trait Foo { + fn foo() {} +} + +impl Foo<3> for () { + fn foo() {} +} + +fn main() { + let _ = Foo::foo(); +}