Skip to content

Commit

Permalink
fix: Unify macro result type with actual type (#6086)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
jfecher authored Sep 18, 2024
1 parent e17dfa5 commit af52873
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 5 deletions.
12 changes: 11 additions & 1 deletion compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ impl<'context> Elaborator<'context> {
}
}

pub(super) fn unify(
pub fn unify(
&mut self,
actual: &Type,
expected: &Type,
Expand Down
20 changes: 20 additions & 0 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand Down
9 changes: 7 additions & 2 deletions noir_stdlib/src/meta/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "macro_result_type"
type = "bin"
authors = [""]
compiler_version = ">=0.34.0"

[dependencies]
13 changes: 13 additions & 0 deletions test_programs/compile_success_empty/macro_result_type/src/main.nr
Original file line number Diff line number Diff line change
@@ -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<let N: u32>(_s: str<N>) -> u32 {
N / 2
}
12 changes: 12 additions & 0 deletions test_programs/compile_success_empty/macro_result_type/t.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@

trait Foo<const N: u32> {
fn foo() {}
}

impl Foo<3> for () {
fn foo() {}
}

fn main() {
let _ = Foo::foo();
}

0 comments on commit af52873

Please sign in to comment.