Skip to content

Commit

Permalink
fix(elaborator): Lazily elaborate globals (#5191)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves some issues with resolving globals in a type position in
certain Noir repositories.
These were an issue before because `non_literal_globals` were resolved
after `func_metas` were defined, but `func_metas` may refer to them in
their type signature.

## Summary\*

Keeps a list of any unresolved globals and resolve them on demand if
needed.

## Additional Context



## Documentation\*

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

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
jfecher authored Jun 6, 2024
1 parent 070d7e7 commit 9c99a97
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 8 deletions.
11 changes: 10 additions & 1 deletion compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ pub struct Elaborator<'context> {
/// Each element of the Vec represents a scope with every scope together making
/// up all currently visible definitions. The first scope is always the global scope.
comptime_scopes: Vec<HashMap<DefinitionId, comptime::Value>>,

/// These are the globals that have yet to be elaborated.
/// This map is used to lazily evaluate these globals if they're encountered before
/// they are elaborated (e.g. in a function's type or another global's RHS).
unresolved_globals: BTreeMap<GlobalId, UnresolvedGlobal>,
}

impl<'context> Elaborator<'context> {
Expand Down Expand Up @@ -192,6 +197,7 @@ impl<'context> Elaborator<'context> {
trait_constraints: Vec::new(),
current_trait_impl: None,
comptime_scopes: vec![HashMap::default()],
unresolved_globals: BTreeMap::new(),
}
}

Expand All @@ -208,6 +214,9 @@ impl<'context> Elaborator<'context> {
// Additionally, we must resolve integer globals before structs since structs may refer to
// the values of integer globals as numeric generics.
let (literal_globals, non_literal_globals) = filter_literal_globals(items.globals);
for global in non_literal_globals {
this.unresolved_globals.insert(global.global_id, global);
}

for global in literal_globals {
this.elaborate_global(global);
Expand Down Expand Up @@ -242,7 +251,7 @@ impl<'context> Elaborator<'context> {

// We must wait to resolve non-literal globals until after we resolve structs since struct
// globals will need to reference the struct type they're initialized to to ensure they are valid.
for global in non_literal_globals {
while let Some((_, global)) = this.unresolved_globals.pop_first() {
this.elaborate_global(global);
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,9 @@ impl<'context> Elaborator<'context> {
}
}
DefinitionKind::Global(global_id) => {
if let Some(global) = self.unresolved_globals.remove(&global_id) {
self.elaborate_global(global);
}
if let Some(current_item) = self.current_item {
self.interner.add_global_dependency(current_item, global_id);
}
Expand Down
15 changes: 10 additions & 5 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,16 @@ impl<'context> Elaborator<'context> {
.or_else(|| self.resolve_trait_method_by_named_generic(path))
}

fn eval_global_as_array_length(&mut self, global: GlobalId, path: &Path) -> u32 {
let Some(stmt) = self.interner.get_global_let_statement(global) else {
let path = path.clone();
self.push_err(ResolverError::NoSuchNumericTypeVariable { path });
return 0;
fn eval_global_as_array_length(&mut self, global_id: GlobalId, path: &Path) -> u32 {
let Some(stmt) = self.interner.get_global_let_statement(global_id) else {
if let Some(global) = self.unresolved_globals.remove(&global_id) {
self.elaborate_global(global);
return self.eval_global_as_array_length(global_id, path);
} else {
let path = path.clone();
self.push_err(ResolverError::NoSuchNumericTypeVariable { path });
return 0;
}
};

let length = stmt.expression;
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub struct UnresolvedTypeAlias {
pub type_alias_def: NoirTypeAlias,
}

#[derive(Clone)]
#[derive(Debug, Clone)]
pub struct UnresolvedGlobal {
pub file_id: FileId,
pub module_id: LocalModuleId,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ impl DefinitionId {
}

/// An ID for a global value
#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)]
#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy, PartialOrd, Ord)]
pub struct GlobalId(usize);

impl GlobalId {
Expand Down

0 comments on commit 9c99a97

Please sign in to comment.