diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 6ce8a70a0c5..ed4d17cadd8 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -500,7 +500,7 @@ impl Item { noir_trait_impl.accept(self.span, visitor); } ItemKind::Impl(type_impl) => type_impl.accept(self.span, visitor), - ItemKind::Global(let_statement) => { + ItemKind::Global(let_statement, _visibility) => { if visitor.visit_global(let_statement, self.span) { let_statement.accept(visitor); } diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index ca441758322..560be895628 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -439,11 +439,12 @@ impl<'context> Elaborator<'context> { resolved_trait_generics: Vec::new(), }); } - TopLevelStatementKind::Global(global) => { + TopLevelStatementKind::Global(global, visibility) => { let (global, error) = dc_mod::collect_global( self.interner, self.def_maps.get_mut(&self.crate_id).unwrap(), Documented::new(global, item.doc_comments), + visibility, self.file, self.local_module, self.crate_id, 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 132b62f84b7..162d39e0adb 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -139,15 +139,16 @@ impl<'a> ModCollector<'a> { fn collect_globals( &mut self, context: &mut Context, - globals: Vec>, + globals: Vec<(Documented, ItemVisibility)>, crate_id: CrateId, ) -> Vec<(CompilationError, fm::FileId)> { let mut errors = vec![]; - for global in globals { + for (global, visibility) in globals { let (global, error) = collect_global( &mut context.def_interner, &mut self.def_collector.def_map, global, + visibility, self.file_id, self.module_id, crate_id, @@ -515,7 +516,7 @@ impl<'a> ModCollector<'a> { if let Err((first_def, second_def)) = self.def_collector.def_map.modules [trait_id.0.local_id.0] - .declare_global(name.clone(), global_id) + .declare_global(name.clone(), ItemVisibility::Public, global_id) { let error = DefCollectorErrorKind::Duplicate { typ: DuplicateType::TraitAssociatedConst, @@ -1160,6 +1161,7 @@ pub(crate) fn collect_global( interner: &mut NodeInterner, def_map: &mut CrateDefMap, global: Documented, + visibility: ItemVisibility, file_id: FileId, module_id: LocalModuleId, crate_id: CrateId, @@ -1180,7 +1182,15 @@ pub(crate) fn collect_global( ); // Add the statement to the scope so its path can be looked up later - let result = def_map.modules[module_id.0].declare_global(name, global_id); + let result = def_map.modules[module_id.0].declare_global(name.clone(), visibility, global_id); + + let parent_module_id = ModuleId { krate: crate_id, local_id: module_id }; + interner.usage_tracker.add_unused_item( + parent_module_id, + name, + UnusedItem::Global(global_id), + visibility, + ); let error = result.err().map(|(first_def, second_def)| { let err = 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 5bbff85079e..149b1977925 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -96,8 +96,13 @@ impl ModuleData { self.definitions.remove_definition(name); } - pub fn declare_global(&mut self, name: Ident, id: GlobalId) -> Result<(), (Ident, Ident)> { - self.declare(name, ItemVisibility::Public, id.into(), None) + pub fn declare_global( + &mut self, + name: Ident, + visibility: ItemVisibility, + id: GlobalId, + ) -> Result<(), (Ident, Ident)> { + self.declare(name, visibility, id.into(), None) } pub fn declare_struct( diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index 968af82a8b3..1b6d573560d 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -47,7 +47,7 @@ pub enum TopLevelStatementKind { Impl(TypeImpl), TypeAlias(NoirTypeAlias), SubModule(ParsedSubModule), - Global(LetStatement), + Global(LetStatement, ItemVisibility), InnerAttribute(SecondaryAttribute), Error, } @@ -64,7 +64,7 @@ impl TopLevelStatementKind { TopLevelStatementKind::Impl(i) => Some(ItemKind::Impl(i)), TopLevelStatementKind::TypeAlias(t) => Some(ItemKind::TypeAlias(t)), TopLevelStatementKind::SubModule(s) => Some(ItemKind::Submodules(s)), - TopLevelStatementKind::Global(c) => Some(ItemKind::Global(c)), + TopLevelStatementKind::Global(c, visibility) => Some(ItemKind::Global(c, visibility)), TopLevelStatementKind::InnerAttribute(a) => Some(ItemKind::InnerAttribute(a)), TopLevelStatementKind::Error => None, } @@ -249,7 +249,7 @@ pub struct SortedModule { pub trait_impls: Vec, pub impls: Vec, pub type_aliases: Vec>, - pub globals: Vec>, + pub globals: Vec<(Documented, ItemVisibility)>, /// Module declarations like `mod foo;` pub module_decls: Vec>, @@ -271,7 +271,7 @@ impl std::fmt::Display for SortedModule { write!(f, "{import}")?; } - for global_const in &self.globals { + for (global_const, _visibility) in &self.globals { write!(f, "{global_const}")?; } @@ -321,7 +321,9 @@ impl ParsedModule { ItemKind::TypeAlias(type_alias) => { module.push_type_alias(type_alias, item.doc_comments); } - ItemKind::Global(global) => module.push_global(global, item.doc_comments), + ItemKind::Global(global, visibility) => { + module.push_global(global, visibility, item.doc_comments); + } ItemKind::ModuleDecl(mod_name) => { module.push_module_decl(mod_name, item.doc_comments); } @@ -354,7 +356,7 @@ pub enum ItemKind { TraitImpl(NoirTraitImpl), Impl(TypeImpl), TypeAlias(NoirTypeAlias), - Global(LetStatement), + Global(LetStatement, ItemVisibility), ModuleDecl(ModuleDeclaration), Submodules(ParsedSubModule), InnerAttribute(SecondaryAttribute), @@ -438,8 +440,13 @@ impl SortedModule { self.submodules.push(Documented::new(submodule, doc_comments)); } - fn push_global(&mut self, global: LetStatement, doc_comments: Vec) { - self.globals.push(Documented::new(global, doc_comments)); + fn push_global( + &mut self, + global: LetStatement, + visibility: ItemVisibility, + doc_comments: Vec, + ) { + self.globals.push((Documented::new(global, doc_comments), visibility)); } } @@ -526,11 +533,10 @@ impl std::fmt::Display for TopLevelStatementKind { TopLevelStatementKind::Function(fun) => fun.fmt(f), TopLevelStatementKind::Module(m) => m.fmt(f), TopLevelStatementKind::Import(tree, visibility) => { - if visibility == &ItemVisibility::Private { - write!(f, "use {tree}") - } else { - write!(f, "{visibility} use {tree}") + if visibility != &ItemVisibility::Private { + write!(f, "{visibility} ")?; } + write!(f, "use {tree}") } TopLevelStatementKind::Trait(t) => t.fmt(f), TopLevelStatementKind::TraitImpl(i) => i.fmt(f), @@ -538,7 +544,12 @@ impl std::fmt::Display for TopLevelStatementKind { TopLevelStatementKind::Impl(i) => i.fmt(f), TopLevelStatementKind::TypeAlias(t) => t.fmt(f), TopLevelStatementKind::SubModule(s) => s.fmt(f), - TopLevelStatementKind::Global(c) => c.fmt(f), + TopLevelStatementKind::Global(c, visibility) => { + if visibility != &ItemVisibility::Private { + write!(f, "{visibility} ")?; + } + c.fmt(f) + } TopLevelStatementKind::InnerAttribute(a) => write!(f, "#![{}]", a), TopLevelStatementKind::Error => write!(f, "error"), } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 3f9a1d7a08b..d902f0a3b5a 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -220,6 +220,7 @@ fn implementation() -> impl NoirParser { /// global_declaration: 'global' ident global_type_annotation '=' literal fn global_declaration() -> impl NoirParser { let p = attributes::attributes() + .then(item_visibility()) .then(maybe_comp_time()) .then(spanned(keyword(Keyword::Mut)).or_not()) .then_ignore(keyword(Keyword::Global).labelled(ParsingRuleLabel::Global)) @@ -229,7 +230,9 @@ fn global_declaration() -> impl NoirParser { let p = then_commit_ignore(p, just(Token::Assign)); let p = then_commit(p, expression()); p.validate( - |(((((attributes, comptime), mutable), mut pattern), r#type), expression), span, emit| { + |((((((attributes, visibility), comptime), mutable), mut pattern), r#type), expression), + span, + emit| { let global_attributes = attributes::validate_secondary_attributes(attributes, span, emit); @@ -239,10 +242,19 @@ fn global_declaration() -> impl NoirParser { let span = mut_span.merge(pattern.span()); pattern = Pattern::Mutable(Box::new(pattern), span, false); } - LetStatement { pattern, r#type, comptime, expression, attributes: global_attributes } + ( + LetStatement { + pattern, + r#type, + comptime, + expression, + attributes: global_attributes, + }, + visibility, + ) }, ) - .map(TopLevelStatementKind::Global) + .map(|(let_statement, visibility)| TopLevelStatementKind::Global(let_statement, visibility)) } /// submodule: 'mod' ident '{' module '}' diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 4c91c90bc31..9b55e689eed 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1376,7 +1376,9 @@ fn ban_mutable_globals() { // Mutable globals are only allowed in a comptime context let src = r#" mut global FOO: Field = 0; - fn main() {} + fn main() { + let _ = FOO; // silence FOO never used warning + } "#; assert_eq!(get_program_errors(src).len(), 1); } diff --git a/compiler/noirc_frontend/src/tests/unused_items.rs b/compiler/noirc_frontend/src/tests/unused_items.rs index be3b2996e5a..a4d379b6358 100644 --- a/compiler/noirc_frontend/src/tests/unused_items.rs +++ b/compiler/noirc_frontend/src/tests/unused_items.rs @@ -239,3 +239,26 @@ fn errors_if_type_alias_aliases_more_private_type_in_generic() { assert_eq!(typ, "Foo"); assert_eq!(item, "Bar"); } + +#[test] +fn warns_on_unused_global() { + let src = r#" + global foo = 1; + global bar = 1; + + fn main() { + let _ = bar; + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item }) = &errors[0].0 + else { + panic!("Expected an unused item warning"); + }; + + assert_eq!(ident.to_string(), "foo"); + assert_eq!(item.item_type(), "global"); +} diff --git a/compiler/noirc_frontend/src/usage_tracker.rs b/compiler/noirc_frontend/src/usage_tracker.rs index cd1041cc36f..275ca1f964b 100644 --- a/compiler/noirc_frontend/src/usage_tracker.rs +++ b/compiler/noirc_frontend/src/usage_tracker.rs @@ -4,7 +4,7 @@ use crate::{ ast::{Ident, ItemVisibility}, hir::def_map::ModuleId, macros_api::StructId, - node_interner::{FuncId, TraitId, TypeAliasId}, + node_interner::{FuncId, GlobalId, TraitId, TypeAliasId}, }; #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -14,6 +14,7 @@ pub enum UnusedItem { Struct(StructId), Trait(TraitId), TypeAlias(TypeAliasId), + Global(GlobalId), } impl UnusedItem { @@ -24,6 +25,7 @@ impl UnusedItem { UnusedItem::Struct(_) => "struct", UnusedItem::Trait(_) => "trait", UnusedItem::TypeAlias(_) => "type alias", + UnusedItem::Global(_) => "global", } } } diff --git a/docs/docs/noir/concepts/globals.md b/docs/docs/noir/concepts/globals.md index 97a92a86e72..1145c55dfc7 100644 --- a/docs/docs/noir/concepts/globals.md +++ b/docs/docs/noir/concepts/globals.md @@ -70,3 +70,13 @@ 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. + +### Visibility + +By default, like functions, globals are private to the module the exist in. You can use `pub` +to make the global public or `pub(crate)` to make it public to just its crate: + +```rust +// This global is now public +pub global N = 5; +``` \ No newline at end of file diff --git a/noir_stdlib/src/field/bn254.nr b/noir_stdlib/src/field/bn254.nr index 0aa5ca0717b..8ff62062d5c 100644 --- a/noir_stdlib/src/field/bn254.nr +++ b/noir_stdlib/src/field/bn254.nr @@ -4,7 +4,7 @@ use crate::runtime::is_unconstrained; global PLO: Field = 53438638232309528389504892708671455233; global PHI: Field = 64323764613183177041862057485226039389; -global TWO_POW_128: Field = 0x100000000000000000000000000000000; +pub(crate) global TWO_POW_128: Field = 0x100000000000000000000000000000000; // Decomposes a single field into two 16 byte fields. fn compute_decomposition(x: Field) -> (Field, Field) { diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index 7d5e1886072..b2a689919bd 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -276,7 +276,7 @@ impl super::FmtVisitor<'_> { ItemKind::Struct(_) | ItemKind::Trait(_) | ItemKind::TypeAlias(_) - | ItemKind::Global(_) + | ItemKind::Global(..) | ItemKind::ModuleDecl(_) | ItemKind::InnerAttribute(_) => { self.push_rewrite(self.slice(span).to_string(), span);