Skip to content

Commit

Permalink
feat(experimental): Implement comptime mut globals (#4926)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #4917 

## Summary\*

Implements mutable comptime globals

## Additional Context

Changes should be relatively straightforward - mostly parser changes to
add parsing `mut` to globals, resolution changes to track this and error
if the global is not also comptime, etc. The one bug that was fixed is
that globals were being re-evaluated in the interpreter each time which
would reset their value whenever they were referenced. This has been
fixed with a lookup check.

## Documentation\*

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

# PR Checklist\*

- [x] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
  • Loading branch information
jfecher and TomAFrench committed Apr 29, 2024
1 parent 59861cf commit dc90de9
Show file tree
Hide file tree
Showing 12 changed files with 114 additions and 15 deletions.
1 change: 1 addition & 0 deletions aztec_macros/src/utils/hir_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
}

Expand Down
11 changes: 8 additions & 3 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
14 changes: 14 additions & 0 deletions compiler/noirc_frontend/src/hir/comptime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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(),
Expand Down
11 changes: 7 additions & 4 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 8 additions & 2 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,11 +653,13 @@ impl NodeInterner {
let_statement: StmtId,
file: FileId,
attributes: Vec<SecondaryAttribute>,
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,
Expand All @@ -682,9 +684,13 @@ impl NodeInterner {
local_id: LocalModuleId,
file: FileId,
attributes: Vec<SecondaryAttribute>,
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.
Expand Down
19 changes: 15 additions & 4 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,16 +235,27 @@ fn implementation() -> impl NoirParser<TopLevelStatement> {
fn global_declaration() -> impl NoirParser<TopLevelStatement> {
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)
}

Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "comptime_mut_global"
type = "bin"
authors = [""]
compiler_version = ">=0.28.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit dc90de9

Please sign in to comment.