From b5ede0a46fa1d4c751bcf5c87492864091c80df4 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 10 Sep 2024 14:37:13 -0500 Subject: [PATCH] Fix using lazily elaborated comptime globals --- .../noirc_frontend/src/elaborator/patterns.rs | 9 -------- .../src/hir/comptime/interpreter.rs | 23 ++++++++++++++----- .../noirc_frontend/src/hir/comptime/value.rs | 5 ++++ compiler/noirc_frontend/src/hir_def/types.rs | 10 ++++++++ .../comptime_globals_regression/Nargo.toml | 7 ++++++ .../comptime_globals_regression/src/main.nr | 23 +++++++++++++++++++ 6 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 test_programs/compile_success_empty/comptime_globals_regression/Nargo.toml create mode 100644 test_programs/compile_success_empty/comptime_globals_regression/src/main.nr diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index f738657fd23..09357e77c0b 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -513,15 +513,6 @@ impl<'context> Elaborator<'context> { let typ = self.type_check_variable(expr, id, generics); self.interner.push_expr_type(id, typ.clone()); - // Comptime variables must be replaced with their values - if let Some(definition) = self.interner.try_definition(definition_id) { - if definition.comptime && !self.in_comptime_context() { - let mut interpreter = self.setup_interpreter(); - let value = interpreter.evaluate(id); - return self.inline_comptime_value(value, span); - } - } - (id, typ) } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 5f58c18d66e..281318c5b7e 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -434,7 +434,14 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { for scope in self.elaborator.interner.comptime_scopes.iter_mut().rev() { if let Entry::Occupied(mut entry) = scope.entry(id) { - entry.insert(argument); + match entry.get() { + Value::Pointer(reference, true) => { + *reference.borrow_mut() = argument; + } + _ => { + entry.insert(argument); + } + } return Ok(()); } } @@ -527,12 +534,13 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { DefinitionKind::Local(_) => self.lookup(&ident), DefinitionKind::Global(global_id) => { // Avoid resetting the value if it is already known - if let Ok(value) = self.lookup(&ident) { - Ok(value) + if let Some(value) = &self.elaborator.interner.get_global(*global_id).value { + Ok(value.clone()) } else { - let crate_of_global = self.elaborator.interner.get_global(*global_id).crate_id; + let global_id = *global_id; + let crate_of_global = self.elaborator.interner.get_global(global_id).crate_id; let let_ = - self.elaborator.interner.get_global_let_statement(*global_id).ok_or_else( + self.elaborator.interner.get_global_let_statement(global_id).ok_or_else( || { let location = self.elaborator.interner.expr_location(&id); InterpreterError::VariableNotInScope { location } @@ -542,7 +550,10 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { if let_.comptime || crate_of_global != self.crate_id { self.evaluate_let(let_.clone())?; } - self.lookup(&ident) + + let value = self.lookup(&ident)?; + self.elaborator.interner.get_global_mut(global_id).value = Some(value.clone()); + Ok(value) } } DefinitionKind::GenericType(type_variable) => { diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index f6450175955..4eee59489a9 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -405,6 +405,11 @@ impl Value { } Value::Quoted(tokens) => HirExpression::Unquote(add_token_spans(tokens, location.span)), Value::TypedExpr(TypedExpr::ExprId(expr_id)) => interner.expression(&expr_id), + // Only convert pointers with auto_deref = true. These are mutable variables + // and we don't need to wrap them in `&mut`. + Value::Pointer(element, true) => { + return element.unwrap_or_clone().into_hir_expression(interner, location); + } Value::TypedExpr(TypedExpr::StmtId(..)) | Value::Expr(..) | Value::Pointer(..) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index d47e6522756..180d773ff8d 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -471,6 +471,16 @@ impl Shared { pub fn borrow_mut(&self) -> std::cell::RefMut { self.0.borrow_mut() } + + pub fn unwrap_or_clone(self) -> T + where + T: Clone, + { + match Rc::try_unwrap(self.0) { + Ok(elem) => elem.into_inner(), + Err(rc) => rc.as_ref().clone().into_inner(), + } + } } /// A restricted subset of binary operators useable on diff --git a/test_programs/compile_success_empty/comptime_globals_regression/Nargo.toml b/test_programs/compile_success_empty/comptime_globals_regression/Nargo.toml new file mode 100644 index 00000000000..5c5b64c712a --- /dev/null +++ b/test_programs/compile_success_empty/comptime_globals_regression/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_globals_regression" +type = "bin" +authors = [""] +compiler_version = ">=0.33.0" + +[dependencies] diff --git a/test_programs/compile_success_empty/comptime_globals_regression/src/main.nr b/test_programs/compile_success_empty/comptime_globals_regression/src/main.nr new file mode 100644 index 00000000000..54dbc0413f0 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_globals_regression/src/main.nr @@ -0,0 +1,23 @@ +comptime mut global COUNTER = 0; + +fn main() { + comptime + { + increment() + }; + comptime + { + increment() + }; + + assert_eq(get_counter(), 2); +} + +fn get_counter() -> Field { + COUNTER +} + +comptime fn increment() { + COUNTER += 1; + assert_eq(get_counter(), COUNTER); +}