feat(parser): allow multiple attributes (#2537)

Co-authored-by: kevaundray <[email protected]>
noir-lang · Sep 11, 2023 · 7cdff2e · 7cdff2e
1 parent c0c462c
commit 7cdff2e
Show file tree

Hide file tree

Showing 17 changed files with 264 additions and 94 deletions.
diff --git a/crates/nargo_cli/tests/compile_failure/multiple_primary_attributes_fail/Nargo.toml b/crates/nargo_cli/tests/compile_failure/multiple_primary_attributes_fail/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "multiple_primary_attributes_fail"
+type = "bin"
+authors = [""]
+compiler_version = "0.9.0"
+
+[dependencies]
diff --git a/crates/nargo_cli/tests/compile_failure/multiple_primary_attributes_fail/src/main.nr b/crates/nargo_cli/tests/compile_failure/multiple_primary_attributes_fail/src/main.nr
@@ -0,0 +1,6 @@
+
+#[oracle(oracleName)]
+#[builtin(builtinName)]
+fn main(x: Field) -> pub Field {
+    x + 1
+}
diff --git a/crates/nargo_cli/tests/compile_success_empty/attributes_multiple/Nargo.toml b/crates/nargo_cli/tests/compile_success_empty/attributes_multiple/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "attributes_multiple"
+type = "bin"
+authors = [""]
+compiler_version = "0.10.5"
+
+[dependencies]
diff --git a/crates/nargo_cli/tests/compile_success_empty/attributes_multiple/src/main.nr b/crates/nargo_cli/tests/compile_success_empty/attributes_multiple/src/main.nr
@@ -0,0 +1,8 @@
+
+fn main() {
+    another_func()
+}
+
+#[aztec(private)]
+#[internal]
+fn another_func() {}
diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs
@@ -1,6 +1,6 @@
 use std::fmt::Display;
 
-use crate::token::{Attribute, Token};
+use crate::token::{Attributes, Token};
 use crate::{
     Distinctness, Ident, Path, Pattern, Recoverable, Statement, TraitConstraint, UnresolvedType,
     UnresolvedTypeData, Visibility,
@@ -351,8 +351,9 @@ pub struct Lambda {
 pub struct FunctionDefinition {
     pub name: Ident,
 
-    // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive
-    pub attribute: Option<Attribute>,
+    // The `Attributes` container holds both `primary` (ones that change the function kind)
+    // and `secondary` attributes (ones that do not change the function kind)
+    pub attributes: Attributes,
 
     /// True if this function was defined with the 'open' keyword
     pub is_open: bool,
@@ -643,7 +644,7 @@ impl FunctionDefinition {
             .collect();
         FunctionDefinition {
             name: name.clone(),
-            attribute: None,
+            attributes: Attributes::empty(),
             is_open: false,
             is_internal: false,
             is_unconstrained: false,
@@ -661,9 +662,7 @@ impl FunctionDefinition {
 
 impl Display for FunctionDefinition {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        if let Some(attribute) = &self.attribute {
-            writeln!(f, "{attribute}")?;
-        }
+        writeln!(f, "{:?}", self.attributes)?;
 
         let parameters = vecmap(&self.parameters, |(name, r#type, visibility)| {
             format!("{name}: {visibility} {type}")

diff --git a/crates/noirc_frontend/src/ast/function.rs b/crates/noirc_frontend/src/ast/function.rs
@@ -2,7 +2,10 @@ use std::fmt::Display;
 
 use noirc_errors::Span;
 
-use crate::{token::Attribute, FunctionReturnType, Ident, Pattern, Visibility};
+use crate::{
+    token::{Attributes, PrimaryAttribute, SecondaryAttribute},
+    FunctionReturnType, Ident, Pattern, Visibility,
+};
 
 use super::{FunctionDefinition, UnresolvedType, UnresolvedTypeData};
 
@@ -59,8 +62,14 @@ impl NoirFunction {
     pub fn parameters(&self) -> &Vec<(Pattern, UnresolvedType, Visibility)> {
         &self.def.parameters
     }
-    pub fn attribute(&self) -> Option<&Attribute> {
-        self.def.attribute.as_ref()
+    pub fn attributes(&self) -> &Attributes {
+        &self.def.attributes
+    }
+    pub fn primary_attribute(&self) -> Option<&PrimaryAttribute> {
+        self.def.attributes.primary.as_ref()
+    }
+    pub fn secondary_attributes(&self) -> &Vec<SecondaryAttribute> {
+        self.def.attributes.secondary.as_ref()
     }
     pub fn def(&self) -> &FunctionDefinition {
         &self.def
@@ -80,20 +89,20 @@ impl NoirFunction {
             FunctionKind::LowLevel => {}
             _ => return None,
         }
-        assert!(self.attribute().unwrap().is_foreign());
+        assert!(self.primary_attribute().unwrap().is_foreign());
         Some(&self.def)
     }
 }
 
 impl From<FunctionDefinition> for NoirFunction {
     fn from(fd: FunctionDefinition) -> Self {
-        let kind = match fd.attribute {
-            Some(Attribute::Builtin(_)) => FunctionKind::Builtin,
-            Some(Attribute::Foreign(_)) => FunctionKind::LowLevel,
-            Some(Attribute::Test { .. }) => FunctionKind::Normal,
-            Some(Attribute::Oracle(_)) => FunctionKind::Oracle,
-            Some(Attribute::Deprecated(_)) | None => FunctionKind::Normal,
-            Some(Attribute::Custom(_)) => FunctionKind::Normal,
+        // The function type is determined by the existence of a primary attribute
+        let kind = match fd.attributes.primary {
+            Some(PrimaryAttribute::Builtin(_)) => FunctionKind::Builtin,
+            Some(PrimaryAttribute::Foreign(_)) => FunctionKind::LowLevel,
+            Some(PrimaryAttribute::Test { .. }) => FunctionKind::Normal,
+            Some(PrimaryAttribute::Oracle(_)) => FunctionKind::Oracle,
+            None => FunctionKind::Normal,
         };
 
         NoirFunction { def: fd, kind }

diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -213,6 +213,7 @@ impl<'a> ModCollector<'a> {
         }
 
         for item in &trait_def.items {
+            // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
             if let TraitItem::Function {
                 name,
                 generics,

diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs
@@ -3,7 +3,7 @@ use crate::hir::def_collector::dc_crate::DefCollector;
 use crate::hir::Context;
 use crate::node_interner::{FuncId, NodeInterner};
 use crate::parser::{parse_program, ParsedModule};
-use crate::token::{Attribute, TestScope};
+use crate::token::{PrimaryAttribute, TestScope};
 use arena::{Arena, Index};
 use fm::{FileId, FileManager};
 use noirc_errors::{FileDiagnostic, Location};
@@ -139,8 +139,10 @@ impl CrateDefMap {
         self.modules.iter().flat_map(|(_, module)| {
             module.value_definitions().filter_map(|id| {
                 if let Some(func_id) = id.as_function() {
-                    match interner.function_meta(&func_id).attributes {
-                        Some(Attribute::Test(scope)) => Some(TestFunction::new(func_id, scope)),
+                    match interner.function_meta(&func_id).attributes.primary {
+                        Some(PrimaryAttribute::Test(scope)) => {
+                            Some(TestFunction::new(func_id, scope))
+                        }
                         _ => None,
                     }
                 } else {

diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs
@@ -17,7 +17,7 @@ use crate::hir_def::expr::{
     HirIfExpression, HirIndexExpression, HirInfixExpression, HirLambda, HirLiteral,
     HirMemberAccess, HirMethodCallExpression, HirPrefixExpression,
 };
-use crate::token::Attribute;
+use crate::token::PrimaryAttribute;
 use regex::Regex;
 use std::collections::{BTreeMap, HashSet};
 use std::rc::Rc;
@@ -678,7 +678,7 @@ impl<'a> Resolver<'a> {
         let id = self.interner.function_definition_id(func_id);
         let name_ident = HirIdent { id, location };
 
-        let attributes = func.attribute().cloned();
+        let attributes = func.attributes().clone();
 
         let mut generics =
             vecmap(self.generics.clone(), |(name, typevar, _)| match &*typevar.borrow() {
@@ -731,7 +731,9 @@ impl<'a> Resolver<'a> {
             self.push_err(ResolverError::DistinctNotAllowed { ident: func.name_ident().clone() });
         }
 
-        if matches!(attributes, Some(Attribute::Test { .. })) && !parameters.is_empty() {
+        if matches!(attributes.primary, Some(PrimaryAttribute::Test { .. }))
+            && !parameters.is_empty()
+        {
             self.push_err(ResolverError::TestFunctionHasParameters {
                 span: func.name_ident().span(),
             });

diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs
@@ -11,7 +11,6 @@ use crate::{
         types::Type,
     },
     node_interner::{DefinitionKind, ExprId, FuncId},
-    token::Attribute::Deprecated,
     Shared, Signedness, TypeBinding, TypeVariableKind, UnaryOp,
 };
 
@@ -26,7 +25,7 @@ impl<'interner> TypeChecker<'interner> {
                 self.interner.try_definition(id).map(|def| &def.kind)
             {
                 let meta = self.interner.function_meta(func_id);
-                if let Some(Deprecated(note)) = meta.attributes {
+                if let Some(note) = meta.attributes.get_deprecated_note() {
                     self.errors.push(TypeCheckError::CallDeprecated {
                         name: self.interner.definition_name(id).to_string(),
                         note,

diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs
@@ -187,6 +187,7 @@ mod test {
         stmt::HirStatement,
     };
     use crate::node_interner::{DefinitionKind, FuncId, NodeInterner};
+    use crate::token::Attributes;
     use crate::{
         hir::{
             def_map::{CrateDefMap, LocalModuleId, ModuleDefId},
@@ -257,7 +258,7 @@ mod test {
             name,
             kind: FunctionKind::Normal,
             module_id: ModuleId::dummy_id(),
-            attributes: None,
+            attributes: Attributes::empty(),
             location,
             contract_function_type: None,
             is_internal: None,

diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs
@@ -5,7 +5,7 @@ use super::expr::{HirBlockExpression, HirExpression, HirIdent};
 use super::stmt::HirPattern;
 use crate::hir::def_map::ModuleId;
 use crate::node_interner::{ExprId, NodeInterner};
-use crate::{token::Attribute, FunctionKind};
+use crate::{token::Attributes, FunctionKind};
 use crate::{ContractFunctionType, Distinctness, FunctionReturnType, Type, Visibility};
 
 /// A Hir function is a block expression
@@ -99,9 +99,9 @@ pub struct FuncMeta {
     pub module_id: ModuleId,
 
     /// A function's attributes are the `#[...]` items above the function
-    /// definition, if any. Currently, this is limited to a maximum of only one
-    /// Attribute per function.
-    pub attributes: Option<Attribute>,
+    /// definition.
+    /// Primary Attributes will alter the function kind, secondary attributes do not
+    pub attributes: Attributes,
 
     /// This function's type in its contract.
     /// If this function is not in a contract, this is always 'Secret'.

diff --git a/crates/noirc_frontend/src/lexer/lexer.rs b/crates/noirc_frontend/src/lexer/lexer.rs
@@ -411,7 +411,7 @@ impl<'a> Iterator for Lexer<'a> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::token::TestScope;
+    use crate::token::{Attribute, PrimaryAttribute, SecondaryAttribute, TestScope};
     #[test]
     fn test_single_double_char() {
         let input = "! != + ( ) { } [ ] | , ; : :: < <= > >= & - -> . .. % / * = == << >>";
@@ -474,7 +474,10 @@ mod tests {
         let mut lexer = Lexer::new(input);
 
         let token = lexer.next().unwrap().unwrap();
-        assert_eq!(token.token(), &Token::Attribute(Attribute::Deprecated(None)));
+        assert_eq!(
+            token.token(),
+            &Token::Attribute(Attribute::Secondary(SecondaryAttribute::Deprecated(None)))
+        );
     }
 
     #[test]
@@ -485,7 +488,9 @@ mod tests {
         let token = lexer.next().unwrap().unwrap();
         assert_eq!(
             token.token(),
-            &Token::Attribute(Attribute::Deprecated("hello".to_string().into()))
+            &Token::Attribute(Attribute::Secondary(crate::token::SecondaryAttribute::Deprecated(
+                "hello".to_string().into()
+            )))
         );
     }
 
@@ -494,9 +499,9 @@ mod tests {
         let input = "#[foreign(sha256)]#[foreign(blake2s)]#[builtin(sum)]";
 
         let expected = vec![
-            Token::Attribute(Attribute::Foreign("sha256".to_string())),
-            Token::Attribute(Attribute::Foreign("blake2s".to_string())),
-            Token::Attribute(Attribute::Builtin("sum".to_string())),
+            Token::Attribute(Attribute::Primary(PrimaryAttribute::Foreign("sha256".to_string()))),
+            Token::Attribute(Attribute::Primary(PrimaryAttribute::Foreign("blake2s".to_string()))),
+            Token::Attribute(Attribute::Primary(PrimaryAttribute::Builtin("sum".to_string()))),
         ];
 
         let mut lexer = Lexer::new(input);
@@ -514,7 +519,9 @@ mod tests {
         let token = lexer.next().unwrap().unwrap();
         assert_eq!(
             token.token(),
-            &Token::Attribute(Attribute::Custom("custom(hello)".to_string()))
+            &Token::Attribute(Attribute::Secondary(SecondaryAttribute::Custom(
+                "custom(hello)".to_string()
+            )))
         );
     }
 
@@ -524,7 +531,10 @@ mod tests {
         let mut lexer = Lexer::new(input);
 
         let token = lexer.next().unwrap().unwrap();
-        assert_eq!(token.token(), &Token::Attribute(Attribute::Test(TestScope::None)));
+        assert_eq!(
+            token.token(),
+            &Token::Attribute(Attribute::Primary(PrimaryAttribute::Test(TestScope::None)))
+        );
     }
     #[test]
     fn test_attribute_with_valid_scope() {
@@ -534,7 +544,9 @@ mod tests {
         let token = lexer.next().unwrap().unwrap();
         assert_eq!(
             token.token(),
-            &Token::Attribute(Attribute::Test(TestScope::ShouldFailWith { reason: None }))
+            &Token::Attribute(Attribute::Primary(PrimaryAttribute::Test(
+                TestScope::ShouldFailWith { reason: None }
+            )))
         );
     }
 
@@ -546,9 +558,9 @@ mod tests {
         let token = lexer.next().unwrap().unwrap();
         assert_eq!(
             token.token(),
-            &Token::Attribute(Attribute::Test(TestScope::ShouldFailWith {
-                reason: Some("hello".to_owned())
-            }))
+            &Token::Attribute(Attribute::Primary(PrimaryAttribute::Test(
+                TestScope::ShouldFailWith { reason: Some("hello".to_owned()) }
+            )))
         );
     }