-
Notifications
You must be signed in to change notification settings - Fork 370
fix: unify macro call type with actual type in comptime blocks #11411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -87,13 +87,23 @@ impl Elaborator<'_> { | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| /// Push a type variable (its ID and type) as a required type variable: it must be | ||||||||||||||||||||||
| /// bound after type-checking the current function. | ||||||||||||||||||||||
| /// | ||||||||||||||||||||||
| /// The type variable is only pushed if the elaborator is not in a comptime context. | ||||||||||||||||||||||
| /// The reason is that in a comptime context the type of a variable might change | ||||||||||||||||||||||
| /// across a loop's iterations, so a type can temporarily remain as `Type<_>` where | ||||||||||||||||||||||
| /// `_` is bound by the interpreter evaluating an expression's type being unified with | ||||||||||||||||||||||
| /// that type. | ||||||||||||||||||||||
| pub(super) fn push_required_type_variable( | ||||||||||||||||||||||
| &mut self, | ||||||||||||||||||||||
| type_variable_id: TypeVariableId, | ||||||||||||||||||||||
| typ: Type, | ||||||||||||||||||||||
| kind: BindableTypeVariableKind, | ||||||||||||||||||||||
| location: Location, | ||||||||||||||||||||||
| ) { | ||||||||||||||||||||||
| if self.in_comptime_context() { | ||||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| let var = RequiredTypeVariable { type_variable_id, typ, kind, location }; | ||||||||||||||||||||||
| self.get_function_context_mut().required_type_variables.push(var); | ||||||||||||||||||||||
|
Comment on lines
+103
to
108
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,7 +41,6 @@ use iter_extended::{try_vecmap, vecmap}; | |
| use noirc_errors::Location; | ||
| use rustc_hash::FxHashMap as HashMap; | ||
|
|
||
| use crate::TypeVariable; | ||
| use crate::ast::{BinaryOpKind, FunctionKind, IntegerBitSize, UnaryOp}; | ||
| use crate::elaborator::{ElaborateReason, Elaborator, ElaboratorOptions}; | ||
| use crate::hir::Context; | ||
|
|
@@ -75,6 +74,7 @@ use crate::{ | |
| }, | ||
| node_interner::{DefinitionId, DefinitionKind, ExprId, FuncId, StmtId}, | ||
| }; | ||
| use crate::{TypeVariable, UnificationError}; | ||
|
|
||
| use super::errors::{IResult, InterpreterError}; | ||
| use super::value::{Closure, Value, unwrap_rc}; | ||
|
|
@@ -1163,7 +1163,36 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { | |
| // the macro result changes across loop iterations. | ||
| let expected_type = self.elaborator.interner.id_type(id); | ||
| let actual_type = result.get_type(); | ||
| self.unify_without_binding(&actual_type, &expected_type, location); | ||
|
|
||
| // Undo any bindings (if any) from the last time we unified this expression's | ||
| // type against the actual type | ||
| if let Some(bindings) = | ||
| self.elaborator.interner.macro_call_expression_bindings.remove(&id) | ||
| { | ||
| for (var, kind, _typ) in bindings.values() { | ||
| var.unbind(var.id(), kind.clone()); | ||
| } | ||
| } | ||
|
|
||
| let mut bindings = TypeBindings::default(); | ||
| match actual_type.try_unify(&expected_type, &mut bindings) { | ||
| Ok(()) => { | ||
| // Store the bindings so we can undo them next time | ||
| self.elaborator | ||
| .interner | ||
| .macro_call_expression_bindings | ||
| .insert(id, bindings.clone()); | ||
| Type::apply_type_bindings(bindings); | ||
| } | ||
| Err(UnificationError) => { | ||
| let error = TypeCheckError::TypeMismatch { | ||
| expected_typ: expected_type.to_string(), | ||
| expr_typ: actual_type.to_string(), | ||
| expr_location: location, | ||
| }; | ||
|
Comment on lines
+1167
to
+1192
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you factor out all or some of this code? It makes |
||
| self.elaborator.push_err(error); | ||
| } | ||
| } | ||
| } | ||
| Ok(result) | ||
| } | ||
|
|
@@ -1175,20 +1204,6 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { | |
| } | ||
| } | ||
|
|
||
| /// This function is used by the interpreter for some comptime code | ||
| /// which can change types e.g. on each iteration of a for loop. | ||
| fn unify_without_binding(&mut self, actual: &Type, expected: &Type, location: Location) { | ||
| let mut bindings = TypeBindings::default(); | ||
| if actual.try_unify(expected, &mut bindings).is_err() { | ||
| let error = TypeCheckError::TypeMismatch { | ||
| expected_typ: expected.to_string(), | ||
| expr_typ: actual.to_string(), | ||
| expr_location: location, | ||
| }; | ||
| self.elaborator.push_err(error); | ||
| } | ||
| } | ||
|
|
||
| fn evaluate_cast(&mut self, cast: &HirCastExpression, id: ExprId) -> IResult<Value> { | ||
| let evaluated_lhs = self.evaluate(cast.lhs)?; | ||
| let location = self.elaborator.interner.expr_location(&id); | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this open us up to regular unbound type variables though? E.g. the case this was meant to originally catch may no longer be caught in comptime code?