Skip to content
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

fix(experimental elaborator): Fix global values used in the elaborator #5135

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,8 @@ impl<'context> Elaborator<'context> {
let func_type =
self.type_check_variable(function_name, function_id, turbofish_generics);

self.interner.push_expr_type(function_id, func_type.clone());
jfecher marked this conversation as resolved.
Show resolved Hide resolved

// 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);
Expand Down
11 changes: 5 additions & 6 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@
// when multiple impls are available. Instead we default first to choose the Field or u64 impl.
for typ in &self.type_variables {
if let Type::TypeVariable(variable, kind) = typ.follow_bindings() {
let msg = "TypeChecker should only track defaultable type vars";

Check warning on line 333 in compiler/noirc_frontend/src/elaborator/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (defaultable)
variable.bind(kind.default_type().expect(msg));
}
}
Expand Down Expand Up @@ -1211,8 +1211,6 @@

let global_id = global.global_id;
self.current_item = Some(DependencyId::Global(global_id));

let definition_kind = DefinitionKind::Global(global_id);
let let_stmt = global.stmt_def;

if !self.in_contract
Expand All @@ -1227,11 +1225,12 @@
self.push_err(ResolverError::MutableGlobal { span });
}

let (let_statement, _typ) = self.elaborate_let(let_stmt);
self.elaborate_global_let(let_stmt, global_id);

let statement_id = self.interner.get_global(global_id).let_statement;
self.interner.get_global_definition_mut(global_id).kind = definition_kind.clone();
self.interner.replace_statement(statement_id, let_statement);
// Avoid defaulting the types of globals here since they may be used in any function.
// Otherwise we may prematurely default to a Field inside the next function if this
// global was unused there, even if it is consistently used as a u8 everywhere else.
self.type_variables.clear();
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
}

fn define_function_metas(
Expand Down
52 changes: 41 additions & 11 deletions compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
macros_api::{
ForLoopStatement, ForRange, HirStatement, LetStatement, Statement, StatementKind,
},
node_interner::{DefinitionId, DefinitionKind, StmtId},
node_interner::{DefinitionId, DefinitionKind, GlobalId, StmtId},
Type,
};

Expand All @@ -24,7 +24,7 @@ use super::Elaborator;
impl<'context> Elaborator<'context> {
fn elaborate_statement_value(&mut self, statement: Statement) -> (HirStatement, Type) {
match statement.kind {
StatementKind::Let(let_stmt) => self.elaborate_let(let_stmt),
StatementKind::Let(let_stmt) => self.elaborate_local_let(let_stmt),
StatementKind::Constrain(constrain) => self.elaborate_constrain(constrain),
StatementKind::Assign(assign) => self.elaborate_assign(assign),
StatementKind::For(for_stmt) => self.elaborate_for(for_stmt),
Expand All @@ -51,7 +51,33 @@ impl<'context> Elaborator<'context> {
(id, typ)
}

pub(super) fn elaborate_let(&mut self, let_stmt: LetStatement) -> (HirStatement, Type) {
pub(super) fn elaborate_local_let(&mut self, let_stmt: LetStatement) -> (HirStatement, Type) {
let (statement, typ, _) = self.elaborate_let(let_stmt);
(statement, typ)
}

/// Elaborates a global let statement. Compared to the local version, this
/// version fixes up the result to use the given DefinitionId rather than
/// the fresh one defined by the let pattern.
pub(super) fn elaborate_global_let(&mut self, let_stmt: LetStatement, global_id: GlobalId) {
let (let_statement, _typ, new_ids) = self.elaborate_let(let_stmt);
let statement_id = self.interner.get_global(global_id).let_statement;

// To apply the changes from the fresh id created in elaborate_let to this global
// we need to change the definition kind and update the type.
let definition_id = self.interner.get_global(global_id).definition_id;
self.interner.definition_mut(definition_id).kind = DefinitionKind::Global(global_id);

assert_eq!(new_ids.len(), 1, "Globals should only define 1 value");
let definition_type = self.interner.definition_type(new_ids[0].id);
self.interner.push_definition_type(definition_id, definition_type);

self.interner.replace_statement(statement_id, let_statement);
}

/// Elaborate a local or global let statement. In addition to the HirLetStatement and unit
/// type, this also returns each HirIdent defined by this let statement.
fn elaborate_let(&mut self, let_stmt: LetStatement) -> (HirStatement, Type, Vec<HirIdent>) {
let expr_span = let_stmt.expression.span;
let (expression, expr_type) = self.elaborate_expression(let_stmt.expression);
let definition = DefinitionKind::Local(Some(expression));
Expand All @@ -77,14 +103,18 @@ impl<'context> Elaborator<'context> {
expr_type
};

let let_ = HirLetStatement {
pattern: self.elaborate_pattern(let_stmt.pattern, r#type.clone(), definition),
r#type,
expression,
attributes: let_stmt.attributes,
comptime: let_stmt.comptime,
};
(HirStatement::Let(let_), Type::Unit)
let mut created_ids = Vec::new();
let pattern = self.elaborate_pattern_and_store_ids(
let_stmt.pattern,
r#type.clone(),
definition,
&mut created_ids,
);

let attributes = let_stmt.attributes;
let comptime = let_stmt.comptime;
let let_ = HirLetStatement { pattern, r#type, expression, attributes, comptime };
(HirStatement::Let(let_), Type::Unit, created_ids)
}

pub(super) fn elaborate_constrain(&mut self, stmt: ConstrainStatement) -> (HirStatement, Type) {
Expand Down
Loading