Skip to content

Commit

Permalink
fix: Allow macros to change types on each iteration of a comptime loop (
Browse files Browse the repository at this point in the history
noir-lang#6105)

# Description

## Problem\*

Resolves
noir-lang#6086 (comment)

## Summary\*

Allows macros to have differing result types on different loop
iterations again after it was banned by the unification in noir-lang#6086.

This is a difficult feature to use since users will still have to work
around the type system only type checking loops once. The recommended
technique found for this PR is to delay type checks by quoting the
relevant function calls that need to be re-typechecked so that when they
are unquoted by the interpreter they are forced to be typechecked every
loop iteration.

The downside of this is that it requires some knowledge of when to do
this. If we go back to the naive approach of
`comptime_change_type_each_loop_iteration` before the quoting:

```noir
fn main() {
    comptime {
        for i in 9..11 {
            // Lengths are different on each iteration: "foo9", "foo10"
            let name = f"foo{i}".as_ctstring().as_quoted_str!();
            let hash = from_signature(name);
            assert(hash > 3);
        }
    }
}

fn from_signature<let N: u32>(_signature: str<N>) -> u32 {
    N
}
```

We get a confusing error now that the unification is gone: "Non-integer
array length `_`" pointing to the body of `from_signature`. The fix for
this is to quote & unquote the `from_signature` call:

```noir
let hash = unquote!(quote { from_signature($name) });
```

But there's nothing telling users this. Ideally we could warn users when
a type changes like this and perhaps have them opt-in to it with a "I
know what I'm doing" of some kind.

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
jfecher authored Sep 19, 2024
1 parent 023cd04 commit 0864e7c
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 23 deletions.
18 changes: 17 additions & 1 deletion compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ impl<'context> Elaborator<'context> {
0
}

pub fn unify(
pub(super) fn unify(
&mut self,
actual: &Type,
expected: &Type,
Expand All @@ -644,6 +644,22 @@ impl<'context> Elaborator<'context> {
}
}

/// Do not apply type bindings even after a successful unification.
/// This function is used by the interpreter for some comptime code
/// which can change types e.g. on each iteration of a for loop.
pub fn unify_without_applying_bindings(
&mut self,
actual: &Type,
expected: &Type,
file: fm::FileId,
make_error: impl FnOnce() -> TypeCheckError,
) {
let mut bindings = TypeBindings::new();
if actual.try_unify(expected, &mut bindings).is_err() {
self.errors.push((make_error().into(), file));
}
}

/// Wrapper of Type::unify_with_coercions using self.errors
pub(super) fn unify_with_coercions(
&mut self,
Expand Down
20 changes: 10 additions & 10 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1303,9 +1303,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
// 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.
// We don't commit any type bindings made this way in case the type of
// the macro result changes across loop iterations.
let expected_type = self.elaborator.interner.id_type(id);
let actual_type = result.get_type();
self.unify(&actual_type, &expected_type, location);
self.unify_without_binding(&actual_type, &expected_type, location);
}
Ok(result)
}
Expand All @@ -1319,16 +1321,14 @@ 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,
fn unify_without_binding(&mut self, actual: &Type, expected: &Type, location: Location) {
self.elaborator.unify_without_applying_bindings(actual, expected, location.file, || {
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(
Expand Down
23 changes: 23 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3725,3 +3725,26 @@ fn use_numeric_generic_in_trait_method() {
println!("{errors:?}");
assert_eq!(errors.len(), 0);
}

#[test]
fn macro_result_type_mismatch() {
let src = r#"
fn main() {
comptime {
let x = unquote!(quote { "test" });
let _: Field = x;
}
}
comptime fn unquote(q: Quoted) -> Quoted {
q
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);
assert!(matches!(
errors[0].0,
CompilationError::TypeError(TypeCheckError::TypeMismatch { .. })
));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "comptime_change_type_each_iteration"
type = "bin"
authors = [""]
compiler_version = ">=0.34.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
fn main() {
comptime
{
for i in 9..11 {
// Lengths are different on each iteration:
// foo9, foo10
let name = f"foo{i}".as_ctstring().as_quoted_str!();

// So to call `from_signature` we need to delay the type check
// by quoting the function call so that we re-typecheck on each iteration
let hash = std::meta::unquote!(quote { from_signature($name) });
assert(hash > 3);
}
}
}

fn from_signature<let N: u32>(_signature: str<N>) -> u32 {
N
}
12 changes: 0 additions & 12 deletions test_programs/compile_success_empty/macro_result_type/t.rs

This file was deleted.

0 comments on commit 0864e7c

Please sign in to comment.