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 69743935dc3..0d1dd1b4337 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -15,7 +15,7 @@ use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker}; use crate::hir::Context; use crate::macros_api::{MacroError, MacroProcessor}; -use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId}; +use crate::node_interner::{FuncId, GlobalId, NodeInterner, StructId, TraitId, TypeAliasId}; use crate::parser::{ParserError, SortedModule}; use crate::{ @@ -109,7 +109,7 @@ pub struct UnresolvedTypeAlias { pub struct UnresolvedGlobal { pub file_id: FileId, pub module_id: LocalModuleId, - pub stmt_id: StmtId, + pub global_id: GlobalId, pub stmt_def: LetStatement, } @@ -317,7 +317,7 @@ impl DefCollector { // Must resolve structs before we resolve globals. errors.extend(resolve_structs(context, def_collector.collected_types, crate_id)); - // We must wait to resolve non-integer globals until after we resolve structs since structs + // We must wait to resolve non-integer 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. resolved_globals.extend(resolve_globals(context, other_globals, crate_id)); errors.extend(resolved_globals.errors); @@ -436,15 +436,15 @@ fn filter_literal_globals( fn type_check_globals( interner: &mut NodeInterner, - global_ids: Vec<(FileId, StmtId)>, + global_ids: Vec<(FileId, GlobalId)>, ) -> Vec<(CompilationError, fm::FileId)> { global_ids - .iter() - .flat_map(|(file_id, stmt_id)| { - TypeChecker::check_global(stmt_id, interner) + .into_iter() + .flat_map(|(file_id, global_id)| { + TypeChecker::check_global(global_id, interner) .iter() .cloned() - .map(|e| (e.into(), *file_id)) + .map(|e| (e.into(), file_id)) .collect::>() }) .collect() @@ -455,12 +455,12 @@ fn type_check_functions( file_func_ids: Vec<(FileId, FuncId)>, ) -> Vec<(CompilationError, fm::FileId)> { file_func_ids - .iter() + .into_iter() .flat_map(|(file, func)| { - type_check_func(interner, *func) + type_check_func(interner, func) .iter() .cloned() - .map(|e| (e.into(), *file)) + .map(|e| (e.into(), file)) .collect::>() }) .collect() 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 3cd60c33b8b..e63f7f4c413 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -89,13 +89,12 @@ impl<'a> ModCollector<'a> { for global in globals { let name = global.pattern.name_ident().clone(); - // First create dummy function in the DefInterner - // So that we can get a StmtId - let stmt_id = context.def_interner.push_empty_global(); + let global_id = + context.def_interner.push_empty_global(name.clone(), self.module_id, self.file_id); // Add the statement to the scope so its path can be looked up later - let result = - self.def_collector.def_map.modules[self.module_id.0].declare_global(name, stmt_id); + let result = self.def_collector.def_map.modules[self.module_id.0] + .declare_global(name, global_id); if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::Duplicate { @@ -109,7 +108,7 @@ impl<'a> ModCollector<'a> { self.def_collector.collected_globals.push(UnresolvedGlobal { file_id: self.file_id, module_id: self.module_id, - stmt_id, + global_id, stmt_def: global, }); } @@ -440,11 +439,15 @@ impl<'a> ModCollector<'a> { } } TraitItem::Constant { name, .. } => { - let stmt_id = context.def_interner.push_empty_global(); + let global_id = context.def_interner.push_empty_global( + name.clone(), + trait_id.0.local_id, + self.file_id, + ); if let Err((first_def, second_def)) = self.def_collector.def_map.modules [trait_id.0.local_id.0] - .declare_global(name.clone(), stmt_id) + .declare_global(name.clone(), global_id) { let error = DefCollectorErrorKind::Duplicate { typ: DuplicateType::TraitAssociatedConst, diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index fbb5e5cf741..309618dd011 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use noirc_errors::Location; use crate::{ - node_interner::{FuncId, StmtId, StructId, TraitId, TypeAliasId}, + node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}, Ident, }; @@ -76,7 +76,7 @@ impl ModuleData { self.definitions.remove_definition(name); } - pub fn declare_global(&mut self, name: Ident, id: StmtId) -> Result<(), (Ident, Ident)> { + pub fn declare_global(&mut self, name: Ident, id: GlobalId) -> Result<(), (Ident, Ident)> { self.declare(name, id.into(), None) } diff --git a/compiler/noirc_frontend/src/hir/def_map/module_def.rs b/compiler/noirc_frontend/src/hir/def_map/module_def.rs index 3e5629639fa..54d092f9515 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_def.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_def.rs @@ -1,4 +1,4 @@ -use crate::node_interner::{FuncId, StmtId, StructId, TraitId, TypeAliasId}; +use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}; use super::ModuleId; @@ -10,7 +10,7 @@ pub enum ModuleDefId { TypeId(StructId), TypeAliasId(TypeAliasId), TraitId(TraitId), - GlobalId(StmtId), + GlobalId(GlobalId), } impl ModuleDefId { @@ -42,9 +42,9 @@ impl ModuleDefId { } } - pub fn as_global(&self) -> Option { + pub fn as_global(&self) -> Option { match self { - ModuleDefId::GlobalId(stmt_id) => Some(*stmt_id), + ModuleDefId::GlobalId(global_id) => Some(*global_id), _ => None, } } @@ -88,9 +88,9 @@ impl From for ModuleDefId { } } -impl From for ModuleDefId { - fn from(stmt_id: StmtId) -> Self { - ModuleDefId::GlobalId(stmt_id) +impl From for ModuleDefId { + fn from(global_id: GlobalId) -> Self { + ModuleDefId::GlobalId(global_id) } } @@ -162,13 +162,13 @@ impl TryFromModuleDefId for TraitId { } } -impl TryFromModuleDefId for StmtId { +impl TryFromModuleDefId for GlobalId { fn try_from(id: ModuleDefId) -> Option { id.as_global() } fn dummy_id() -> Self { - StmtId::dummy_id() + GlobalId::dummy_id() } fn description() -> String { diff --git a/compiler/noirc_frontend/src/hir/resolution/globals.rs b/compiler/noirc_frontend/src/hir/resolution/globals.rs index b5aec212dbf..9fb31271727 100644 --- a/compiler/noirc_frontend/src/hir/resolution/globals.rs +++ b/compiler/noirc_frontend/src/hir/resolution/globals.rs @@ -6,13 +6,13 @@ use crate::{ def_map::ModuleId, Context, }, - node_interner::StmtId, + node_interner::GlobalId, }; use fm::FileId; use iter_extended::vecmap; pub(crate) struct ResolvedGlobals { - pub(crate) globals: Vec<(FileId, StmtId)>, + pub(crate) globals: Vec<(FileId, GlobalId)>, pub(crate) errors: Vec<(CompilationError, FileId)>, } @@ -40,16 +40,13 @@ pub(crate) fn resolve_globals( global.file_id, ); - let name = global.stmt_def.pattern.name_ident().clone(); - - let hir_stmt = resolver.resolve_global_let(global.stmt_def); + let hir_stmt = resolver.resolve_global_let(global.stmt_def, global.global_id); errors.extend(take_errors(global.file_id, resolver)); - context.def_interner.update_global(global.stmt_id, hir_stmt); - - context.def_interner.push_global(global.stmt_id, name, global.module_id); + let statement_id = context.def_interner.get_global(global.global_id).let_statement; + context.def_interner.replace_statement(statement_id, hir_stmt); - (global.file_id, global.stmt_id) + (global.file_id, global.global_id) }); ResolvedGlobals { globals, errors } } diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 347bf9451f6..c124e132a40 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -28,8 +28,8 @@ use crate::graph::CrateId; use crate::hir::def_map::{LocalModuleId, ModuleDefId, TryFromModuleDefId, MAIN_FUNCTION}; use crate::hir_def::stmt::{HirAssignStatement, HirForStatement, HirLValue, HirPattern}; use crate::node_interner::{ - DefinitionId, DefinitionKind, ExprId, FuncId, NodeInterner, StmtId, StructId, TraitId, - TraitImplId, TraitMethodId, + DefinitionId, DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, NodeInterner, StmtId, + StructId, TraitId, TraitImplId, TraitMethodId, }; use crate::{ hir::{def_map::CrateDefMap, resolution::path_resolver::PathResolver}, @@ -93,8 +93,9 @@ pub struct Resolver<'a> { /// to the corresponding trait impl ID. current_trait_impl: Option, - /// If we're currently resolving fields in a struct type, this is set to that type. - current_struct_type: Option, + /// The current dependency item we're resolving. + /// Used to link items to their dependencies in the dependency graph + current_item: Option, /// True if the current module is a contract. /// This is usually determined by self.path_resolver.module_id(), but it can @@ -151,7 +152,7 @@ impl<'a> Resolver<'a> { errors: Vec::new(), lambda_stack: Vec::new(), current_trait_impl: None, - current_struct_type: None, + current_item: None, file, in_contract, } @@ -188,6 +189,7 @@ impl<'a> Resolver<'a> { func_id: FuncId, ) -> (HirFunction, FuncMeta, Vec) { self.scopes.start_function(); + self.current_item = Some(DependencyId::Function(func_id)); // Check whether the function has globals in the local module and add them to the scope self.resolve_local_globals(); @@ -342,21 +344,22 @@ impl<'a> Resolver<'a> { // This check is necessary to maintain the same definition ids in the interner. Currently, each function uses a new resolver that has its own ScopeForest and thus global scope. // We must first check whether an existing definition ID has been inserted as otherwise there will be multiple definitions for the same global statement. // This leads to an error in evaluation where the wrong definition ID is selected when evaluating a statement using the global. The check below prevents this error. - let mut stmt_id = None; + let mut global_id = None; let global = self.interner.get_all_globals(); - for (global_stmt_id, global_info) in global { + for global_info in global { if global_info.ident == name && global_info.local_id == self.path_resolver.local_module_id() { - stmt_id = Some(global_stmt_id); + global_id = Some(global_info.id); } } - let (ident, resolver_meta) = if let Some(id) = stmt_id { - let hir_let_stmt = self.interner.let_statement(&id); - let ident = hir_let_stmt.ident(); + let (ident, resolver_meta) = if let Some(id) = global_id { + let global = self.interner.get_global(id); + let hir_ident = HirIdent::non_trait_method(global.definition_id, global.location); + let ident = hir_ident.clone(); let resolver_meta = ResolverMeta { num_times_used: 0, ident, warn_if_unused: true }; - (hir_let_stmt.ident(), resolver_meta) + (hir_ident, resolver_meta) } else { let location = Location::new(name.span(), self.file); let id = @@ -603,9 +606,9 @@ impl<'a> Resolver<'a> { struct_type.borrow().to_string() }); - if let Some(current_struct) = self.current_struct_type { + if let Some(current_item) = self.current_item { let dependency_id = struct_type.borrow().id; - self.interner.add_type_dependency(current_struct, dependency_id); + self.interner.add_type_dependency(current_item, dependency_id); } Type::Struct(struct_type, args) @@ -660,10 +663,10 @@ impl<'a> Resolver<'a> { // If we cannot find a local generic of the same name, try to look up a global match self.path_resolver.resolve(self.def_maps, path.clone()) { Ok(ModuleDefId::GlobalId(id)) => { - if let Some(current_struct) = self.current_struct_type { - self.interner.add_type_global_dependency(current_struct, id); + if let Some(current_item) = self.current_item { + self.interner.add_global_dependency(current_item, id); } - Some(Type::Constant(self.eval_global_as_array_length(id))) + Some(Type::Constant(self.eval_global_as_array_length(id, path))) } _ => None, } @@ -849,18 +852,20 @@ impl<'a> Resolver<'a> { // Check whether the struct definition has globals in the local module and add them to the scope self.resolve_local_globals(); - self.current_struct_type = Some(struct_id); + self.current_item = Some(DependencyId::Struct(struct_id)); let fields = vecmap(unresolved.fields, |(ident, typ)| (ident, self.resolve_type(typ))); (generics, fields, self.errors) } fn resolve_local_globals(&mut self) { - for (stmt_id, global_info) in self.interner.get_all_globals() { - if global_info.local_id == self.path_resolver.local_module_id() { - let global_stmt = self.interner.let_statement(&stmt_id); - let definition = DefinitionKind::Global(global_stmt.expression); - self.add_global_variable_decl(global_info.ident, definition); + let globals = vecmap(self.interner.get_all_globals(), |global| { + (global.id, global.local_id, global.ident.clone()) + }); + for (id, local_module_id, name) in globals { + if local_module_id == self.path_resolver.local_module_id() { + let definition = DefinitionKind::Global(id); + self.add_global_variable_decl(name, definition); } } } @@ -1124,9 +1129,15 @@ impl<'a> Resolver<'a> { } } - pub fn resolve_global_let(&mut self, let_stmt: crate::LetStatement) -> HirStatement { + pub fn resolve_global_let( + &mut self, + let_stmt: crate::LetStatement, + global_id: GlobalId, + ) -> HirStatement { + self.current_item = Some(DependencyId::Global(global_id)); let expression = self.resolve_expression(let_stmt.expression); - let definition = DefinitionKind::Global(expression); + let global_id = self.interner.next_global_id(); + let definition = DefinitionKind::Global(global_id); HirStatement::Let(HirLetStatement { pattern: self.resolve_pattern(let_stmt.pattern, definition), @@ -1405,6 +1416,10 @@ impl<'a> Resolver<'a> { if hir_ident.id != DefinitionId::dummy_id() { match self.interner.definition(hir_ident.id).kind { DefinitionKind::Function(id) => { + if let Some(current_item) = self.current_item { + self.interner.add_function_dependency(current_item, id); + } + if self.interner.function_visibility(id) != FunctionVisibility::Public { @@ -1416,7 +1431,11 @@ impl<'a> Resolver<'a> { ); } } - DefinitionKind::Global(_) => {} + DefinitionKind::Global(global_id) => { + if let Some(current_item) = self.current_item { + self.interner.add_global_dependency(current_item, global_id); + } + } DefinitionKind::GenericType(_) => { // Initialize numeric generics to a polymorphic integer type in case // they're used in expressions. We must do this here since the type @@ -1712,8 +1731,8 @@ impl<'a> Resolver<'a> { } if let Some(global) = TryFromModuleDefId::try_from(id) { - let let_stmt = self.interner.let_statement(&global); - return Ok(let_stmt.ident().id); + let global = self.interner.get_global(global); + return Ok(global.definition_id); } let expected = "global variable".into(); @@ -1895,10 +1914,11 @@ impl<'a> Resolver<'a> { self.interner.push_expr(hir_block) } - fn eval_global_as_array_length(&mut self, global: StmtId) -> u64 { - let stmt = match self.interner.statement(&global) { - HirStatement::Let(let_expr) => let_expr, - _ => return 0, + fn eval_global_as_array_length(&mut self, global: GlobalId, path: &Path) -> u64 { + let Some(stmt) = self.interner.get_global_let_statement(global) 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/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 3c2a970ee84..8952ba83586 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -15,7 +15,7 @@ pub use errors::TypeCheckError; use crate::{ hir_def::{expr::HirExpression, stmt::HirStatement, traits::TraitConstraint}, - node_interner::{ExprId, FuncId, NodeInterner, StmtId}, + node_interner::{ExprId, FuncId, GlobalId, NodeInterner}, Type, }; @@ -193,7 +193,10 @@ impl<'interner> TypeChecker<'interner> { (body_type, std::mem::take(&mut self.delayed_type_checks)) } - pub fn check_global(id: &StmtId, interner: &'interner mut NodeInterner) -> Vec { + pub fn check_global( + id: GlobalId, + interner: &'interner mut NodeInterner, + ) -> Vec { let mut this = Self { delayed_type_checks: Vec::new(), interner, @@ -201,7 +204,8 @@ impl<'interner> TypeChecker<'interner> { trait_constraints: Vec::new(), current_function: None, }; - this.check_statement(id); + let statement = this.interner.get_global(id).let_statement; + this.check_statement(&statement); this.errors } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 3323f1e24c4..74e04696607 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -726,7 +726,12 @@ impl<'interner> Monomorphizer<'interner> { ident_expression } } - DefinitionKind::Global(expr_id) => self.expr(*expr_id), + DefinitionKind::Global(global_id) => { + let Some(let_) = self.interner.get_global_let_statement(*global_id) else { + unreachable!("Globals should have a corresponding let statement by monomorphization") + }; + self.expr(let_.expression) + } DefinitionKind::Local(_) => self.lookup_captured_expr(ident.id).unwrap_or_else(|| { let ident = self.local_ident(&ident).unwrap(); ast::Expression::Ident(ident) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 53583bb0f17..bd8c8bbd6cf 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -140,7 +140,9 @@ pub struct NodeInterner { /// checking. field_indices: HashMap, - globals: HashMap, // NOTE: currently only used for checking repeat globals and restricting their scope to a module + // Maps GlobalId -> GlobalInfo + // NOTE: currently only used for checking repeat globals and restricting their scope to a module + globals: Vec, next_type_variable_id: std::cell::Cell, @@ -179,7 +181,7 @@ pub struct NodeInterner { #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum DependencyId { Struct(StructId), - Global(StmtId), + Global(GlobalId), Function(FuncId), Alias(TypeAliasId), } @@ -279,6 +281,17 @@ impl From for Index { } } +/// An ID for a global value +#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] +pub struct GlobalId(usize); + +impl GlobalId { + // Dummy id for error reporting + pub fn dummy_id() -> Self { + GlobalId(std::usize::MAX) + } +} + #[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] pub struct StmtId(Index); @@ -429,7 +442,7 @@ impl DefinitionInfo { pub enum DefinitionKind { Function(FuncId), - Global(ExprId), + Global(GlobalId), /// Locals may be defined in let statements or parameters, /// in which case they will not have an associated ExprId @@ -450,7 +463,7 @@ impl DefinitionKind { pub fn get_rhs(&self) -> Option { match self { DefinitionKind::Function(_) => None, - DefinitionKind::Global(id) => Some(*id), + DefinitionKind::Global(_) => None, DefinitionKind::Local(id) => *id, DefinitionKind::GenericType(_) => None, } @@ -459,8 +472,12 @@ impl DefinitionKind { #[derive(Debug, Clone)] pub struct GlobalInfo { + pub id: GlobalId, + pub definition_id: DefinitionId, pub ident: Ident, pub local_id: LocalModuleId, + pub location: Location, + pub let_statement: StmtId, } impl Default for NodeInterner { @@ -489,7 +506,7 @@ impl Default for NodeInterner { instantiation_bindings: HashMap::new(), field_indices: HashMap::new(), next_type_variable_id: std::cell::Cell::new(0), - globals: HashMap::new(), + globals: Vec::new(), struct_methods: HashMap::new(), primitive_methods: HashMap::new(), type_alias_ref: Vec::new(), @@ -651,26 +668,41 @@ impl NodeInterner { self.type_ref_locations.push((typ, location)); } - pub fn push_global(&mut self, stmt_id: StmtId, ident: Ident, local_id: LocalModuleId) { - self.globals.insert(stmt_id, GlobalInfo { ident, local_id }); + fn push_global( + &mut self, + ident: Ident, + local_id: LocalModuleId, + let_statement: StmtId, + file: FileId, + ) -> 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); + self.globals.push(GlobalInfo { + id, + definition_id, + ident, + local_id, + let_statement, + location, + }); + id } - /// Intern an empty global stmt. Used for collecting globals - pub fn push_empty_global(&mut self) -> StmtId { - self.push_stmt(HirStatement::Error) + pub fn next_global_id(&mut self) -> GlobalId { + GlobalId(self.globals.len()) } - pub fn update_global(&mut self, stmt_id: StmtId, hir_stmt: HirStatement) { - let def = - self.nodes.get_mut(stmt_id.0).expect("ice: all function ids should have definitions"); - - let stmt = match def { - Node::Statement(stmt) => stmt, - _ => { - panic!("ice: all global ids should correspond to a statement in the interner") - } - }; - *stmt = hir_stmt; + /// Intern an empty global. Used for collecting globals before they're defined + pub fn push_empty_global( + &mut self, + name: Ident, + local_id: LocalModuleId, + file: FileId, + ) -> GlobalId { + let statement = self.push_stmt(HirStatement::Error); + self.push_global(name, local_id, statement, file) } /// Intern an empty function. @@ -850,19 +882,19 @@ impl NodeInterner { } } - /// Returns the interned let statement corresponding to `stmt_id` - pub fn let_statement(&self, stmt_id: &StmtId) -> HirLetStatement { - let def = - self.nodes.get(stmt_id.0).expect("ice: all statement ids should have definitions"); + /// Try to get the `HirLetStatement` which defines a given global value + pub fn get_global_let_statement(&self, global: GlobalId) -> Option { + let global = self.get_global(global); + let def = self.nodes.get(global.let_statement.0)?; match def { - Node::Statement(hir_stmt) => { - match hir_stmt { - HirStatement::Let(let_stmt) => let_stmt.clone(), - _ => panic!("ice: all let statement ids should correspond to a let statement in the interner"), + Node::Statement(hir_stmt) => match hir_stmt { + HirStatement::Let(let_stmt) => Some(let_stmt.clone()), + _ => { + panic!("ice: all globals should correspond to a let statement in the interner") } }, - _ => panic!("ice: all statement ids should correspond to a statement in the interner"), + _ => panic!("ice: all globals should correspond to a statement in the interner"), } } @@ -928,12 +960,17 @@ impl NodeInterner { &self.type_aliases[id.0] } - pub fn get_global(&self, stmt_id: &StmtId) -> Option { - self.globals.get(stmt_id).cloned() + pub fn get_global(&self, global_id: GlobalId) -> &GlobalInfo { + &self.globals[global_id.0] } - pub fn get_all_globals(&self) -> HashMap { - self.globals.clone() + pub fn get_global_definition(&self, global_id: GlobalId) -> &DefinitionInfo { + let global = self.get_global(global_id); + self.definition(global.definition_id) + } + + pub fn get_all_globals(&self) -> &[GlobalInfo] { + &self.globals } /// Returns the type of an item stored in the Interner or Error if it was not found. @@ -969,6 +1006,12 @@ impl NodeInterner { *old = Node::Expression(new); } + /// Replaces the HirStatement at the given StmtId with a new HirStatement + pub fn replace_statement(&mut self, stmt_id: StmtId, hir_stmt: HirStatement) { + let old = self.nodes.get_mut(stmt_id.0).unwrap(); + *old = Node::Statement(hir_stmt); + } + pub fn next_type_variable_id(&self) -> TypeVariableId { let id = self.next_type_variable_id.get(); self.next_type_variable_id.set(id + 1); @@ -1456,12 +1499,16 @@ impl NodeInterner { /// Register that `dependent` depends on `dependency`. /// This is usually because `dependent` refers to `dependency` in one of its struct fields. - pub fn add_type_dependency(&mut self, dependent: StructId, dependency: StructId) { - self.add_dependency(DependencyId::Struct(dependent), DependencyId::Struct(dependency)); + pub fn add_type_dependency(&mut self, dependent: DependencyId, dependency: StructId) { + self.add_dependency(dependent, DependencyId::Struct(dependency)); + } + + pub fn add_global_dependency(&mut self, dependent: DependencyId, dependency: GlobalId) { + self.add_dependency(dependent, DependencyId::Global(dependency)); } - pub fn add_type_global_dependency(&mut self, dependent: StructId, dependency: StmtId) { - self.add_dependency(DependencyId::Struct(dependent), DependencyId::Global(dependency)); + pub fn add_function_dependency(&mut self, dependent: DependencyId, dependency: FuncId) { + self.add_dependency(dependent, DependencyId::Function(dependency)); } fn add_dependency(&mut self, dependent: DependencyId, dependency: DependencyId) { @@ -1503,20 +1550,15 @@ impl NodeInterner { break; } DependencyId::Global(global_id) => { - let let_stmt = match self.statement(&global_id) { - HirStatement::Let(let_stmt) => let_stmt, - other => unreachable!( - "Expected global to be a `let` statement, found {other:?}" - ), - }; - - let (name, location) = - self.pattern_name_and_location(&let_stmt.pattern); - push_error(name, &scc, i, location); + let global = self.get_global(global_id); + let name = global.ident.to_string(); + push_error(name, &scc, i, global.location); + break; } DependencyId::Alias(alias_id) => { let alias = self.get_type_alias(alias_id); push_error(alias.name.to_string(), &scc, i, alias.location); + break; } // Mutually recursive functions are allowed DependencyId::Function(_) => (), @@ -1539,57 +1581,20 @@ impl NodeInterner { Cow::Borrowed(self.get_type_alias(id).name.0.contents.as_ref()) } DependencyId::Global(id) => { - let let_stmt = match self.statement(&id) { - HirStatement::Let(let_stmt) => let_stmt, - other => { - unreachable!("Expected global to be a `let` statement, found {other:?}") - } - }; - Cow::Owned(self.pattern_name_and_location(&let_stmt.pattern).0) + Cow::Borrowed(self.get_global(id).ident.0.contents.as_ref()) } }; let mut cycle = index_to_string(scc[start_index]).to_string(); - for i in 0..scc.len() { + // Reversing the dependencies here matches the order users would expect for the error message + for i in (0..scc.len()).rev() { cycle += " -> "; - cycle += &index_to_string(scc[(start_index + i + 1) % scc.len()]); + cycle += &index_to_string(scc[(start_index + i) % scc.len()]); } cycle } - - /// Returns the name and location of this pattern. - /// If this pattern is a tuple or struct the 'name' will be a string representation of the - /// entire pattern. Similarly, the location will be the merged location of all fields. - fn pattern_name_and_location( - &self, - pattern: &crate::hir_def::stmt::HirPattern, - ) -> (String, Location) { - match pattern { - crate::hir_def::stmt::HirPattern::Identifier(ident) => { - let definition = self.definition(ident.id); - (definition.name.clone(), definition.location) - } - crate::hir_def::stmt::HirPattern::Mutable(pattern, _) => { - self.pattern_name_and_location(pattern) - } - crate::hir_def::stmt::HirPattern::Tuple(fields, location) => { - let fields = vecmap(fields, |field| self.pattern_name_and_location(field).0); - - let fields = fields.join(", "); - (format!("({fields})"), *location) - } - crate::hir_def::stmt::HirPattern::Struct(typ, fields, location) => { - let fields = vecmap(fields, |(field_name, field)| { - let field = self.pattern_name_and_location(field).0; - format!("{field_name}: {field}") - }); - let fields = fields.join(", "); - (format!("{typ} {{ {fields} }}"), *location) - } - } - } } impl Methods { diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 1f41a29dd4f..507d58ad8f1 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -133,7 +133,7 @@ fn global_declaration() -> impl NoirParser { ); let p = then_commit(p, optional_type_annotation()); let p = then_commit_ignore(p, just(Token::Assign)); - let p = then_commit(p, literal_or_collection(expression()).map_with_span(Expression::new)); + let p = then_commit(p, expression()); p.map(LetStatement::new_let).map(TopLevelStatement::Global) } @@ -1676,24 +1676,6 @@ fn literal() -> impl NoirParser { }) } -fn literal_with_sign() -> impl NoirParser { - choice(( - literal(), - just(Token::Minus).then(literal()).map(|(_, exp)| match exp { - ExpressionKind::Literal(Literal::Integer(value, sign)) => { - ExpressionKind::Literal(Literal::Integer(value, !sign)) - } - _ => unreachable!(), - }), - )) -} - -fn literal_or_collection<'a>( - expr_parser: impl ExprParser + 'a, -) -> impl NoirParser + 'a { - choice((literal_with_sign(), constructor(expr_parser.clone()), array_expr(expr_parser))) -} - #[cfg(test)] mod test { use noirc_errors::CustomDiagnostic; diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 5aae579ecb4..1deff446d7e 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1174,4 +1174,14 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { "#; assert_eq!(get_program_errors(src).len(), 1); } + + #[test] + fn deny_cyclic_globals() { + let src = r#" + global A = B; + global B = A; + fn main() {} + "#; + assert_eq!(get_program_errors(src).len(), 1); + } } diff --git a/docs/docs/noir/concepts/globals.md b/docs/docs/noir/concepts/globals.md new file mode 100644 index 00000000000..063a3d89248 --- /dev/null +++ b/docs/docs/noir/concepts/globals.md @@ -0,0 +1,72 @@ +--- +title: Global Variables +description: + Learn about global variables in Noir. Discover how + to declare, modify, and use them in your programs. +keywords: [noir programming language, globals, global variables, constants] +sidebar_position: 8 +--- + +## Globals + + +Noir supports global variables. The global's type can be inferred by the compiler entirely: + +```rust +global N = 5; // Same as `global N: Field = 5` + +global TUPLE = (3, 2); + +fn main() { + assert(N == 5); + assert(N == TUPLE.0 + TUPLE.1); +} +``` + +:::info + +Globals can be defined as any expression, so long as they don't depend on themselves - otherwise there would be a dependency cycle! For example: + +```rust +global T = foo(T); // dependency error +``` + +::: + + +If they are initialized to a literal integer, globals can be used to specify an array's length: + +```rust +global N: Field = 2; + +fn main(y : [Field; N]) { + assert(y[0] == y[1]) +} +``` + +A global from another module can be imported or referenced externally like any other name: + +```rust +global N = 20; + +fn main() { + assert(my_submodule::N != N); +} + +mod my_submodule { + global N: Field = 10; +} +``` + +When a global is used, Noir replaces the name with its definition on each occurrence. +This means globals defined using function calls will repeat the call each time they're used: + +```rust +global RESULT = foo(); + +fn foo() -> [Field; 100] { ... } +``` + +This is usually fine since Noir will generally optimize any function call that does not +refer to a program input into a constant. It should be kept in mind however, if the called +function performs side-effects like `println`, as these will still occur on each use. diff --git a/test_programs/execution_success/global_consts/src/main.nr b/test_programs/execution_success/global_consts/src/main.nr index 70c7a745a22..4b22940b3d1 100644 --- a/test_programs/execution_success/global_consts/src/main.nr +++ b/test_programs/execution_success/global_consts/src/main.nr @@ -5,7 +5,10 @@ global M: Field = 32; global L: Field = 10; // Unused globals currently allowed global N: Field = 5; global T_LEN = 2; // Type inference is allowed on globals -//global N: Field = 5; // Uncomment to see duplicate globals error + +// Globals can reference other globals +global DERIVED = M + L; + struct Dummy { x: [Field; N], y: [Field; foo::MAGIC_NUMBER] @@ -70,6 +73,7 @@ fn main( foo::from_foo(d); baz::from_baz(c); + assert(DERIVED == M + L); } fn multiplyByM(x: Field) -> Field {