feat: Adding internal keyword (#1873)

* feat: add internal keyword * chore: remove 👺 * fix: comment cleanup * feat: add `is_internal` to abi * Update crates/noirc_frontend/src/hir/resolution/resolver.rs Co-authored-by: jfecher <jfecher11@gmail.com> * fix: cleanup * fix: is_internal name * Update crates/noirc_driver/src/lib.rs Co-authored-by: jfecher <jfecher11@gmail.com> --------- Co-authored-by: jfecher <jfecher11@gmail.com> Co-authored-by: kevaundray <kevtheappdev@gmail.com>
noir-lang · Jul 8, 2023 · 7a85493 · 7a85493
1 parent fc755b9
commit 7a85493
Show file tree

Hide file tree

Showing 12 changed files with 53 additions and 7 deletions.
diff --git a/crates/nargo/src/artifacts/contract.rs b/crates/nargo/src/artifacts/contract.rs
@@ -28,6 +28,8 @@ pub struct PreprocessedContractFunction {
 
     pub function_type: ContractFunctionType,
 
+    pub is_internal: bool,
+
     pub abi: Abi,
 
     #[serde(

diff --git a/crates/nargo/src/ops/preprocess.rs b/crates/nargo/src/ops/preprocess.rs
@@ -53,6 +53,7 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
     Ok(PreprocessedContractFunction {
         name: func.name,
         function_type: func.function_type,
+        is_internal: func.is_internal,
         abi: func.abi,
 
         bytecode: optimized_bytecode,

diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/contracts/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/contracts/src/main.nr
@@ -5,4 +5,6 @@ fn main(x : Field, y : pub Field) {
 contract Foo {
     fn double(x: Field) -> pub Field { x * 2 }
     fn triple(x: Field) -> pub Field { x * 3 }
+    internal fn quadruple(x: Field) -> pub Field { x * 4 }
+    open internal fn skibbidy(x: Field) -> pub Field { x * 5 }
 }
diff --git a/crates/noirc_driver/src/contract.rs b/crates/noirc_driver/src/contract.rs
@@ -41,6 +41,8 @@ pub struct ContractFunction {
 
     pub function_type: ContractFunctionType,
 
+    pub is_internal: bool,
+
     pub abi: Abi,
 
     #[serde(serialize_with = "serialize_circuit", deserialize_with = "deserialize_circuit")]

diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs
@@ -305,6 +305,7 @@ fn compile_contract(
         functions.push(ContractFunction {
             name,
             function_type,
+            is_internal: func_meta.is_internal.unwrap_or(false),
             abi: function.abi,
             bytecode: function.circuit,
         });

diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs
@@ -328,6 +328,8 @@ pub struct FunctionDefinition {
     /// True if this function was defined with the 'open' keyword
     pub is_open: bool,
 
+    pub is_internal: bool,
+
     /// True if this function was defined with the 'unconstrained' keyword
     pub is_unconstrained: bool,
 

diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs
@@ -64,6 +64,8 @@ pub enum ResolverError {
     MutableReferenceToImmutableVariable { variable: String, span: Span },
     #[error("Mutable references to array indices are unsupported")]
     MutableReferenceToArrayElement { span: Span },
+    #[error("Function is not defined in a contract yet sets is_internal")]
+    ContractFunctionInternalInNormalFunction { span: Span },
 }
 
 impl ResolverError {
@@ -268,6 +270,11 @@ impl From<ResolverError> for Diagnostic {
             ResolverError::MutableReferenceToArrayElement { span } => {
                 Diagnostic::simple_error("Mutable references to array elements are currently unsupported".into(), "Try storing the element in a fresh variable first".into(), span)
             },
+            ResolverError::ContractFunctionInternalInNormalFunction { span } => Diagnostic::simple_error(
+                "Only functions defined within contracts can set their functions to be internal".into(),
+                "Non-contract functions cannot be 'internal'".into(),
+                span,
+            ),
         }
     }
 }
diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs
@@ -652,6 +652,7 @@ impl<'a> Resolver<'a> {
             kind: func.kind,
             attributes,
             contract_function_type: self.handle_function_type(func),
+            is_internal: self.handle_is_function_internal(func),
             is_unconstrained: func.def.is_unconstrained,
             location,
             typ,
@@ -697,6 +698,19 @@ impl<'a> Resolver<'a> {
         }
     }
 
+    fn handle_is_function_internal(&mut self, func: &NoirFunction) -> Option<bool> {
+        if self.in_contract() {
+            Some(func.def.is_internal)
+        } else {
+            if func.def.is_internal {
+                self.push_err(ResolverError::ContractFunctionInternalInNormalFunction {
+                    span: func.name_ident().span(),
+                });
+            }
+            None
+        }
+    }
+
     fn declare_numeric_generics(&mut self, params: &[Type], return_type: &Type) {
         if self.generics.is_empty() {
             return;

diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs
@@ -232,6 +232,7 @@ mod test {
             attributes: None,
             location,
             contract_function_type: None,
+            is_internal: None,
             is_unconstrained: false,
             typ: Type::Function(vec![Type::field(None), Type::field(None)], Box::new(Type::Unit)),
             parameters: vec![

diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs
@@ -121,10 +121,15 @@ pub struct FuncMeta {
     /// Attribute per function.
     pub attributes: Option<Attribute>,
 
-    /// This function's visibility in its contract.
+    /// This function's type in its contract.
     /// If this function is not in a contract, this is always 'Secret'.
     pub contract_function_type: Option<ContractFunctionType>,
 
+    /// This function's visibility.
+    /// If this function is internal can only be called by itself.
+    /// Will be None if not in contract.
+    pub is_internal: Option<bool>,
+
     pub is_unconstrained: bool,
 
     pub parameters: Parameters,

diff --git a/crates/noirc_frontend/src/lexer/token.rs b/crates/noirc_frontend/src/lexer/token.rs
@@ -431,6 +431,7 @@ pub enum Keyword {
     Impl,
     If,
     In,
+    Internal,
     Let,
     Mod,
     Mut,
@@ -466,6 +467,7 @@ impl fmt::Display for Keyword {
             Keyword::Impl => write!(f, "impl"),
             Keyword::If => write!(f, "if"),
             Keyword::In => write!(f, "in"),
+            Keyword::Internal => write!(f, "internal"),
             Keyword::Let => write!(f, "let"),
             Keyword::Mod => write!(f, "mod"),
             Keyword::Mut => write!(f, "mut"),
@@ -504,6 +506,7 @@ impl Keyword {
             "impl" => Keyword::Impl,
             "if" => Keyword::If,
             "in" => Keyword::In,
+            "internal" => Keyword::Internal,
             "let" => Keyword::Let,
             "mod" => Keyword::Mod,
             "mut" => Keyword::Mut,

diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs
@@ -162,7 +162,10 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
         .map(
             |(
                 (
-                    ((((attribute, (is_unconstrained, is_open)), name), generics), parameters),
+                    (
+                        (((attribute, (is_unconstrained, is_open, is_internal)), name), generics),
+                        parameters,
+                    ),
                     ((return_distinctness, return_visibility), return_type),
                 ),
                 body,
@@ -172,6 +175,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
                     name,
                     attribute, // 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
                     is_open,
+                    is_internal,
                     is_unconstrained,
                     generics,
                     parameters,
@@ -185,14 +189,16 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
         )
 }
 
-/// function_modifiers: 'unconstrained' 'open' | 'unconstrained' | 'open' | %empty
-///
-/// returns (is_unconstrained, is_open) for whether each keyword was present
-fn function_modifiers() -> impl NoirParser<(bool, bool)> {
+/// function_modifiers:  'open internal' | 'internal' | 'unconstrained' | 'open' | %empty
+/// returns (is_unconstrained, is_open, is_internal) for whether each keyword was present
+fn function_modifiers() -> impl NoirParser<(bool, bool, bool)> {
     keyword(Keyword::Unconstrained)
         .or_not()
         .then(keyword(Keyword::Open).or_not())
-        .map(|(unconstrained, open)| (unconstrained.is_some(), open.is_some()))
+        .then(keyword(Keyword::Internal).or_not())
+        .map(|((unconstrained, open), internal)| {
+            (unconstrained.is_some(), open.is_some(), internal.is_some())
+        })
 }
 
 /// non_empty_ident_list: ident ',' non_empty_ident_list