From 9c99a97ca8f42bee23cf97ebd724fdc51e647c60 Mon Sep 17 00:00:00 2001 From: jfecher Date: Thu, 6 Jun 2024 14:24:23 +0100 Subject: [PATCH] fix(elaborator): Lazily elaborate globals (#5191) # 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. --- compiler/noirc_frontend/src/elaborator/mod.rs | 11 ++++++++++- .../noirc_frontend/src/elaborator/patterns.rs | 3 +++ compiler/noirc_frontend/src/elaborator/types.rs | 15 ++++++++++----- .../src/hir/def_collector/dc_crate.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 2 +- 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 9403e12496f..964b143b981 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -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>, + + /// 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, } impl<'context> Elaborator<'context> { @@ -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(), } } @@ -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); @@ -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); } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 411caeba7b8..e337726b579 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -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); } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 159ad1c7493..1b611d8f71f 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -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; diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 096bab2b47e..9b47a104a40 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -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, diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 40ca9ce04e0..327900af641 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -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 {