feat: add crate for pub modifier (#3271)

Co-authored-by: jfecher <jake@aztecprotocol.com>

compiler/noirc_frontend/src/ast/expression.rs

@@ -3,7 +3,7 @@ use std::fmt::Display;
 
 use crate::token::{Attributes, Token};
 use crate::{
-    Distinctness, Ident, Path, Pattern, Recoverable, Statement, StatementKind,
+    Distinctness, FunctionVisibility, Ident, Path, Pattern, Recoverable, Statement, StatementKind,
     UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
 };
 use acvm::FieldElement;
@@ -366,8 +366,8 @@ pub struct FunctionDefinition {
     /// True if this function was defined with the 'unconstrained' keyword
     pub is_unconstrained: bool,
 
-    /// True if this function was defined with the 'pub' keyword
-    pub is_public: bool,
+    /// Indicate if this function was defined with the 'pub' keyword
+    pub visibility: FunctionVisibility,
 
     pub generics: UnresolvedGenerics,
     pub parameters: Vec<(Pattern, UnresolvedType, Visibility)>,
@@ -644,7 +644,7 @@ impl FunctionDefinition {
             is_open: false,
             is_internal: false,
             is_unconstrained: false,
-            is_public: false,
+            visibility: FunctionVisibility::Private,
             generics: generics.clone(),
             parameters: p,
             body: body.clone(),

compiler/noirc_frontend/src/ast/mod.rs

@@ -270,6 +270,14 @@ impl UnresolvedTypeExpression {
     }
 }
 
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+/// Represents whether the function can be called outside its module/crate
+pub enum FunctionVisibility {
+    Public,
+    Private,
+    PublicCrate,
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 /// Represents whether the parameter is public or known only to the prover.
 pub enum Visibility {

compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -78,6 +78,8 @@ pub enum ResolverError {
     InvalidClosureEnvironment { typ: Type, span: Span },
     #[error("{name} is private and not visible from the current module")]
     PrivateFunctionCalled { name: String, span: Span },
+    #[error("{name} is not visible from the current crate")]
+    NonCrateFunctionCalled { name: String, span: Span },
     #[error("Only sized types may be used in the entry point to a program")]
     InvalidTypeForEntryPoint { span: Span },
 }
@@ -296,6 +298,9 @@ impl From<ResolverError> for Diagnostic {
             ResolverError::PrivateFunctionCalled { span, name } => Diagnostic::simple_warning(
                 format!("{name} is private and not visible from the current module"),
                 format!("{name} is private"), span),
+            ResolverError::NonCrateFunctionCalled { span, name } => Diagnostic::simple_warning(
+                    format!("{name} is not visible from the current crate"),
+                    format!("{name} is only visible within its crate"), span),
             ResolverError::InvalidTypeForEntryPoint { span } => Diagnostic::simple_error(
                 "Only sized types may be used in the entry point to a program".to_string(),
                 "Slices, references, or any type containing them may not be used in main or a contract function".to_string(), span),

compiler/noirc_frontend/src/hir/resolution/resolver.rs

@@ -36,10 +36,10 @@ use crate::{
     StatementKind,
 };
 use crate::{
-    ArrayLiteral, ContractFunctionType, Distinctness, Generics, LValue, NoirStruct, NoirTypeAlias,
-    Path, PathKind, Pattern, Shared, StructType, Type, TypeAliasType, TypeBinding, TypeVariable,
-    UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
-    UnresolvedTypeExpression, Visibility, ERROR_IDENT,
+    ArrayLiteral, ContractFunctionType, Distinctness, FunctionVisibility, Generics, LValue,
+    NoirStruct, NoirTypeAlias, Path, PathKind, Pattern, Shared, StructType, Type, TypeAliasType,
+    TypeBinding, TypeVariable, UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint,
+    UnresolvedType, UnresolvedTypeData, UnresolvedTypeExpression, Visibility, ERROR_IDENT,
 };
 use fm::FileId;
 use iter_extended::vecmap;
@@ -1058,20 +1058,39 @@ impl<'a> Resolver<'a> {
         }
     }
 
-    // Issue an error if the given private function is being called from a non-child module
-    fn check_can_reference_private_function(&mut self, func: FuncId, span: Span) {
+    // Issue an error if the given private function is being called from a non-child module, or
+    // if the given pub(crate) function is being called from another crate
+    fn check_can_reference_function(
+        &mut self,
+        func: FuncId,
+        span: Span,
+        visibility: FunctionVisibility,
+    ) {
         let function_module = self.interner.function_module(func);
         let current_module = self.path_resolver.module_id();
 
         let same_crate = function_module.krate == current_module.krate;
         let krate = function_module.krate;
         let current_module = current_module.local_id;
-
-        if !same_crate
-            || !self.module_descendent_of_target(krate, function_module.local_id, current_module)
-        {
-            let name = self.interner.function_name(&func).to_string();
-            self.errors.push(ResolverError::PrivateFunctionCalled { span, name });
+        let name = self.interner.function_name(&func).to_string();
+        match visibility {
+            FunctionVisibility::Public => (),
+            FunctionVisibility::Private => {
+                if !same_crate
+                    || !self.module_descendent_of_target(
+                        krate,
+                        function_module.local_id,
+                        current_module,
+                    )
+                {
+                    self.errors.push(ResolverError::PrivateFunctionCalled { span, name });
+                }
+            }
+            FunctionVisibility::PublicCrate => {
+                if !same_crate {
+                    self.errors.push(ResolverError::NonCrateFunctionCalled { span, name });
+                }
+            }
         }
     }
 
@@ -1165,9 +1184,15 @@ impl<'a> Resolver<'a> {
                     if hir_ident.id != DefinitionId::dummy_id() {
                         match self.interner.definition(hir_ident.id).kind {
                             DefinitionKind::Function(id) => {
-                                if self.interner.function_visibility(id) == Visibility::Private {
+                                if self.interner.function_visibility(id)
+                                    != FunctionVisibility::Public
+                                {
                                     let span = hir_ident.location.span;
-                                    self.check_can_reference_private_function(id, span);
+                                    self.check_can_reference_function(
+                                        id,
+                                        span,
+                                        self.interner.function_visibility(id),
+                                    );
                                 }
                             }
                             DefinitionKind::Global(_) => {}

compiler/noirc_frontend/src/node_interner.rs

@@ -21,8 +21,8 @@ use crate::hir_def::{
 };
 use crate::token::{Attributes, SecondaryAttribute};
 use crate::{
-    ContractFunctionType, FunctionDefinition, Generics, Shared, TypeAliasType, TypeBinding,
-    TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind, Visibility,
+    ContractFunctionType, FunctionDefinition, FunctionVisibility, Generics, Shared, TypeAliasType,
+    TypeBinding, TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind,
 };
 
 #[derive(Eq, PartialEq, Hash, Clone)]
@@ -132,7 +132,7 @@ pub struct FunctionModifiers {
     pub name: String,
 
     /// Whether the function is `pub` or not.
-    pub visibility: Visibility,
+    pub visibility: FunctionVisibility,
 
     pub attributes: Attributes,
 
@@ -155,7 +155,7 @@ impl FunctionModifiers {
     pub fn new() -> Self {
         Self {
             name: String::new(),
-            visibility: Visibility::Public,
+            visibility: FunctionVisibility::Public,
             attributes: Attributes::empty(),
             is_unconstrained: false,
             is_internal: None,
@@ -637,7 +637,7 @@ impl NodeInterner {
         // later during name resolution.
         let modifiers = FunctionModifiers {
             name: function.name.0.contents.clone(),
-            visibility: if function.is_public { Visibility::Public } else { Visibility::Private },
+            visibility: function.visibility,
             attributes: function.attributes.clone(),
             is_unconstrained: function.is_unconstrained,
             contract_function_type: Some(if function.is_open { Open } else { Secret }),
@@ -670,7 +670,7 @@ impl NodeInterner {
     ///
     /// The underlying function_visibilities map is populated during def collection,
     /// so this function can be called anytime afterward.
-    pub fn function_visibility(&self, func: FuncId) -> Visibility {
+    pub fn function_visibility(&self, func: FuncId) -> FunctionVisibility {
         self.function_modifiers[&func].visibility
     }
 

compiler/noirc_frontend/src/parser/parser.rs

@@ -38,9 +38,9 @@ use crate::parser::{force, ignore_then_commit, statement_recovery};
 use crate::token::{Attribute, Attributes, Keyword, SecondaryAttribute, Token, TokenKind};
 use crate::{
     BinaryOp, BinaryOpKind, BlockExpression, ConstrainStatement, Distinctness, FunctionDefinition,
-    FunctionReturnType, Ident, IfExpression, InfixExpression, LValue, Lambda, Literal,
-    NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Path, PathKind, Pattern,
-    Recoverable, Statement, TraitBound, TraitImplItem, TraitItem, TypeImpl, UnaryOp,
+    FunctionReturnType, FunctionVisibility, Ident, IfExpression, InfixExpression, LValue, Lambda,
+    Literal, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Path, PathKind,
+    Pattern, Recoverable, Statement, TraitBound, TraitImplItem, TraitItem, TypeImpl, UnaryOp,
     UnresolvedTraitConstraint, UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility,
 };
 
@@ -183,9 +183,15 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
                 name,
                 attributes: attrs,
                 is_unconstrained: modifiers.0,
-                is_open: modifiers.1,
-                is_internal: modifiers.2,
-                is_public: modifiers.3,
+                is_open: modifiers.2,
+                is_internal: modifiers.3,
+                visibility: if modifiers.1 {
+                    FunctionVisibility::PublicCrate
+                } else if modifiers.4 {
+                    FunctionVisibility::Public
+                } else {
+                    FunctionVisibility::Private
+                },
                 generics,
                 parameters,
                 body,
@@ -198,19 +204,34 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
         })
 }
 
-/// function_modifiers: 'unconstrained'? 'open'? 'internal'?
+/// function_modifiers: 'unconstrained'? 'pub(crate)'? 'pub'? 'open'? 'internal'?
 ///
-/// returns (is_unconstrained, is_open, is_internal) for whether each keyword was present
-fn function_modifiers() -> impl NoirParser<(bool, bool, bool, bool)> {
+/// returns (is_unconstrained, is_pub_crate, is_open, is_internal, is_pub) for whether each keyword was present
+fn function_modifiers() -> impl NoirParser<(bool, bool, bool, bool, bool)> {
     keyword(Keyword::Unconstrained)
         .or_not()
+        .then(is_pub_crate())
         .then(keyword(Keyword::Pub).or_not())
         .then(keyword(Keyword::Open).or_not())
         .then(keyword(Keyword::Internal).or_not())
-        .map(|(((unconstrained, public), open), internal)| {
-            (unconstrained.is_some(), open.is_some(), internal.is_some(), public.is_some())
+        .map(|((((unconstrained, pub_crate), public), open), internal)| {
+            (
+                unconstrained.is_some(),
+                pub_crate,
+                open.is_some(),
+                internal.is_some(),
+                public.is_some(),
+            )
         })
 }
+fn is_pub_crate() -> impl NoirParser<bool> {
+    (keyword(Keyword::Pub)
+        .then_ignore(just(Token::LeftParen))
+        .then_ignore(keyword(Keyword::Crate))
+        .then_ignore(just(Token::RightParen)))
+    .or_not()
+    .map(|a| a.is_some())
+}
 
 /// non_empty_ident_list: ident ',' non_empty_ident_list
 ///                     | ident

docs/docs/language_concepts/01_functions.md

@@ -20,6 +20,12 @@ By default, functions are visible only within the package they are defined. To m
 pub fn foo() {}
 ```
 
+You can also restrict the visibility of the function to only the crate it was defined in, by specifying `pub(crate)`:
+
+```rust
+pub(crate) fn foo() {}  //foo can only be called within its crate
+```
+
 All parameters in a function must have a type and all types are known at compile time. The parameter
 is pre-pended with a colon and the parameter type. Multiple parameters are separated using a comma.