Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add crate for pub modifier #3271

Merged
merged 5 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)>,
Expand Down Expand Up @@ -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(),
Expand Down
8 changes: 8 additions & 0 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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),
Expand Down
53 changes: 39 additions & 14 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@
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;
Expand Down Expand Up @@ -89,7 +89,7 @@

/// True if the current module is a contract.
/// This is usually determined by self.path_resolver.module_id(), but it can
/// be overriden for impls. Impls are an odd case since the methods within resolve

Check warning on line 92 in compiler/noirc_frontend/src/hir/resolution/resolver.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (overriden)
/// as if they're in the parent module, but should be placed in a child module.
/// Since they should be within a child module, in_contract is manually set to false
/// for these so we can still resolve them in the parent module without them being in a contract.
Expand Down Expand Up @@ -1058,20 +1058,39 @@
}
}

// 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 });
}
}
}
}

Expand Down Expand Up @@ -1165,9 +1184,15 @@
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(_) => {}
Expand Down
12 changes: 6 additions & 6 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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,

Expand All @@ -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,
Expand Down Expand Up @@ -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 }),
Expand Down Expand Up @@ -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
}

Expand Down
43 changes: 32 additions & 11 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions docs/docs/language_concepts/01_functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
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.

Expand Down Expand Up @@ -161,7 +167,7 @@
The field can be defined implicitly, by using the name of the elliptic curve usually associated to it - for instance bn254, bls12_381 - or explicitly by using the field (prime) order, in decimal or hexadecimal form.
As a result, it is possible to define multiple versions of a function with each version specialized for a different field attribute. This can be useful when a function requires different parameters depending on the underlying elliptic curve.

Example: we define the function `foo()` three times below. Once for the default Noir bn254 curve, once for the field $\mathbb F_{23}$, which will normally never be used by Noir, and once again for the bls12_381 curve.

Check warning on line 170 in docs/docs/language_concepts/01_functions.md

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (mathbb)

```rust
#[field(bn254)]
Expand Down
Loading