diff --git a/aztec_macros/src/utils/hir_utils.rs b/aztec_macros/src/utils/hir_utils.rs index 3801ff1cc75..99b02acd606 100644 --- a/aztec_macros/src/utils/hir_utils.rs +++ b/aztec_macros/src/utils/hir_utils.rs @@ -228,6 +228,7 @@ pub fn inject_global( module_id, file_id, global.attributes.clone(), + false, ); // Add the statement to the scope so its path can be looked up later diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index ad950740a6f..0da39edfd85 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -486,7 +486,8 @@ impl Pattern { pub fn name_ident(&self) -> &Ident { match self { Pattern::Identifier(name_ident) => name_ident, - _ => panic!("only the identifier pattern can return a name"), + Pattern::Mutable(pattern, ..) => pattern.name_ident(), + _ => panic!("Only the Identifier or Mutable patterns can return a name"), } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index a2fce3cc8bd..26b7c212a30 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -336,9 +336,14 @@ impl<'a> Interpreter<'a> { DefinitionKind::Local(_) => self.lookup(&ident), DefinitionKind::Global(global_id) => { // Don't need to check let_.comptime, we can evaluate non-comptime globals too. - let let_ = self.interner.get_global_let_statement(*global_id).unwrap(); - self.evaluate_let(let_)?; - self.lookup(&ident) + // Avoid resetting the value if it is already known + if let Ok(value) = self.lookup(&ident) { + Ok(value) + } else { + let let_ = self.interner.get_global_let_statement(*global_id).unwrap(); + self.evaluate_let(let_)?; + self.lookup(&ident) + } } DefinitionKind::GenericType(type_variable) => { let value = match &*type_variable.borrow() { diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index 1a84dae4a87..5a12eb7292c 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -76,6 +76,20 @@ fn mutating_arrays() { assert_eq!(result, Value::U8(22)); } +#[test] +fn mutate_in_new_scope() { + let program = "fn main() -> pub u8 { + let mut x = 0; + x += 1; + { + x += 1; + } + x + }"; + let result = interpret(program, vec!["main".into()]); + assert_eq!(result, Value::U8(2)); +} + #[test] fn for_loop() { let program = "fn main() -> pub u8 { diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index baedb109967..b2ec7dbc813 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -6,7 +6,8 @@ use noirc_errors::Location; use crate::ast::{ FunctionDefinition, Ident, ItemVisibility, LetStatement, ModuleDeclaration, NoirFunction, - NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, TraitImplItem, TraitItem, TypeImpl, + NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Pattern, TraitImplItem, TraitItem, + TypeImpl, }; use crate::{ graph::CrateId, @@ -109,6 +110,7 @@ impl<'a> ModCollector<'a> { self.module_id, self.file_id, global.attributes.clone(), + matches!(global.pattern, Pattern::Mutable { .. }), ); // Add the statement to the scope so its path can be looked up later @@ -463,6 +465,7 @@ impl<'a> ModCollector<'a> { trait_id.0.local_id, self.file_id, vec![], + false, ); if let Err((first_def, second_def)) = self.def_collector.def_map.modules diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index d542bd4802a..1727471c34f 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -86,6 +86,8 @@ pub enum ResolverError { JumpInConstrainedFn { is_break: bool, span: Span }, #[error("break/continue are only allowed within loops")] JumpOutsideLoop { is_break: bool, span: Span }, + #[error("Only `comptime` globals can be mutable")] + MutableGlobal { span: Span }, #[error("Self-referential structs are not supported")] SelfReferentialStruct { span: Span }, #[error("#[inline(tag)] attribute is only allowed on constrained functions")] @@ -346,6 +348,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) }, + ResolverError::MutableGlobal { span } => { + Diagnostic::simple_error( + "Only `comptime` globals may be mutable".into(), + String::new(), + *span, + ) + }, ResolverError::SelfReferentialStruct { span } => { Diagnostic::simple_error( "Self-referential structs are not supported".into(), diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 74de7bfc09b..b13ffe05c2d 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1222,15 +1222,18 @@ impl<'a> Resolver<'a> { ) -> HirStatement { self.current_item = Some(DependencyId::Global(global_id)); let expression = self.resolve_expression(let_stmt.expression); - let global_id = self.interner.next_global_id(); let definition = DefinitionKind::Global(global_id); if !self.in_contract && let_stmt.attributes.iter().any(|attr| matches!(attr, SecondaryAttribute::Abi(_))) { - self.push_err(ResolverError::AbiAttributeOusideContract { - span: let_stmt.pattern.span(), - }); + let span = let_stmt.pattern.span(); + self.push_err(ResolverError::AbiAttributeOusideContract { span }); + } + + if !let_stmt.comptime && matches!(let_stmt.pattern, Pattern::Mutable(..)) { + let span = let_stmt.pattern.span(); + self.push_err(ResolverError::MutableGlobal { span }); } HirStatement::Let(HirLetStatement { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 5e4fa3df6c5..7ec32613cb7 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -653,11 +653,13 @@ impl NodeInterner { let_statement: StmtId, file: FileId, attributes: Vec, + mutable: bool, ) -> GlobalId { let id = GlobalId(self.globals.len()); let location = Location::new(ident.span(), file); let name = ident.to_string(); - let definition_id = self.push_definition(name, false, DefinitionKind::Global(id), location); + let definition_id = + self.push_definition(name, mutable, DefinitionKind::Global(id), location); self.globals.push(GlobalInfo { id, @@ -682,9 +684,13 @@ impl NodeInterner { local_id: LocalModuleId, file: FileId, attributes: Vec, + mutable: bool, ) -> GlobalId { let statement = self.push_stmt(HirStatement::Error); - self.push_global(name, local_id, statement, file, attributes) + let span = name.span(); + let id = self.push_global(name, local_id, statement, file, attributes, mutable); + self.push_statement_location(statement, span, file); + id } /// Intern an empty function. diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 84a0103c019..4a85f6cb18f 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -235,16 +235,27 @@ fn implementation() -> impl NoirParser { fn global_declaration() -> impl NoirParser { let p = attributes::attributes() .then(maybe_comp_time()) + .then(spanned(keyword(Keyword::Mut)).or_not()) .then_ignore(keyword(Keyword::Global).labelled(ParsingRuleLabel::Global)) .then(ident().map(Pattern::Identifier)); let p = then_commit(p, optional_type_annotation()); let p = then_commit_ignore(p, just(Token::Assign)); let p = then_commit(p, expression()); - p.validate(|((((attributes, comptime), pattern), r#type), expression), span, emit| { - let global_attributes = attributes::validate_secondary_attributes(attributes, span, emit); - LetStatement { pattern, r#type, comptime, expression, attributes: global_attributes } - }) + p.validate( + |(((((attributes, comptime), mutable), mut pattern), r#type), expression), span, emit| { + let global_attributes = + attributes::validate_secondary_attributes(attributes, span, emit); + + // Only comptime globals are allowed to be mutable, but we always parse the `mut` + // and throw the error in name resolution. + if let Some((_, mut_span)) = mutable { + let span = mut_span.merge(pattern.span()); + pattern = Pattern::Mutable(Box::new(pattern), span, false); + } + LetStatement { pattern, r#type, comptime, expression, attributes: global_attributes } + }, + ) .map(TopLevelStatement::Global) } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index fb60047b233..e017ea9e97b 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1284,6 +1284,15 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { } #[test] + fn ban_mutable_globals() { + // Mutable globals are only allowed in a comptime context + let src = r#" + mut global FOO: Field = 0; + fn main() {} + "#; + assert_eq!(get_program_errors(src).len(), 1); + } + fn deny_inline_attribute_on_unconstrained() { let src = r#" #[inline(never)] diff --git a/test_programs/compile_success_empty/comptime_mut_global/Nargo.toml b/test_programs/compile_success_empty/comptime_mut_global/Nargo.toml new file mode 100644 index 00000000000..c29aa8d83b9 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_mut_global/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_mut_global" +type = "bin" +authors = [""] +compiler_version = ">=0.28.0" + +[dependencies] diff --git a/test_programs/compile_success_empty/comptime_mut_global/src/main.nr b/test_programs/compile_success_empty/comptime_mut_global/src/main.nr new file mode 100644 index 00000000000..fa739289d37 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_mut_global/src/main.nr @@ -0,0 +1,30 @@ +comptime mut global COUNTER: Field = 0; + +comptime fn get_unique_id() -> Field { + let id = COUNTER; + COUNTER += 1; + id +} + +fn id1() -> Field { + comptime + { + get_unique_id() + } +} + +fn id2() -> Field { + comptime + { + get_unique_id() + } +} + +fn main() { + // Order of comptime evaluation between functions isn't guaranteed + // so we don't know if (id1 == 0 && id2 == 1) or if (id1 == 1 && id2 == 0). + // we only know they are not equal + let id1 = id1(); + let id2 = id2(); + assert(id1 != id2); +}