From 80a8e38148b5046e83da567fe3be9a075daa6915 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 15:17:01 -0300 Subject: [PATCH 01/20] Allow secondary attributes on modules and contracts --- compiler/noirc_frontend/src/ast/statement.rs | 1 + compiler/noirc_frontend/src/parser/mod.rs | 3 +- compiler/noirc_frontend/src/parser/parser.rs | 53 ++++++++++++++++---- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 2fc08e1aea1..48d612226bd 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -292,6 +292,7 @@ pub trait Recoverable { #[derive(Debug, PartialEq, Eq, Clone)] pub struct ModuleDeclaration { pub ident: Ident, + pub attributes: Vec, } impl std::fmt::Display for ModuleDeclaration { diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index 11944cd3304..b12de60d0a2 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -15,7 +15,7 @@ use crate::ast::{ Expression, Ident, ImportStatement, LetStatement, ModuleDeclaration, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Recoverable, StatementKind, TypeImpl, UseTree, }; -use crate::token::{Keyword, Token}; +use crate::token::{Keyword, SecondaryAttribute, Token}; use chumsky::prelude::*; use chumsky::primitive::Container; @@ -341,6 +341,7 @@ pub enum ItemKind { pub struct ParsedSubModule { pub name: Ident, pub contents: ParsedModule, + pub attributes: Vec, pub is_contract: bool, } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 5f0ef8909e7..504f3d8afb3 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -26,6 +26,7 @@ use self::path::as_trait_path; use self::primitives::{keyword, macro_quote_marker, mutable_reference, variable}; use self::types::{generic_type_args, maybe_comp_time}; +use attributes::{attributes, validate_secondary_attributes}; pub use types::parse_type; use super::{ @@ -285,25 +286,39 @@ fn global_declaration() -> impl NoirParser { /// submodule: 'mod' ident '{' module '}' fn submodule(module_parser: impl NoirParser) -> impl NoirParser { - keyword(Keyword::Mod) - .ignore_then(ident()) + attributes() + .then_ignore(keyword(Keyword::Mod)) + .then(ident()) .then_ignore(just(Token::LeftBrace)) .then(module_parser) .then_ignore(just(Token::RightBrace)) - .map(|(name, contents)| { - TopLevelStatement::SubModule(ParsedSubModule { name, contents, is_contract: false }) + .validate(|((attributes, name), contents), span, emit| { + let attributes = validate_secondary_attributes(attributes, span, emit); + TopLevelStatement::SubModule(ParsedSubModule { + name, + contents, + attributes, + is_contract: false, + }) }) } /// contract: 'contract' ident '{' module '}' fn contract(module_parser: impl NoirParser) -> impl NoirParser { - keyword(Keyword::Contract) - .ignore_then(ident()) + attributes() + .then_ignore(keyword(Keyword::Contract)) + .then(ident()) .then_ignore(just(Token::LeftBrace)) .then(module_parser) .then_ignore(just(Token::RightBrace)) - .map(|(name, contents)| { - TopLevelStatement::SubModule(ParsedSubModule { name, contents, is_contract: true }) + .validate(|((attributes, name), contents), span, emit| { + let attributes = validate_secondary_attributes(attributes, span, emit); + TopLevelStatement::SubModule(ParsedSubModule { + name, + contents, + attributes, + is_contract: true, + }) }) } @@ -432,9 +447,12 @@ fn optional_type_annotation<'a>() -> impl NoirParser + 'a { } fn module_declaration() -> impl NoirParser { - keyword(Keyword::Mod) - .ignore_then(ident()) - .map(|ident| TopLevelStatement::Module(ModuleDeclaration { ident })) + attributes().then_ignore(keyword(Keyword::Mod)).then(ident()).validate( + |(attributes, ident), span, emit| { + let attributes = validate_secondary_attributes(attributes, span, emit); + TopLevelStatement::Module(ModuleDeclaration { ident, attributes }) + }, + ) } fn use_statement() -> impl NoirParser { @@ -1517,9 +1535,22 @@ mod test { #[test] fn parse_module_declaration() { parse_with(module_declaration(), "mod foo").unwrap(); + parse_with(module_declaration(), "#[attr] mod foo").unwrap(); parse_with(module_declaration(), "mod 1").unwrap_err(); } + #[test] + fn parse_submodule_declaration() { + parse_with(submodule(module()), "mod foo {}").unwrap(); + parse_with(submodule(module()), "#[attr] mod foo {}").unwrap(); + } + + #[test] + fn parse_contract() { + parse_with(contract(module()), "contract foo {}").unwrap(); + parse_with(contract(module()), "#[attr] contract foo {}").unwrap(); + } + #[test] fn parse_use() { let valid_use_statements = [ From cd41b51b3e5a3788a26533d8c123b3f2ff02208e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 15:47:22 -0300 Subject: [PATCH 02/20] Let the formatter handle attributes --- tooling/nargo_fmt/src/visitor/item.rs | 6 ++++++ tooling/nargo_fmt/tests/expected/module.nr | 5 +++++ tooling/nargo_fmt/tests/input/module.nr | 5 +++++ 3 files changed, 16 insertions(+) diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index 94a32449ebe..3241c12f3f0 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -165,6 +165,12 @@ impl super::FmtVisitor<'_> { continue; } + for attribute in module.attributes { + self.push_str(&attribute.to_string()); + self.push_str("\n"); + self.push_str(&self.indent.to_string()); + } + let name = module.name; let after_brace = self.span_after(span, Token::LeftBrace).start(); self.last_position = after_brace; diff --git a/tooling/nargo_fmt/tests/expected/module.nr b/tooling/nargo_fmt/tests/expected/module.nr index e419543dbc4..1dfd391ec99 100644 --- a/tooling/nargo_fmt/tests/expected/module.nr +++ b/tooling/nargo_fmt/tests/expected/module.nr @@ -13,6 +13,8 @@ mod a { Data2 { a } } + #[custom] + #[another_custom] mod tests { #[test] fn test() { @@ -20,4 +22,7 @@ mod a { data2(1); } } + + #[attr] + mod baz; } diff --git a/tooling/nargo_fmt/tests/input/module.nr b/tooling/nargo_fmt/tests/input/module.nr index e419543dbc4..1dfd391ec99 100644 --- a/tooling/nargo_fmt/tests/input/module.nr +++ b/tooling/nargo_fmt/tests/input/module.nr @@ -13,6 +13,8 @@ mod a { Data2 { a } } + #[custom] + #[another_custom] mod tests { #[test] fn test() { @@ -20,4 +22,7 @@ mod a { data2(1); } } + + #[attr] + mod baz; } From 135dda1a499238429265e1a394e93582e521c0d0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 16:17:30 -0300 Subject: [PATCH 03/20] Parse inner attributes --- compiler/noirc_frontend/src/lexer/errors.rs | 8 +++++ compiler/noirc_frontend/src/lexer/lexer.rs | 34 +++++++++++++++++-- compiler/noirc_frontend/src/lexer/token.rs | 11 ++++-- compiler/noirc_frontend/src/parser/mod.rs | 1 + compiler/noirc_frontend/src/parser/parser.rs | 25 +++++++++----- .../src/parser/parser/attributes.rs | 13 +++++++ tooling/nargo_fmt/src/visitor/item.rs | 2 +- 7 files changed, 80 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_frontend/src/lexer/errors.rs b/compiler/noirc_frontend/src/lexer/errors.rs index be5180a777b..2440109af15 100644 --- a/compiler/noirc_frontend/src/lexer/errors.rs +++ b/compiler/noirc_frontend/src/lexer/errors.rs @@ -20,6 +20,8 @@ pub enum LexerErrorKind { IntegerLiteralTooLarge { span: Span, limit: String }, #[error("{:?} is not a valid attribute", found)] MalformedFuncAttribute { span: Span, found: String }, + #[error("{:?} is not a valid inner attribute", found)] + InvalidInnerAttribute { span: Span, found: String }, #[error("Logical and used instead of bitwise and")] LogicalAnd { span: Span }, #[error("Unterminated block comment")] @@ -57,6 +59,7 @@ impl LexerErrorKind { LexerErrorKind::InvalidIntegerLiteral { span, .. } => *span, LexerErrorKind::IntegerLiteralTooLarge { span, .. } => *span, LexerErrorKind::MalformedFuncAttribute { span, .. } => *span, + LexerErrorKind::InvalidInnerAttribute { span, .. } => *span, LexerErrorKind::LogicalAnd { span } => *span, LexerErrorKind::UnterminatedBlockComment { span } => *span, LexerErrorKind::UnterminatedStringLiteral { span } => *span, @@ -103,6 +106,11 @@ impl LexerErrorKind { format!(" {found} is not a valid attribute"), *span, ), + LexerErrorKind::InvalidInnerAttribute { span, found } => ( + "Invalid inner attribute".to_string(), + format!(" {found} is not a valid inner attribute"), + *span, + ), LexerErrorKind::LogicalAnd { span } => ( "Noir has no logical-and (&&) operator since short-circuiting is much less efficient when compiling to circuits".to_string(), "Try `&` instead, or use `if` only if you require short-circuiting".to_string(), diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index 0afcb02caac..171ff823249 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -286,6 +286,13 @@ impl<'a> Lexer<'a> { fn eat_attribute(&mut self) -> SpannedTokenResult { let start = self.position; + let is_inner = if self.peek_char_is('!') { + self.next_char(); + true + } else { + false + }; + if !self.peek_char_is('[') { return Err(LexerErrorKind::UnexpectedCharacter { span: Span::single_char(self.position), @@ -309,8 +316,19 @@ impl<'a> Lexer<'a> { let end = self.position; let attribute = Attribute::lookup_attribute(&word, Span::inclusive(start, end))?; - - Ok(attribute.into_span(start, end)) + if is_inner { + match attribute { + Attribute::Function(attribute) => Err(LexerErrorKind::InvalidInnerAttribute { + span: Span::from(start..end), + found: attribute.to_string(), + }), + Attribute::Secondary(attribute) => { + Ok(Token::InnerAttribute(attribute).into_span(start, end)) + } + } + } else { + Ok(Token::Attribute(attribute).into_span(start, end)) + } } //XXX(low): Can increase performance if we use iterator semantic and utilize some of the methods on String. See below @@ -898,6 +916,18 @@ mod tests { assert_eq!(sub_string, "test(invalid_scope)"); } + #[test] + fn test_inner_attribute() { + let input = r#"#![test]"#; + let mut lexer = Lexer::new(input); + + let token = lexer.next_token().unwrap(); + assert_eq!( + token.token(), + &Token::InnerAttribute(SecondaryAttribute::Custom("test".to_string())) + ); + } + #[test] fn test_int_type() { let input = "u16 i16 i108 u104.5"; diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 585e22ce92b..4e56ee8cd21 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -27,6 +27,7 @@ pub enum BorrowedToken<'input> { Keyword(Keyword), IntType(IntType), Attribute(Attribute), + InnerAttribute(SecondaryAttribute), LineComment(&'input str, Option), BlockComment(&'input str, Option), Quote(&'input Tokens), @@ -132,6 +133,7 @@ pub enum Token { Keyword(Keyword), IntType(IntType), Attribute(Attribute), + InnerAttribute(SecondaryAttribute), LineComment(String, Option), BlockComment(String, Option), // A `quote { ... }` along with the tokens in its token stream. @@ -244,6 +246,7 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { Token::RawStr(ref b, hashes) => BorrowedToken::RawStr(b, *hashes), Token::Keyword(k) => BorrowedToken::Keyword(*k), Token::Attribute(ref a) => BorrowedToken::Attribute(a.clone()), + Token::InnerAttribute(ref a) => BorrowedToken::InnerAttribute(a.clone()), Token::LineComment(ref s, _style) => BorrowedToken::LineComment(s, *_style), Token::BlockComment(ref s, _style) => BorrowedToken::BlockComment(s, *_style), Token::Quote(stream) => BorrowedToken::Quote(stream), @@ -363,6 +366,7 @@ impl fmt::Display for Token { } Token::Keyword(k) => write!(f, "{k}"), Token::Attribute(ref a) => write!(f, "{a}"), + Token::InnerAttribute(ref a) => write!(f, "#![{a}]"), Token::LineComment(ref s, _style) => write!(f, "//{s}"), Token::BlockComment(ref s, _style) => write!(f, "/*{s}*/"), Token::Quote(ref stream) => { @@ -428,6 +432,7 @@ pub enum TokenKind { Literal, Keyword, Attribute, + InnerAttribute, Quote, QuotedType, InternedExpr, @@ -445,6 +450,7 @@ impl fmt::Display for TokenKind { TokenKind::Literal => write!(f, "literal"), TokenKind::Keyword => write!(f, "keyword"), TokenKind::Attribute => write!(f, "attribute"), + TokenKind::InnerAttribute => write!(f, "inner attribute"), TokenKind::Quote => write!(f, "quote"), TokenKind::QuotedType => write!(f, "quoted type"), TokenKind::InternedExpr => write!(f, "interned expr"), @@ -467,6 +473,7 @@ impl Token { | Token::FmtStr(_) => TokenKind::Literal, Token::Keyword(_) => TokenKind::Keyword, Token::Attribute(_) => TokenKind::Attribute, + Token::InnerAttribute(_) => TokenKind::InnerAttribute, Token::UnquoteMarker(_) => TokenKind::UnquoteMarker, Token::Quote(_) => TokenKind::Quote, Token::QuotedType(_) => TokenKind::QuotedType, @@ -697,7 +704,7 @@ impl fmt::Display for Attribute { impl Attribute { /// If the string is a fixed attribute return that, else /// return the custom attribute - pub(crate) fn lookup_attribute(word: &str, span: Span) -> Result { + pub(crate) fn lookup_attribute(word: &str, span: Span) -> Result { let word_segments: Vec<&str> = word .split(|c| c == '(' || c == ')') .filter(|string_segment| !string_segment.is_empty()) @@ -774,7 +781,7 @@ impl Attribute { } }; - Ok(Token::Attribute(attribute)) + Ok(attribute) } } diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index b12de60d0a2..71a0d78f05b 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -290,6 +290,7 @@ impl std::fmt::Display for SortedModule { #[derive(Clone, Debug, Default)] pub struct ParsedModule { pub items: Vec, + pub attributes: Vec, } impl ParsedModule { diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 504f3d8afb3..544bdd3ab5a 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -26,7 +26,7 @@ use self::path::as_trait_path; use self::primitives::{keyword, macro_quote_marker, mutable_reference, variable}; use self::types::{generic_type_args, maybe_comp_time}; -use attributes::{attributes, validate_secondary_attributes}; +use attributes::{attributes, inner_attributes, validate_secondary_attributes}; pub use types::parse_type; use super::{ @@ -90,7 +90,7 @@ pub fn parse_program(source_program: &str) -> (ParsedModule, Vec) { let (module, mut parsing_errors) = program().parse_recovery_verbose(tokens); parsing_errors.extend(lexing_errors.into_iter().map(Into::into)); - let parsed_module = module.unwrap_or(ParsedModule { items: vec![] }); + let parsed_module = module.unwrap_or_default(); if cfg!(feature = "experimental_parser") { for parsed_item in &parsed_module.items { @@ -174,13 +174,20 @@ fn program() -> impl NoirParser { /// | %empty fn module() -> impl NoirParser { recursive(|module_parser| { - empty() - .to(ParsedModule::default()) - .then(spanned(top_level_statement(module_parser)).repeated()) - .foldl(|mut program, (statement, span)| { - if let Some(kind) = statement.into_item_kind() { - program.items.push(Item { kind, span }); - } + inner_attributes() + .then( + empty() + .to(ParsedModule::default()) + .then(spanned(top_level_statement(module_parser)).repeated()) + .foldl(|mut program, (statement, span)| { + if let Some(kind) = statement.into_item_kind() { + program.items.push(Item { kind, span }); + } + program + }), + ) + .map(|(attributes, mut program)| { + program.attributes = attributes; program }) }) diff --git a/compiler/noirc_frontend/src/parser/parser/attributes.rs b/compiler/noirc_frontend/src/parser/parser/attributes.rs index 47add6f82e0..2354708aa75 100644 --- a/compiler/noirc_frontend/src/parser/parser/attributes.rs +++ b/compiler/noirc_frontend/src/parser/parser/attributes.rs @@ -67,3 +67,16 @@ pub(super) fn validate_secondary_attributes( struct_attributes } + +fn inner_attribute() -> impl NoirParser { + token_kind(TokenKind::InnerAttribute).map(|token| match token { + Token::InnerAttribute(attribute) => attribute, + _ => unreachable!( + "Parser should have already errored due to token not being an inner attribute" + ), + }) +} + +pub(super) fn inner_attributes() -> impl NoirParser> { + inner_attribute().repeated() +} diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index 3241c12f3f0..9479f9ec138 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -166,7 +166,7 @@ impl super::FmtVisitor<'_> { } for attribute in module.attributes { - self.push_str(&attribute.to_string()); + self.push_str(attribute.as_ref()); self.push_str("\n"); self.push_str(&self.indent.to_string()); } From d3b30ea41c2f95c9f2b555f8460de0ee263a8cc7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 16:20:45 -0300 Subject: [PATCH 04/20] Make sure inner attributes are formatted well --- tooling/nargo_fmt/tests/expected/module.nr | 2 ++ tooling/nargo_fmt/tests/input/module.nr | 3 +++ 2 files changed, 5 insertions(+) diff --git a/tooling/nargo_fmt/tests/expected/module.nr b/tooling/nargo_fmt/tests/expected/module.nr index 1dfd391ec99..79e4039347a 100644 --- a/tooling/nargo_fmt/tests/expected/module.nr +++ b/tooling/nargo_fmt/tests/expected/module.nr @@ -1,3 +1,5 @@ +#![inner] +#![inner2] mod a { mod b { struct Data { diff --git a/tooling/nargo_fmt/tests/input/module.nr b/tooling/nargo_fmt/tests/input/module.nr index 1dfd391ec99..1bfef1d068b 100644 --- a/tooling/nargo_fmt/tests/input/module.nr +++ b/tooling/nargo_fmt/tests/input/module.nr @@ -1,3 +1,6 @@ +#![inner] +#![inner2] + mod a { mod b { struct Data { From 89ea45dbc1e8d715ef8f1f50ef93094b451369d4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 16:36:55 -0300 Subject: [PATCH 05/20] Collect ModuleData attributes --- .../noirc_frontend/src/hir/comptime/tests.rs | 2 +- .../src/hir/def_collector/dc_mod.rs | 16 +++++++++++++--- compiler/noirc_frontend/src/hir/def_map/mod.rs | 2 +- .../src/hir/def_map/module_data.rs | 11 ++++++++++- compiler/noirc_frontend/src/parser/mod.rs | 4 ++++ compiler/noirc_frontend/src/tests.rs | 3 ++- 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index 4c1adf9fca0..d36971c5dc3 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -23,7 +23,7 @@ fn interpret_helper(src: &str) -> Result { let module_id = LocalModuleId(Index::unsafe_zeroed()); let mut modules = noirc_arena::Arena::default(); let location = Location::new(Default::default(), file); - let root = LocalModuleId(modules.insert(ModuleData::new(None, location, false))); + let root = LocalModuleId(modules.insert(ModuleData::new(None, location, Vec::new(), false))); assert_eq!(root, module_id); let file_manager = FileManager::new(&PathBuf::new()); 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 459c4869379..c2df740dc90 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -17,6 +17,7 @@ use crate::ast::{ use crate::hir::resolution::errors::ResolverError; use crate::macros_api::{Expression, NodeInterner, UnresolvedType, UnresolvedTypeData}; use crate::node_interner::ModuleAttributes; +use crate::token::SecondaryAttribute; use crate::{ graph::CrateId, hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait}, @@ -63,7 +64,7 @@ pub fn collect_defs( for decl in ast.module_decls { errors.extend(collector.parse_module_declaration( context, - &decl, + decl, crate_id, macro_processors, )); @@ -303,6 +304,7 @@ impl<'a> ModCollector<'a> { context, &name, Location::new(name.span(), self.file_id), + Vec::new(), false, false, ) { @@ -435,6 +437,7 @@ impl<'a> ModCollector<'a> { context, &name, Location::new(name.span(), self.file_id), + Vec::new(), false, false, ) { @@ -624,10 +627,15 @@ impl<'a> ModCollector<'a> { ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; for submodule in submodules { + let mut attributes = Vec::new(); + attributes.extend(submodule.attributes); + attributes.extend(submodule.contents.attributes.clone()); + match self.push_child_module( context, &submodule.name, Location::new(submodule.name.span(), file_id), + attributes, true, submodule.is_contract, ) { @@ -656,7 +664,7 @@ impl<'a> ModCollector<'a> { fn parse_module_declaration( &mut self, context: &mut Context, - mod_decl: &ModuleDeclaration, + mod_decl: ModuleDeclaration, crate_id: CrateId, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { @@ -719,6 +727,7 @@ impl<'a> ModCollector<'a> { context, &mod_decl.ident, Location::new(Span::empty(0), child_file_id), + mod_decl.attributes, true, false, ) { @@ -750,6 +759,7 @@ impl<'a> ModCollector<'a> { context: &mut Context, mod_name: &Ident, mod_location: Location, + attributes: Vec, add_to_parent_scope: bool, is_contract: bool, ) -> Result { @@ -763,7 +773,7 @@ impl<'a> ModCollector<'a> { // Eventually the location put in `ModuleData` is used for codelenses about `contract`s, // so we keep using `location` so that it continues to work as usual. let location = Location::new(mod_name.span(), mod_location.file); - let new_module = ModuleData::new(parent, location, is_contract); + let new_module = ModuleData::new(parent, location, attributes, is_contract); let module_id = self.def_collector.def_map.modules.insert(new_module); let modules = &mut self.def_collector.def_map.modules; diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 758b4cf6e5c..258d0b01502 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -111,7 +111,7 @@ impl CrateDefMap { // Allocate a default Module for the root, giving it a ModuleId let mut modules: Arena = Arena::default(); let location = Location::new(Default::default(), root_file_id); - let root = modules.insert(ModuleData::new(None, location, false)); + let root = modules.insert(ModuleData::new(None, location, ast.attributes.clone(), false)); let def_map = CrateDefMap { root: LocalModuleId(root), 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 7b14db8be77..bd6fe17ed3a 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -5,6 +5,7 @@ use noirc_errors::Location; use super::{ItemScope, LocalModuleId, ModuleDefId, ModuleId, PerNs}; use crate::ast::{Ident, ItemVisibility}; use crate::node_interner::{FuncId, GlobalId, StructId, TraitId, TypeAliasId}; +use crate::token::SecondaryAttribute; /// Contains the actual contents of a module: its parent (if one exists), /// children, and scope with all definitions defined within the scope. @@ -25,13 +26,20 @@ pub struct ModuleData { /// True if this module is a `contract Foo { ... }` module containing contract functions pub is_contract: bool, + pub attributes: Vec, + /// List of all unused imports. Each time something is imported into this module it's added /// to this set. When it's used, it's removed. At the end of the program only unused imports remain. unused_imports: HashSet, } impl ModuleData { - pub fn new(parent: Option, location: Location, is_contract: bool) -> ModuleData { + pub fn new( + parent: Option, + location: Location, + attributes: Vec, + is_contract: bool, + ) -> ModuleData { ModuleData { parent, children: HashMap::new(), @@ -39,6 +47,7 @@ impl ModuleData { definitions: ItemScope::default(), location, is_contract, + attributes, unused_imports: HashSet::new(), } } diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index 71a0d78f05b..b2294b207a1 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -246,6 +246,8 @@ pub struct SortedModule { /// Full submodules as in `mod foo { ... definitions ... }` pub submodules: Vec, + + pub attributes: Vec, } impl std::fmt::Display for SortedModule { @@ -351,6 +353,7 @@ impl ParsedSubModule { SortedSubModule { name: self.name, contents: self.contents.into_sorted(), + attributes: self.attributes, is_contract: self.is_contract, } } @@ -372,6 +375,7 @@ impl std::fmt::Display for SortedSubModule { pub struct SortedSubModule { pub name: Ident, pub contents: SortedModule, + pub attributes: Vec, pub is_contract: bool, } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 870c781b89d..62dee6110f6 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -67,7 +67,8 @@ pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec<(Compilation // Allocate a default Module for the root, giving it a ModuleId let mut modules: Arena = Arena::default(); let location = Location::new(Default::default(), root_file_id); - let root = modules.insert(ModuleData::new(None, location, false)); + let root = + modules.insert(ModuleData::new(None, location, program.attributes.clone(), false)); let def_map = CrateDefMap { root: LocalModuleId(root), From 3015ac8abbf30ae19e435404089d38dd4ae7ced2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 16:44:35 -0300 Subject: [PATCH 06/20] Fix formatter --- tooling/nargo_fmt/src/visitor/item.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index 9479f9ec138..cfbb3279227 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -166,8 +166,7 @@ impl super::FmtVisitor<'_> { } for attribute in module.attributes { - self.push_str(attribute.as_ref()); - self.push_str("\n"); + self.push_str(&format!("#[{}]\n", attribute.as_ref())); self.push_str(&self.indent.to_string()); } From 0382dd62577cf8c60482b2ba046516d1354f7e83 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 17:02:40 -0300 Subject: [PATCH 07/20] Add `Module::has_named_attribute` --- .../src/hir/comptime/interpreter/builtin.rs | 37 +++++++++++++++++++ .../src/hir/def_map/module_data.rs | 4 +- compiler/noirc_frontend/src/parser/mod.rs | 1 + .../docs/noir/standard_library/meta/module.md | 6 +++ noir_stdlib/src/meta/module.nr | 5 +++ .../comptime_module/src/main.nr | 11 ++++++ 6 files changed, 63 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 7a196d98f52..189630aa3ff 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -108,6 +108,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { function_def_set_return_type(self, arguments, location) } "module_functions" => module_functions(self, arguments, location), + "module_has_named_attribute" => module_has_named_attribute(self, arguments, location), "module_is_contract" => module_is_contract(self, arguments, location), "module_name" => module_name(interner, arguments, location), "modulus_be_bits" => modulus_be_bits(interner, arguments, location), @@ -1799,6 +1800,42 @@ fn module_functions( Ok(Value::Slice(func_ids, slice_type)) } +// fn has_named_attribute(self, name: Quoted) -> bool +fn module_has_named_attribute( + interpreter: &Interpreter, + arguments: Vec<(Value, Location)>, + location: Location, +) -> IResult { + let (self_argument, name) = check_two_arguments(arguments, location)?; + let module_id = get_module(self_argument)?; + let module_data = interpreter.elaborator.get_module(module_id); + let name = get_quoted(name)?; + + let attributes = &module_data.attributes; + if attributes.is_empty() { + return Ok(Value::Bool(false)); + }; + + let name = name.iter().map(|token| token.to_string()).collect::>().join(""); + + for attribute in attributes { + let parse_result = Elaborator::parse_attribute(attribute, location.file); + let Ok(Some((function, _arguments))) = parse_result else { + continue; + }; + + let ExpressionKind::Variable(path) = function.kind else { + continue; + }; + + if path.last_name() == name { + return Ok(Value::Bool(true)); + } + } + + Ok(Value::Bool(false)) +} + // fn is_contract(self) -> bool fn module_is_contract( interpreter: &Interpreter, 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 bd6fe17ed3a..b3aea8926ea 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -26,7 +26,7 @@ pub struct ModuleData { /// True if this module is a `contract Foo { ... }` module containing contract functions pub is_contract: bool, - pub attributes: Vec, + pub attributes: Vec, /// List of all unused imports. Each time something is imported into this module it's added /// to this set. When it's used, it's removed. At the end of the program only unused imports remain. @@ -40,6 +40,8 @@ impl ModuleData { attributes: Vec, is_contract: bool, ) -> ModuleData { + let attributes = attributes.iter().filter_map(|attr| attr.as_custom()); + let attributes = attributes.map(|attr| attr.to_string()).collect(); ModuleData { parent, children: HashMap::new(), diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index b2294b207a1..0f9860f05e1 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -314,6 +314,7 @@ impl ParsedModule { } } + module.attributes = self.attributes; module } } diff --git a/docs/docs/noir/standard_library/meta/module.md b/docs/docs/noir/standard_library/meta/module.md index d283f2da8b2..870e366461c 100644 --- a/docs/docs/noir/standard_library/meta/module.md +++ b/docs/docs/noir/standard_library/meta/module.md @@ -20,6 +20,12 @@ Returns the name of the module. Returns each function in the module. +### has_named_attribute + +#include_code has_named_attribute noir_stdlib/src/meta/module.nr rust + +Returns true if this module has a custom attribute with the given name. + ### is_contract #include_code is_contract noir_stdlib/src/meta/module.nr rust diff --git a/noir_stdlib/src/meta/module.nr b/noir_stdlib/src/meta/module.nr index 6ea3ca55fb1..b3f76812b8a 100644 --- a/noir_stdlib/src/meta/module.nr +++ b/noir_stdlib/src/meta/module.nr @@ -1,4 +1,9 @@ impl Module { + #[builtin(module_has_named_attribute)] + // docs:start:has_named_attribute + fn has_named_attribute(self, name: Quoted) -> bool {} + // docs:end:has_named_attribute + #[builtin(module_is_contract)] // docs:start:is_contract fn is_contract(self) -> bool {} diff --git a/test_programs/compile_success_empty/comptime_module/src/main.nr b/test_programs/compile_success_empty/comptime_module/src/main.nr index 8d834381fea..f48ba977567 100644 --- a/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -1,10 +1,14 @@ mod foo { + #![some_attribute] fn x() {} fn y() {} } contract bar {} +#[some_attribute] +mod another_module {} + fn main() { comptime { @@ -15,6 +19,8 @@ fn main() { let bar = quote { bar }.as_module().unwrap(); assert(bar.is_contract()); + let another_module = quote { another_module }.as_module().unwrap(); + // Check Module::functions assert_eq(foo.functions().len(), 2); assert_eq(bar.functions().len(), 0); @@ -22,6 +28,11 @@ fn main() { // Check Module::name assert_eq(foo.name(), quote { foo }); assert_eq(bar.name(), quote { bar }); + + // Check Module::has_named_attribute + assert(foo.has_named_attribute(quote { some_attribute })); + assert(!bar.has_named_attribute(quote { some_attribute })); + assert(another_module.has_named_attribute(quote { some_attribute })); } } From 5fc2ac6b8f6c11bf3c5ac760f574deb1dc2f17cd Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 18:04:15 -0300 Subject: [PATCH 08/20] Trying to run comptime attributes for modules --- compiler/noirc_frontend/src/ast/statement.rs | 2 +- .../noirc_frontend/src/elaborator/comptime.rs | 44 +++++++++++++++++++ .../src/hir/comptime/interpreter/builtin.rs | 6 +-- .../noirc_frontend/src/hir/comptime/tests.rs | 8 +++- .../src/hir/def_collector/dc_mod.rs | 19 ++++---- .../noirc_frontend/src/hir/def_map/mod.rs | 8 +++- .../src/hir/def_map/module_data.rs | 17 ++++--- compiler/noirc_frontend/src/parser/mod.rs | 12 ++--- compiler/noirc_frontend/src/parser/parser.rs | 8 ++-- compiler/noirc_frontend/src/tests.rs | 9 +++- .../comptime_module/src/main.nr | 11 +++++ tooling/nargo_fmt/src/visitor/item.rs | 2 +- 12 files changed, 112 insertions(+), 34 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 48d612226bd..b648c629989 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -292,7 +292,7 @@ pub trait Recoverable { #[derive(Debug, PartialEq, Eq, Clone)] pub struct ModuleDeclaration { pub ident: Ident, - pub attributes: Vec, + pub outer_attributes: Vec, } impl std::fmt::Display for ModuleDeclaration { diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 3e71f167802..7df6fef6163 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -15,6 +15,7 @@ use crate::{ }, dc_mod, }, + def_map::{LocalModuleId, ModuleId}, resolution::errors::ResolverError, }, hir_def::expr::HirIdent, @@ -380,6 +381,8 @@ impl<'context> Elaborator<'context> { ) -> CollectedItems { let mut generated_items = CollectedItems::default(); + self.run_attributes_on_modules(&mut generated_items); + for (trait_id, trait_) in traits { let attributes = &trait_.trait_def.attributes; let item = Value::TraitDefinition(*trait_id); @@ -402,6 +405,43 @@ impl<'context> Elaborator<'context> { generated_items } + fn run_attributes_on_modules(&mut self, generated_items: &mut CollectedItems) { + let def_map = &self.def_maps[&self.crate_id]; + let mut data = Vec::new(); + + for (index, module_data) in def_map.modules.iter() { + if !module_data.outer_attributes.is_empty() { + let parent = def_map.modules()[index].parent.unwrap(); + let module_data = &def_map.modules()[parent.0]; + let module_id = ModuleId { krate: self.crate_id, local_id: parent }; + let attributes = module_data.outer_attributes.clone(); + data.push((module_id, module_data.location, attributes)); + } + + if !module_data.inner_attributes.is_empty() { + let local_id = LocalModuleId(index); + let module_id = ModuleId { krate: self.crate_id, local_id }; + let attributes = module_data.inner_attributes.clone(); + data.push((module_id, module_data.location, attributes)); + } + } + + for (module_id, location, attributes) in data { + let item = Value::ModuleDefinition(module_id); + self.local_module = module_id.local_id; + self.file = location.file; + let span = location.span; + for name in attributes { + let item = item.clone(); + if let Err(error) = + self.run_comptime_attribute_on_item(&name, item, span, generated_items) + { + self.errors.push(error); + } + } + } + } + fn run_attributes_on_functions( &mut self, function_sets: &[UnresolvedFunctions], @@ -416,6 +456,10 @@ impl<'context> Elaborator<'context> { let attributes = function.secondary_attributes(); let item = Value::FunctionDefinition(*function_id); let span = function.span(); + // println!("FUNC"); + // dbg!(self.local_module); + // dbg!(self.file); + self.run_comptime_attributes_on_item(attributes, item, span, generated_items); } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 189630aa3ff..841b4415d0b 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -1811,13 +1811,9 @@ fn module_has_named_attribute( let module_data = interpreter.elaborator.get_module(module_id); let name = get_quoted(name)?; - let attributes = &module_data.attributes; - if attributes.is_empty() { - return Ok(Value::Bool(false)); - }; - let name = name.iter().map(|token| token.to_string()).collect::>().join(""); + let attributes = module_data.outer_attributes.iter().chain(&module_data.inner_attributes); for attribute in attributes { let parse_result = Elaborator::parse_attribute(attribute, location.file); let Ok(Some((function, _arguments))) = parse_result else { diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index d36971c5dc3..64b489422a0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -23,7 +23,13 @@ fn interpret_helper(src: &str) -> Result { let module_id = LocalModuleId(Index::unsafe_zeroed()); let mut modules = noirc_arena::Arena::default(); let location = Location::new(Default::default(), file); - let root = LocalModuleId(modules.insert(ModuleData::new(None, location, Vec::new(), false))); + let root = LocalModuleId(modules.insert(ModuleData::new( + None, + location, + Vec::new(), + Vec::new(), + false, + ))); assert_eq!(root, module_id); let file_manager = FileManager::new(&PathBuf::new()); 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 c2df740dc90..11a08adc1bf 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -305,6 +305,7 @@ impl<'a> ModCollector<'a> { &name, Location::new(name.span(), self.file_id), Vec::new(), + Vec::new(), false, false, ) { @@ -438,6 +439,7 @@ impl<'a> ModCollector<'a> { &name, Location::new(name.span(), self.file_id), Vec::new(), + Vec::new(), false, false, ) { @@ -627,15 +629,12 @@ impl<'a> ModCollector<'a> { ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; for submodule in submodules { - let mut attributes = Vec::new(); - attributes.extend(submodule.attributes); - attributes.extend(submodule.contents.attributes.clone()); - match self.push_child_module( context, &submodule.name, Location::new(submodule.name.span(), file_id), - attributes, + submodule.outer_attributes, + submodule.contents.inner_attributes.clone(), true, submodule.is_contract, ) { @@ -727,7 +726,8 @@ impl<'a> ModCollector<'a> { context, &mod_decl.ident, Location::new(Span::empty(0), child_file_id), - mod_decl.attributes, + mod_decl.outer_attributes, + Vec::new(), true, false, ) { @@ -754,12 +754,14 @@ impl<'a> ModCollector<'a> { /// Add a child module to the current def_map. /// On error this returns None and pushes to `errors` + #[allow(clippy::too_many_arguments)] fn push_child_module( &mut self, context: &mut Context, mod_name: &Ident, mod_location: Location, - attributes: Vec, + outer_attributes: Vec, + inner_attributes: Vec, add_to_parent_scope: bool, is_contract: bool, ) -> Result { @@ -773,7 +775,8 @@ impl<'a> ModCollector<'a> { // Eventually the location put in `ModuleData` is used for codelenses about `contract`s, // so we keep using `location` so that it continues to work as usual. let location = Location::new(mod_name.span(), mod_location.file); - let new_module = ModuleData::new(parent, location, attributes, is_contract); + let new_module = + ModuleData::new(parent, location, outer_attributes, inner_attributes, is_contract); let module_id = self.def_collector.def_map.modules.insert(new_module); let modules = &mut self.def_collector.def_map.modules; diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 258d0b01502..a1c4d04cb30 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -111,7 +111,13 @@ impl CrateDefMap { // Allocate a default Module for the root, giving it a ModuleId let mut modules: Arena = Arena::default(); let location = Location::new(Default::default(), root_file_id); - let root = modules.insert(ModuleData::new(None, location, ast.attributes.clone(), false)); + let root = modules.insert(ModuleData::new( + None, + location, + Vec::new(), + ast.inner_attributes.clone(), + false, + )); let def_map = CrateDefMap { root: LocalModuleId(root), 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 b3aea8926ea..92479cfc992 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -26,7 +26,8 @@ pub struct ModuleData { /// True if this module is a `contract Foo { ... }` module containing contract functions pub is_contract: bool, - pub attributes: Vec, + pub outer_attributes: Vec, + pub inner_attributes: Vec, /// List of all unused imports. Each time something is imported into this module it's added /// to this set. When it's used, it's removed. At the end of the program only unused imports remain. @@ -37,11 +38,16 @@ impl ModuleData { pub fn new( parent: Option, location: Location, - attributes: Vec, + outer_attributes: Vec, + inner_attributes: Vec, is_contract: bool, ) -> ModuleData { - let attributes = attributes.iter().filter_map(|attr| attr.as_custom()); - let attributes = attributes.map(|attr| attr.to_string()).collect(); + let outer_attributes = outer_attributes.iter().filter_map(|attr| attr.as_custom()); + let outer_attributes = outer_attributes.map(|attr| attr.to_string()).collect(); + + let inner_attributes = inner_attributes.iter().filter_map(|attr| attr.as_custom()); + let inner_attributes = inner_attributes.map(|attr| attr.to_string()).collect(); + ModuleData { parent, children: HashMap::new(), @@ -49,7 +55,8 @@ impl ModuleData { definitions: ItemScope::default(), location, is_contract, - attributes, + outer_attributes, + inner_attributes, unused_imports: HashSet::new(), } } diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index 0f9860f05e1..bbd128609f8 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -247,7 +247,7 @@ pub struct SortedModule { /// Full submodules as in `mod foo { ... definitions ... }` pub submodules: Vec, - pub attributes: Vec, + pub inner_attributes: Vec, } impl std::fmt::Display for SortedModule { @@ -292,7 +292,7 @@ impl std::fmt::Display for SortedModule { #[derive(Clone, Debug, Default)] pub struct ParsedModule { pub items: Vec, - pub attributes: Vec, + pub inner_attributes: Vec, } impl ParsedModule { @@ -314,7 +314,7 @@ impl ParsedModule { } } - module.attributes = self.attributes; + module.inner_attributes = self.inner_attributes; module } } @@ -345,7 +345,7 @@ pub enum ItemKind { pub struct ParsedSubModule { pub name: Ident, pub contents: ParsedModule, - pub attributes: Vec, + pub outer_attributes: Vec, pub is_contract: bool, } @@ -354,7 +354,7 @@ impl ParsedSubModule { SortedSubModule { name: self.name, contents: self.contents.into_sorted(), - attributes: self.attributes, + outer_attributes: self.outer_attributes, is_contract: self.is_contract, } } @@ -376,7 +376,7 @@ impl std::fmt::Display for SortedSubModule { pub struct SortedSubModule { pub name: Ident, pub contents: SortedModule, - pub attributes: Vec, + pub outer_attributes: Vec, pub is_contract: bool, } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 544bdd3ab5a..e086a1b02d5 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -187,7 +187,7 @@ fn module() -> impl NoirParser { }), ) .map(|(attributes, mut program)| { - program.attributes = attributes; + program.inner_attributes = attributes; program }) }) @@ -304,7 +304,7 @@ fn submodule(module_parser: impl NoirParser) -> impl NoirParser) -> impl NoirParser impl NoirParser { attributes().then_ignore(keyword(Keyword::Mod)).then(ident()).validate( |(attributes, ident), span, emit| { let attributes = validate_secondary_attributes(attributes, span, emit); - TopLevelStatement::Module(ModuleDeclaration { ident, attributes }) + TopLevelStatement::Module(ModuleDeclaration { ident, outer_attributes: attributes }) }, ) } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 62dee6110f6..c6b9fc901c3 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -67,8 +67,13 @@ pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec<(Compilation // Allocate a default Module for the root, giving it a ModuleId let mut modules: Arena = Arena::default(); let location = Location::new(Default::default(), root_file_id); - let root = - modules.insert(ModuleData::new(None, location, program.attributes.clone(), false)); + let root = modules.insert(ModuleData::new( + None, + location, + Vec::new(), + program.inner_attributes.clone(), + false, + )); let def_map = CrateDefMap { root: LocalModuleId(root), diff --git a/test_programs/compile_success_empty/comptime_module/src/main.nr b/test_programs/compile_success_empty/comptime_module/src/main.nr index f48ba977567..1e1f55fce9a 100644 --- a/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -9,6 +9,12 @@ contract bar {} #[some_attribute] mod another_module {} +#[module_func] +mod yet_another_module {} + +#[func_func] +fn coco(){} + fn main() { comptime { @@ -36,6 +42,11 @@ fn main() { } } +fn module_func(m: Module) { + assert_eq(m.name(), quote { yet_another_module }); +} + + // docs:start:as_module_example mod baz { mod qux {} diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index cfbb3279227..11bdd57c48f 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -165,7 +165,7 @@ impl super::FmtVisitor<'_> { continue; } - for attribute in module.attributes { + for attribute in module.outer_attributes { self.push_str(&format!("#[{}]\n", attribute.as_ref())); self.push_str(&self.indent.to_string()); } From de61e3c236e4a58c7f0d27084ab09dd9ddd76a75 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 19:04:40 -0300 Subject: [PATCH 09/20] Pass separate module file inner attributes --- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a20f32424f8..be6d2fc0a7a 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -728,7 +728,7 @@ impl<'a> ModCollector<'a> { &mod_decl.ident, Location::new(Span::empty(0), child_file_id), mod_decl.outer_attributes, - Vec::new(), + ast.inner_attributes.clone(), true, false, ) { From de7c3866de83550d195ef60fa9805fb2ed0bc24d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 19:37:24 -0300 Subject: [PATCH 10/20] Correctly run comptime module attributes functions --- .../noirc_frontend/src/elaborator/comptime.rs | 26 +++++++++---------- .../comptime_module/src/main.nr | 22 ++++++++++------ 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index e33781aabc2..5be1917c680 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -402,6 +402,7 @@ impl<'context> Elaborator<'context> { } self.run_attributes_on_functions(functions, &mut generated_items); + generated_items } @@ -410,27 +411,30 @@ impl<'context> Elaborator<'context> { let mut data = Vec::new(); for (index, module_data) in def_map.modules.iter() { + let local_id = LocalModuleId(index); + if !module_data.outer_attributes.is_empty() { + // When resolving an outer attribute we need to do it as if we are located in the parent module let parent = def_map.modules()[index].parent.unwrap(); - let module_data = &def_map.modules()[parent.0]; - let module_id = ModuleId { krate: self.crate_id, local_id: parent }; + let parent_module_data = &def_map.modules()[parent.0]; let attributes = module_data.outer_attributes.clone(); - data.push((module_id, module_data.location, attributes)); + data.push((local_id, parent, parent_module_data.location, attributes)); } if !module_data.inner_attributes.is_empty() { - let local_id = LocalModuleId(index); - let module_id = ModuleId { krate: self.crate_id, local_id }; let attributes = module_data.inner_attributes.clone(); - data.push((module_id, module_data.location, attributes)); + data.push((local_id, local_id, module_data.location, attributes)); } } - for (module_id, location, attributes) in data { + for (local_id, attribute_module, location, attributes) in data { + let module_id = ModuleId { krate: self.crate_id, local_id }; let item = Value::ModuleDefinition(module_id); - self.local_module = module_id.local_id; - self.file = location.file; let span = location.span; + + self.local_module = attribute_module; + self.file = location.file; + for name in attributes { let item = item.clone(); if let Err(error) = @@ -456,10 +460,6 @@ impl<'context> Elaborator<'context> { let attributes = function.secondary_attributes(); let item = Value::FunctionDefinition(*function_id); let span = function.span(); - // println!("FUNC"); - // dbg!(self.local_module); - // dbg!(self.file); - self.run_comptime_attributes_on_item(attributes, item, span, generated_items); } } diff --git a/test_programs/compile_success_empty/comptime_module/src/main.nr b/test_programs/compile_success_empty/comptime_module/src/main.nr index 1e1f55fce9a..794d2cece3b 100644 --- a/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -1,3 +1,4 @@ +#[outer_attribute] mod foo { #![some_attribute] fn x() {} @@ -9,11 +10,18 @@ contract bar {} #[some_attribute] mod another_module {} -#[module_func] -mod yet_another_module {} +#[outer_attribute_func] +mod yet_another_module { + #![inner_attribute_func] + fn foo() {} +} + +comptime mut global counter = 0; -#[func_func] -fn coco(){} +fn outer_attribute_func(m: Module) { + assert_eq(m.name(), quote { yet_another_module }); + counter += 1; +} fn main() { comptime @@ -37,16 +45,14 @@ fn main() { // Check Module::has_named_attribute assert(foo.has_named_attribute(quote { some_attribute })); + assert(foo.has_named_attribute(quote { outer_attribute })); assert(!bar.has_named_attribute(quote { some_attribute })); assert(another_module.has_named_attribute(quote { some_attribute })); } -} -fn module_func(m: Module) { - assert_eq(m.name(), quote { yet_another_module }); + assert_eq(counter, 2); } - // docs:start:as_module_example mod baz { mod qux {} From c4321fd9f85bc926a65ae3e48f34698bc7494dfb Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 19:46:19 -0300 Subject: [PATCH 11/20] Run module attributes once --- .../noirc_frontend/src/elaborator/comptime.rs | 5 +++++ compiler/noirc_frontend/src/elaborator/mod.rs | 4 ++++ .../comptime_module/src/main.nr | 15 +++++++++------ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 5be1917c680..1d68f1ecaa9 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -407,6 +407,11 @@ impl<'context> Elaborator<'context> { } fn run_attributes_on_modules(&mut self, generated_items: &mut CollectedItems) { + if self.ran_module_attributes { + return; + } + self.ran_module_attributes = true; + let def_map = &self.def_maps[&self.crate_id]; let mut data = Vec::new(); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index e84ed76050d..4141b6131c9 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -170,6 +170,9 @@ pub struct Elaborator<'context> { enable_arithmetic_generics: bool, pub(crate) interpreter_call_stack: im::Vector, + + /// Did we already run module attributes? + ran_module_attributes: bool, } #[derive(Default)] @@ -218,6 +221,7 @@ impl<'context> Elaborator<'context> { enable_arithmetic_generics, current_trait: None, interpreter_call_stack, + ran_module_attributes: false, } } diff --git a/test_programs/compile_success_empty/comptime_module/src/main.nr b/test_programs/compile_success_empty/comptime_module/src/main.nr index 794d2cece3b..0f2cfa83673 100644 --- a/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -12,15 +12,18 @@ mod another_module {} #[outer_attribute_func] mod yet_another_module { - #![inner_attribute_func] + #![super::inner_attribute_func] fn foo() {} } -comptime mut global counter = 0; +fn outer_attribute_func(m: Module) -> Quoted { + assert_eq(m.name(), quote { yet_another_module }); + quote { pub fn created_by_outer_attribute() {} } +} -fn outer_attribute_func(m: Module) { +fn inner_attribute_func(m: Module) -> Quoted { assert_eq(m.name(), quote { yet_another_module }); - counter += 1; + quote { pub fn created_by_inner_attribute() {} } } fn main() { @@ -49,8 +52,8 @@ fn main() { assert(!bar.has_named_attribute(quote { some_attribute })); assert(another_module.has_named_attribute(quote { some_attribute })); } - - assert_eq(counter, 2); + // created_by_inner_attribute(); + // created_by_outer_attribute(); } // docs:start:as_module_example From 22a401df76b75a045798de0709a6e18992a2f673 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 2 Sep 2024 20:19:41 -0300 Subject: [PATCH 12/20] Fix test --- compiler/noirc_frontend/src/lexer/lexer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index 171ff823249..f0b685b79f6 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -918,13 +918,13 @@ mod tests { #[test] fn test_inner_attribute() { - let input = r#"#![test]"#; + let input = r#"#![something]"#; let mut lexer = Lexer::new(input); let token = lexer.next_token().unwrap(); assert_eq!( token.token(), - &Token::InnerAttribute(SecondaryAttribute::Custom("test".to_string())) + &Token::InnerAttribute(SecondaryAttribute::Custom("something".to_string())) ); } From 49485ca7ba00e0158af8dba7ae24a90c76722265 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 3 Sep 2024 07:37:49 -0300 Subject: [PATCH 13/20] Make sure all attributes run for separate modules --- .../comptime_module/src/main.nr | 26 ++++++++++++++----- .../comptime_module/src/separate_module.nr | 6 +++++ 2 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 test_programs/compile_success_empty/comptime_module/src/separate_module.nr diff --git a/test_programs/compile_success_empty/comptime_module/src/main.nr b/test_programs/compile_success_empty/comptime_module/src/main.nr index 0f2cfa83673..5722d42ca26 100644 --- a/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -16,14 +16,28 @@ mod yet_another_module { fn foo() {} } -fn outer_attribute_func(m: Module) -> Quoted { +#[outer_attribute_separate_module] +mod separate_module; + +comptime mut global counter = 0; + +fn increment_counter() { + counter += 1; +} + +fn outer_attribute_func(m: Module) { assert_eq(m.name(), quote { yet_another_module }); - quote { pub fn created_by_outer_attribute() {} } + increment_counter(); } -fn inner_attribute_func(m: Module) -> Quoted { +fn inner_attribute_func(m: Module) { assert_eq(m.name(), quote { yet_another_module }); - quote { pub fn created_by_inner_attribute() {} } + increment_counter(); +} + +fn outer_attribute_separate_module(m: Module) { + assert_eq(m.name(), quote { separate_module }); + increment_counter(); } fn main() { @@ -52,8 +66,8 @@ fn main() { assert(!bar.has_named_attribute(quote { some_attribute })); assert(another_module.has_named_attribute(quote { some_attribute })); } - // created_by_inner_attribute(); - // created_by_outer_attribute(); + + assert_eq(counter, 4); } // docs:start:as_module_example diff --git a/test_programs/compile_success_empty/comptime_module/src/separate_module.nr b/test_programs/compile_success_empty/comptime_module/src/separate_module.nr new file mode 100644 index 00000000000..f2507290bff --- /dev/null +++ b/test_programs/compile_success_empty/comptime_module/src/separate_module.nr @@ -0,0 +1,6 @@ +#![inner_attribute_separate_module] + +fn inner_attribute_separate_module(m: Module) { + assert_eq(m.name(), quote { separate_module }); + super::increment_counter(); +} \ No newline at end of file From 3e036d6192703f98336581cf79cd589def9c9227 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 3 Sep 2024 09:53:09 -0300 Subject: [PATCH 14/20] nargo fmt --- .../comptime_module/src/separate_module.nr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test_programs/compile_success_empty/comptime_module/src/separate_module.nr b/test_programs/compile_success_empty/comptime_module/src/separate_module.nr index f2507290bff..53784101507 100644 --- a/test_programs/compile_success_empty/comptime_module/src/separate_module.nr +++ b/test_programs/compile_success_empty/comptime_module/src/separate_module.nr @@ -1,6 +1,5 @@ #![inner_attribute_separate_module] - fn inner_attribute_separate_module(m: Module) { assert_eq(m.name(), quote { separate_module }); super::increment_counter(); -} \ No newline at end of file +} From 9e174464e9328f1cafa07a9ea463e5f6d2819d53 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 3 Sep 2024 16:07:13 -0300 Subject: [PATCH 15/20] Parse inner attributes as top-level items --- aztec_macros/src/utils/parse_utils.rs | 1 + compiler/noirc_frontend/src/ast/visitor.rs | 13 +++++++++- .../noirc_frontend/src/elaborator/comptime.rs | 3 ++- compiler/noirc_frontend/src/parser/mod.rs | 7 ++++-- compiler/noirc_frontend/src/parser/parser.rs | 24 +++++++------------ .../src/parser/parser/attributes.rs | 6 +---- compiler/noirc_frontend/src/tests.rs | 17 +++++++++++-- tooling/nargo_fmt/src/visitor/item.rs | 3 ++- tooling/nargo_fmt/tests/expected/module.nr | 5 ++++ tooling/nargo_fmt/tests/input/module.nr | 4 ++++ 10 files changed, 56 insertions(+), 27 deletions(-) diff --git a/aztec_macros/src/utils/parse_utils.rs b/aztec_macros/src/utils/parse_utils.rs index 6a2a876e682..e7b3e347a96 100644 --- a/aztec_macros/src/utils/parse_utils.rs +++ b/aztec_macros/src/utils/parse_utils.rs @@ -53,6 +53,7 @@ fn empty_item(item: &mut Item) { ItemKind::Import(use_tree, _) => empty_use_tree(use_tree), ItemKind::Struct(noir_struct) => empty_noir_struct(noir_struct), ItemKind::TypeAlias(noir_type_alias) => empty_noir_type_alias(noir_type_alias), + ItemKind::InnerAttribute(_) => (), } } diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 3955e50b03e..0aeeed39dd0 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -16,7 +16,7 @@ use crate::{ QuotedTypeId, }, parser::{Item, ItemKind, ParsedSubModule}, - token::Tokens, + token::{SecondaryAttribute, Tokens}, ParsedModule, QuotedType, }; @@ -432,6 +432,8 @@ pub trait Visitor { fn visit_struct_pattern(&mut self, _: &Path, _: &[(Ident, Pattern)], _: Span) -> bool { true } + + fn visit_secondary_attribute(&mut self, _: &SecondaryAttribute, _: Span) {} } impl ParsedModule { @@ -481,6 +483,9 @@ impl Item { ItemKind::ModuleDecl(module_declaration) => { module_declaration.accept(self.span, visitor); } + ItemKind::InnerAttribute(attribute) => { + attribute.accept(self.span, visitor); + } } } } @@ -1289,6 +1294,12 @@ impl Pattern { } } +impl SecondaryAttribute { + pub fn accept(&self, span: Span, visitor: &mut impl Visitor) { + visitor.visit_secondary_attribute(self, span); + } +} + fn visit_expressions(expressions: &[Expression], visitor: &mut impl Visitor) { for expression in expressions { expression.accept(visitor); diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 1d68f1ecaa9..1f47b53d98d 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -339,7 +339,8 @@ impl<'context> Elaborator<'context> { | TopLevelStatement::Trait(_) | TopLevelStatement::Impl(_) | TopLevelStatement::TypeAlias(_) - | TopLevelStatement::SubModule(_) => { + | TopLevelStatement::SubModule(_) + | TopLevelStatement::InnerAttribute(_) => { let item = item.to_string(); let error = InterpreterError::UnsupportedTopLevelItemUnquote { item, location }; self.errors.push(error.into_compilation_error_pair()); diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index 1d2c6d7cc09..596d15176bc 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -41,6 +41,7 @@ pub enum TopLevelStatement { TypeAlias(NoirTypeAlias), SubModule(ParsedSubModule), Global(LetStatement), + InnerAttribute(SecondaryAttribute), Error, } @@ -57,6 +58,7 @@ impl TopLevelStatement { TopLevelStatement::TypeAlias(t) => Some(ItemKind::TypeAlias(t)), TopLevelStatement::SubModule(s) => Some(ItemKind::Submodules(s)), TopLevelStatement::Global(c) => Some(ItemKind::Global(c)), + TopLevelStatement::InnerAttribute(a) => Some(ItemKind::InnerAttribute(a)), TopLevelStatement::Error => None, } } @@ -293,7 +295,6 @@ impl std::fmt::Display for SortedModule { #[derive(Clone, Debug, Default)] pub struct ParsedModule { pub items: Vec, - pub inner_attributes: Vec, } impl ParsedModule { @@ -312,10 +313,10 @@ impl ParsedModule { ItemKind::Global(global) => module.push_global(global), ItemKind::ModuleDecl(mod_name) => module.push_module_decl(mod_name), ItemKind::Submodules(submodule) => module.push_submodule(submodule.into_sorted()), + ItemKind::InnerAttribute(attribute) => module.inner_attributes.push(attribute), } } - module.inner_attributes = self.inner_attributes; module } } @@ -338,6 +339,7 @@ pub enum ItemKind { Global(LetStatement), ModuleDecl(ModuleDeclaration), Submodules(ParsedSubModule), + InnerAttribute(SecondaryAttribute), } /// A submodule defined via `mod name { contents }` in some larger file. @@ -519,6 +521,7 @@ impl std::fmt::Display for TopLevelStatement { TopLevelStatement::TypeAlias(t) => t.fmt(f), TopLevelStatement::SubModule(s) => s.fmt(f), TopLevelStatement::Global(c) => c.fmt(f), + TopLevelStatement::InnerAttribute(a) => write!(f, "#![{}]", a), TopLevelStatement::Error => write!(f, "error"), } } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index d7a9cc1862f..2bc7a88c6c5 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -26,7 +26,7 @@ use self::path::as_trait_path; use self::primitives::{keyword, macro_quote_marker, mutable_reference, variable}; use self::types::{generic_type_args, maybe_comp_time}; -use attributes::{attributes, inner_attributes, validate_secondary_attributes}; +use attributes::{attributes, inner_attribute, validate_secondary_attributes}; pub use types::parse_type; use visibility::visibility_modifier; @@ -176,20 +176,13 @@ fn program() -> impl NoirParser { /// | %empty fn module() -> impl NoirParser { recursive(|module_parser| { - inner_attributes() - .then( - empty() - .to(ParsedModule::default()) - .then(spanned(top_level_statement(module_parser)).repeated()) - .foldl(|mut program, (statement, span)| { - if let Some(kind) = statement.into_item_kind() { - program.items.push(Item { kind, span }); - } - program - }), - ) - .map(|(attributes, mut program)| { - program.inner_attributes = attributes; + empty() + .to(ParsedModule::default()) + .then(spanned(top_level_statement(module_parser)).repeated()) + .foldl(|mut program, (statement, span)| { + if let Some(kind) = statement.into_item_kind() { + program.items.push(Item { kind, span }); + } program }) }) @@ -223,6 +216,7 @@ fn top_level_statement<'a>( module_declaration().then_ignore(force(just(Token::Semicolon))), use_statement().then_ignore(force(just(Token::Semicolon))), global_declaration().then_ignore(force(just(Token::Semicolon))), + inner_attribute().map(TopLevelStatement::InnerAttribute), )) .recover_via(top_level_statement_recovery()) } diff --git a/compiler/noirc_frontend/src/parser/parser/attributes.rs b/compiler/noirc_frontend/src/parser/parser/attributes.rs index 2354708aa75..66d0ca29ca6 100644 --- a/compiler/noirc_frontend/src/parser/parser/attributes.rs +++ b/compiler/noirc_frontend/src/parser/parser/attributes.rs @@ -68,7 +68,7 @@ pub(super) fn validate_secondary_attributes( struct_attributes } -fn inner_attribute() -> impl NoirParser { +pub(super) fn inner_attribute() -> impl NoirParser { token_kind(TokenKind::InnerAttribute).map(|token| match token { Token::InnerAttribute(attribute) => attribute, _ => unreachable!( @@ -76,7 +76,3 @@ fn inner_attribute() -> impl NoirParser { ), }) } - -pub(super) fn inner_attributes() -> impl NoirParser> { - inner_attribute().repeated() -} diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index c0a85859031..ee5cc27083e 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -28,7 +28,8 @@ use crate::hir::def_collector::dc_crate::DefCollector; use crate::hir_def::expr::HirExpression; use crate::hir_def::stmt::HirStatement; use crate::monomorphization::monomorphize; -use crate::parser::ParserErrorReason; +use crate::parser::{ItemKind, ParserErrorReason}; +use crate::token::SecondaryAttribute; use crate::ParsedModule; use crate::{ hir::def_map::{CrateDefMap, LocalModuleId}, @@ -64,6 +65,18 @@ pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec<(Compilation remove_experimental_warnings(&mut errors); if !has_parser_error(&errors) { + let inner_attributes: Vec = program + .items + .iter() + .filter_map(|item| { + if let ItemKind::InnerAttribute(attribute) = &item.kind { + Some(attribute.clone()) + } else { + None + } + }) + .collect(); + // Allocate a default Module for the root, giving it a ModuleId let mut modules: Arena = Arena::default(); let location = Location::new(Default::default(), root_file_id); @@ -71,7 +84,7 @@ pub(crate) fn get_program(src: &str) -> (ParsedModule, Context, Vec<(Compilation None, location, Vec::new(), - program.inner_attributes.clone(), + inner_attributes.clone(), false, )); diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index f8a17317868..9e556e0fcbe 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -232,7 +232,8 @@ impl super::FmtVisitor<'_> { | ItemKind::TraitImpl(_) | ItemKind::TypeAlias(_) | ItemKind::Global(_) - | ItemKind::ModuleDecl(_) => { + | ItemKind::ModuleDecl(_) + | ItemKind::InnerAttribute(_) => { self.push_rewrite(self.slice(span).to_string(), span); self.last_position = span.end(); } diff --git a/tooling/nargo_fmt/tests/expected/module.nr b/tooling/nargo_fmt/tests/expected/module.nr index 79e4039347a..0a051a1b50f 100644 --- a/tooling/nargo_fmt/tests/expected/module.nr +++ b/tooling/nargo_fmt/tests/expected/module.nr @@ -1,5 +1,6 @@ #![inner] #![inner2] + mod a { mod b { struct Data { @@ -27,4 +28,8 @@ mod a { #[attr] mod baz; + + mod empty { + #![inner] + } } diff --git a/tooling/nargo_fmt/tests/input/module.nr b/tooling/nargo_fmt/tests/input/module.nr index 1bfef1d068b..0a051a1b50f 100644 --- a/tooling/nargo_fmt/tests/input/module.nr +++ b/tooling/nargo_fmt/tests/input/module.nr @@ -28,4 +28,8 @@ mod a { #[attr] mod baz; + + mod empty { + #![inner] + } } From b3eae444b17dcdbee51d5c910d79f5c5af036fb1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 3 Sep 2024 16:57:33 -0300 Subject: [PATCH 16/20] Collect module attributes in generated items --- .../noirc_frontend/src/elaborator/comptime.rs | 55 ++++++----------- compiler/noirc_frontend/src/elaborator/mod.rs | 11 ++-- .../src/hir/def_collector/dc_crate.rs | 18 ++++++ .../src/hir/def_collector/dc_mod.rs | 59 ++++++++++++++++++- 4 files changed, 99 insertions(+), 44 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 1f47b53d98d..92ac4c993be 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -10,12 +10,12 @@ use crate::{ comptime::{Interpreter, InterpreterError, Value}, def_collector::{ dc_crate::{ - CollectedItems, CompilationError, UnresolvedFunctions, UnresolvedStruct, - UnresolvedTrait, UnresolvedTraitImpl, + CollectedItems, CompilationError, ModuleAttribute, UnresolvedFunctions, + UnresolvedStruct, UnresolvedTrait, UnresolvedTraitImpl, }, dc_mod, }, - def_map::{LocalModuleId, ModuleId}, + def_map::ModuleId, resolution::errors::ResolverError, }, hir_def::expr::HirIdent, @@ -376,13 +376,14 @@ impl<'context> Elaborator<'context> { /// Returns any new items generated by attributes. pub(super) fn run_attributes( &mut self, + module_attributes: &Vec, traits: &BTreeMap, types: &BTreeMap, functions: &[UnresolvedFunctions], ) -> CollectedItems { let mut generated_items = CollectedItems::default(); - self.run_attributes_on_modules(&mut generated_items); + self.run_attributes_on_modules(module_attributes, &mut generated_items); for (trait_id, trait_) in traits { let attributes = &trait_.trait_def.attributes; @@ -407,44 +408,24 @@ impl<'context> Elaborator<'context> { generated_items } - fn run_attributes_on_modules(&mut self, generated_items: &mut CollectedItems) { - if self.ran_module_attributes { - return; - } - self.ran_module_attributes = true; - - let def_map = &self.def_maps[&self.crate_id]; - let mut data = Vec::new(); - - for (index, module_data) in def_map.modules.iter() { - let local_id = LocalModuleId(index); - - if !module_data.outer_attributes.is_empty() { - // When resolving an outer attribute we need to do it as if we are located in the parent module - let parent = def_map.modules()[index].parent.unwrap(); - let parent_module_data = &def_map.modules()[parent.0]; - let attributes = module_data.outer_attributes.clone(); - data.push((local_id, parent, parent_module_data.location, attributes)); - } - - if !module_data.inner_attributes.is_empty() { - let attributes = module_data.inner_attributes.clone(); - data.push((local_id, local_id, module_data.location, attributes)); - } - } - - for (local_id, attribute_module, location, attributes) in data { + fn run_attributes_on_modules( + &mut self, + module_attributes: &Vec, + generated_items: &mut CollectedItems, + ) { + for module_attribute in module_attributes { + let local_id = module_attribute.module_id; let module_id = ModuleId { krate: self.crate_id, local_id }; let item = Value::ModuleDefinition(module_id); - let span = location.span; + let attribute = &module_attribute.attribute; + let span = Span::default(); - self.local_module = attribute_module; - self.file = location.file; + self.local_module = module_attribute.attribute_module_id; + self.file = module_attribute.attribute_file_id; - for name in attributes { - let item = item.clone(); + if let SecondaryAttribute::Custom(name) = attribute { if let Err(error) = - self.run_comptime_attribute_on_item(&name, item, span, generated_items) + self.run_comptime_attribute_on_item(name, item.clone(), span, generated_items) { self.errors.push(error); } diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 4141b6131c9..0c1455d688f 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -170,9 +170,6 @@ pub struct Elaborator<'context> { enable_arithmetic_generics: bool, pub(crate) interpreter_call_stack: im::Vector, - - /// Did we already run module attributes? - ran_module_attributes: bool, } #[derive(Default)] @@ -221,7 +218,6 @@ impl<'context> Elaborator<'context> { enable_arithmetic_generics, current_trait: None, interpreter_call_stack, - ran_module_attributes: false, } } @@ -324,7 +320,12 @@ impl<'context> Elaborator<'context> { // We have to run any comptime attributes on functions before the function is elaborated // since the generated items are checked beforehand as well. - let generated_items = self.run_attributes(&items.traits, &items.types, &items.functions); + let generated_items = self.run_attributes( + &items.module_attributes, + &items.traits, + &items.types, + &items.functions, + ); // After everything is collected, we can elaborate our generated items. // It may be better to inline these within `items` entirely since elaborating them 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 6a6cabe593d..98555375790 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -7,6 +7,7 @@ use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; use crate::hir::resolution::errors::ResolverError; use crate::hir::resolution::path_resolver; use crate::hir::type_check::TypeCheckError; +use crate::token::SecondaryAttribute; use crate::{Generics, Type}; use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution}; @@ -111,6 +112,21 @@ pub struct UnresolvedGlobal { pub stmt_def: LetStatement, } +pub struct ModuleAttribute { + // The file in which the module is defined + pub file_id: FileId, + // The module this attribute is attached to + pub module_id: LocalModuleId, + // The file where the attribute exists (it could be the same as `file_id` + // or a different one if it's an inner attribute in a different file) + pub attribute_file_id: FileId, + // The module where the attribute is defined (similar to `attribute_file_id`, + // it could be different than `module_id` for inner attributes) + pub attribute_module_id: LocalModuleId, + pub attribute: SecondaryAttribute, + pub is_inner: bool, +} + /// Given a Crate root, collect all definitions in that crate pub struct DefCollector { pub(crate) def_map: CrateDefMap, @@ -127,6 +143,7 @@ pub struct CollectedItems { pub globals: Vec, pub(crate) impls: ImplMap, pub(crate) trait_impls: Vec, + pub(crate) module_attributes: Vec, } impl CollectedItems { @@ -238,6 +255,7 @@ impl DefCollector { impls: HashMap::default(), globals: vec![], trait_impls: vec![], + module_attributes: vec![], }, } } 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 be6d2fc0a7a..7aa9b60aa8f 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -27,6 +27,7 @@ use crate::{ }; use crate::{Generics, Kind, ResolvedGeneric, Type, TypeVariable}; +use super::dc_crate::ModuleAttribute; use super::{ dc_crate::{ CompilationError, DefCollector, UnresolvedFunctions, UnresolvedGlobal, UnresolvedTraitImpl, @@ -66,6 +67,8 @@ pub fn collect_defs( context, decl, crate_id, + file_id, + module_id, macro_processors, )); } @@ -73,6 +76,7 @@ pub fn collect_defs( errors.extend(collector.collect_submodules( context, crate_id, + module_id, ast.submodules, file_id, macro_processors, @@ -103,10 +107,40 @@ pub fn collect_defs( collector.collect_impls(context, ast.impls, crate_id); + collector.collect_attributes( + ast.inner_attributes, + file_id, + module_id, + file_id, + module_id, + true, + ); + errors } impl<'a> ModCollector<'a> { + fn collect_attributes( + &mut self, + attributes: Vec, + file_id: FileId, + module_id: LocalModuleId, + attribute_file_id: FileId, + attribute_module_id: LocalModuleId, + is_inner: bool, + ) { + for attribute in attributes { + self.def_collector.items.module_attributes.push(ModuleAttribute { + file_id, + module_id, + attribute_file_id, + attribute_module_id, + attribute, + is_inner, + }); + } + } + fn collect_globals( &mut self, context: &mut Context, @@ -624,6 +658,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, crate_id: CrateId, + parent_module_id: LocalModuleId, submodules: Vec, file_id: FileId, macro_processors: &[&dyn MacroProcessor], @@ -634,12 +669,21 @@ impl<'a> ModCollector<'a> { context, &submodule.name, Location::new(submodule.name.span(), file_id), - submodule.outer_attributes, + submodule.outer_attributes.clone(), submodule.contents.inner_attributes.clone(), true, submodule.is_contract, ) { Ok(child) => { + self.collect_attributes( + submodule.outer_attributes, + file_id, + child.local_id, + file_id, + parent_module_id, + false, + ); + errors.extend(collect_defs( self.def_collector, submodule.contents, @@ -666,6 +710,8 @@ impl<'a> ModCollector<'a> { context: &mut Context, mod_decl: ModuleDeclaration, crate_id: CrateId, + parent_file_id: FileId, + parent_module_id: LocalModuleId, macro_processors: &[&dyn MacroProcessor], ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; @@ -727,12 +773,21 @@ impl<'a> ModCollector<'a> { context, &mod_decl.ident, Location::new(Span::empty(0), child_file_id), - mod_decl.outer_attributes, + mod_decl.outer_attributes.clone(), ast.inner_attributes.clone(), true, false, ) { Ok(child_mod_id) => { + self.collect_attributes( + mod_decl.outer_attributes, + child_file_id, + child_mod_id.local_id, + parent_file_id, + parent_module_id, + false, + ); + // Track that the "foo" in `mod foo;` points to the module "foo" context.def_interner.add_module_reference(child_mod_id, location); From e3991e48858627da071d47503eaa1a8f2480414e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 3 Sep 2024 17:00:59 -0300 Subject: [PATCH 17/20] Refactor to remove duplicate code --- .../noirc_frontend/src/elaborator/comptime.rs | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 92ac4c993be..1f5501a8cb1 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -97,17 +97,27 @@ impl<'context> Elaborator<'context> { generated_items: &mut CollectedItems, ) { for attribute in attributes { - if let SecondaryAttribute::Custom(name) = attribute { - if let Err(error) = - self.run_comptime_attribute_on_item(name, item.clone(), span, generated_items) - { - self.errors.push(error); - } - } + self.run_comptime_attribute_on_item(attribute, &item, span, generated_items); } } fn run_comptime_attribute_on_item( + &mut self, + attribute: &SecondaryAttribute, + item: &Value, + span: Span, + generated_items: &mut CollectedItems, + ) { + if let SecondaryAttribute::Custom(name) = attribute { + if let Err(error) = + self.run_comptime_attribute_name_on_item(name, item.clone(), span, generated_items) + { + self.errors.push(error); + } + } + } + + fn run_comptime_attribute_name_on_item( &mut self, attribute: &str, item: Value, @@ -423,13 +433,7 @@ impl<'context> Elaborator<'context> { self.local_module = module_attribute.attribute_module_id; self.file = module_attribute.attribute_file_id; - if let SecondaryAttribute::Custom(name) = attribute { - if let Err(error) = - self.run_comptime_attribute_on_item(name, item.clone(), span, generated_items) - { - self.errors.push(error); - } - } + self.run_comptime_attribute_on_item(attribute, &item, span, generated_items) } } From 1ad7c46009c76f0a1ee15d8b439b40359a21a4da Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 3 Sep 2024 17:05:55 -0300 Subject: [PATCH 18/20] clippy --- compiler/noirc_frontend/src/elaborator/comptime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 1f5501a8cb1..70c88e2a5f4 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -433,7 +433,7 @@ impl<'context> Elaborator<'context> { self.local_module = module_attribute.attribute_module_id; self.file = module_attribute.attribute_file_id; - self.run_comptime_attribute_on_item(attribute, &item, span, generated_items) + self.run_comptime_attribute_on_item(attribute, &item, span, generated_items); } } From 4bcfdc01048d7fe0855b3ee14f757f19b7aac8a9 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 4 Sep 2024 14:29:09 -0300 Subject: [PATCH 19/20] Change Vec references to slice references --- compiler/noirc_frontend/src/elaborator/comptime.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 70c88e2a5f4..eb7ff914eb0 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -386,7 +386,7 @@ impl<'context> Elaborator<'context> { /// Returns any new items generated by attributes. pub(super) fn run_attributes( &mut self, - module_attributes: &Vec, + module_attributes: &[ModuleAttribute], traits: &BTreeMap, types: &BTreeMap, functions: &[UnresolvedFunctions], @@ -420,7 +420,7 @@ impl<'context> Elaborator<'context> { fn run_attributes_on_modules( &mut self, - module_attributes: &Vec, + module_attributes: &[ModuleAttribute], generated_items: &mut CollectedItems, ) { for module_attribute in module_attributes { From 5a1cc1c6946a6501ff2a9ccdc1dd8c750ba07cf4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 4 Sep 2024 14:42:18 -0300 Subject: [PATCH 20/20] Run module attributes last --- compiler/noirc_frontend/src/elaborator/comptime.rs | 6 +++--- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index eb7ff914eb0..1fece528d41 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -386,15 +386,13 @@ impl<'context> Elaborator<'context> { /// Returns any new items generated by attributes. pub(super) fn run_attributes( &mut self, - module_attributes: &[ModuleAttribute], traits: &BTreeMap, types: &BTreeMap, functions: &[UnresolvedFunctions], + module_attributes: &[ModuleAttribute], ) -> CollectedItems { let mut generated_items = CollectedItems::default(); - self.run_attributes_on_modules(module_attributes, &mut generated_items); - for (trait_id, trait_) in traits { let attributes = &trait_.trait_def.attributes; let item = Value::TraitDefinition(*trait_id); @@ -415,6 +413,8 @@ impl<'context> Elaborator<'context> { self.run_attributes_on_functions(functions, &mut generated_items); + self.run_attributes_on_modules(module_attributes, &mut generated_items); + generated_items } diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 0c1455d688f..a05a6641c58 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -321,10 +321,10 @@ impl<'context> Elaborator<'context> { // We have to run any comptime attributes on functions before the function is elaborated // since the generated items are checked beforehand as well. let generated_items = self.run_attributes( - &items.module_attributes, &items.traits, &items.types, &items.functions, + &items.module_attributes, ); // After everything is collected, we can elaborate our generated items.