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

chore: generalise FunctionVisibility to ItemVisibility #4495

Merged
merged 7 commits into from
Mar 12, 2024
17 changes: 9 additions & 8 deletions aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,19 @@
use noirc_frontend::macros_api::FieldElement;
use noirc_frontend::macros_api::{
BlockExpression, CallExpression, CastExpression, Distinctness, Expression, ExpressionKind,
ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility,
HirContext, HirExpression, HirLiteral, HirStatement, Ident, IndexExpression, LetStatement,
Literal, MemberAccessExpression, MethodCallExpression, NoirFunction, NoirStruct, Param, Path,
PathKind, Pattern, PrefixExpression, SecondaryAttribute, Signedness, Span, Statement,
StatementKind, StructType, Type, TypeImpl, UnaryOp, UnresolvedType, UnresolvedTypeData,
Visibility,
ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, HirContext, HirExpression,
HirLiteral, HirStatement, Ident, IndexExpression, LetStatement, Literal,
MemberAccessExpression, MethodCallExpression, NoirFunction, NoirStruct, Param, Path, PathKind,
Pattern, PrefixExpression, SecondaryAttribute, Signedness, Span, Statement, StatementKind,
StructType, Type, TypeImpl, UnaryOp, UnresolvedType, UnresolvedTypeData, Visibility,
};
use noirc_frontend::macros_api::{CrateId, FileId};
use noirc_frontend::macros_api::{MacroError, MacroProcessor};
use noirc_frontend::macros_api::{ModuleDefId, NodeInterner, SortedModule, StructId};
use noirc_frontend::node_interner::{FuncId, TraitId, TraitImplId, TraitImplKind};
use noirc_frontend::{BinaryOpKind, ConstrainKind, ConstrainStatement, InfixExpression, Lambda};
use noirc_frontend::{
BinaryOpKind, ConstrainKind, ConstrainStatement, InfixExpression, ItemVisibility, Lambda,
};
pub struct AztecMacro;

impl MacroProcessor for AztecMacro {
Expand Down Expand Up @@ -445,7 +446,7 @@
} else if is_custom_attribute(&secondary_attribute, "aztec(initializer)") {
is_initializer = true;
insert_init_check = false;
} else if is_custom_attribute(&secondary_attribute, "aztec(noinitcheck)") {

Check warning on line 449 in aztec_macros/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (noinitcheck)
insert_init_check = false;
} else if is_custom_attribute(&secondary_attribute, "aztec(internal)") {
is_internal = true;
Expand Down Expand Up @@ -1100,7 +1101,7 @@
&return_type,
);

selector_fn_def.visibility = FunctionVisibility::Public;
selector_fn_def.visibility = ItemVisibility::Public;

// Seems to be necessary on contract modules
selector_fn_def.return_visibility = Visibility::Public;
Expand Down Expand Up @@ -1754,8 +1755,8 @@
// If compute_note_hash_and_nullifier is already defined by the user, we skip auto-generation in order to provide an
// escape hatch for this mechanism.
// TODO(#4647): improve this diagnosis and error messaging.
if collected_functions.iter().any(|coll_funcs_data| {

Check warning on line 1758 in aztec_macros/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (funcs)
check_for_compute_note_hash_and_nullifier_definition(&coll_funcs_data.functions, module_id)

Check warning on line 1759 in aztec_macros/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (funcs)
}) {
return Ok(());
}
Expand Down
6 changes: 3 additions & 3 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, FunctionVisibility, Ident, Path, Pattern, Recoverable, Statement, StatementKind,
Distinctness, Ident, ItemVisibility, Path, Pattern, Recoverable, Statement, StatementKind,
UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
};
use acvm::FieldElement;
Expand Down Expand Up @@ -378,7 +378,7 @@ pub struct FunctionDefinition {
pub is_unconstrained: bool,

/// Indicate if this function was defined with the 'pub' keyword
pub visibility: FunctionVisibility,
pub visibility: ItemVisibility,

pub generics: UnresolvedGenerics,
pub parameters: Vec<Param>,
Expand Down Expand Up @@ -677,7 +677,7 @@ impl FunctionDefinition {
is_open: false,
is_internal: false,
is_unconstrained: false,
visibility: FunctionVisibility::Private,
visibility: ItemVisibility::Private,
generics: generics.clone(),
parameters: p,
body: body.clone(),
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,8 @@ impl UnresolvedTypeExpression {
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
/// Represents whether the function can be called outside its module/crate
pub enum FunctionVisibility {
/// Represents whether the definition can be referenced outside its module/crate
pub enum ItemVisibility {
Public,
Private,
PublicCrate,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ impl<'a> ModCollector<'a> {

let modifiers = FunctionModifiers {
name: name.to_string(),
visibility: crate::FunctionVisibility::Public,
visibility: crate::ItemVisibility::Public,
// TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
attributes: crate::token::Attributes::empty(),
is_unconstrained: false,
Expand Down
13 changes: 4 additions & 9 deletions compiler/noirc_frontend/src/hir/def_map/item_scope.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
use super::{namespace::PerNs, ModuleDefId, ModuleId};
use crate::{
node_interner::{FuncId, TraitId},
Ident,
Ident, ItemVisibility,
};
use std::collections::{hash_map::Entry, HashMap};

type Scope = HashMap<Option<TraitId>, (ModuleDefId, Visibility, bool /*is_prelude*/)>;

#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub enum Visibility {
Public,
}
type Scope = HashMap<Option<TraitId>, (ModuleDefId, ItemVisibility, bool /*is_prelude*/)>;

#[derive(Default, Debug, PartialEq, Eq)]
pub struct ItemScope {
Expand Down Expand Up @@ -55,12 +50,12 @@ impl ItemScope {
Err((old_ident.clone(), name))
}
} else {
trait_hashmap.insert(trait_id, (mod_def, Visibility::Public, is_prelude));
trait_hashmap.insert(trait_id, (mod_def, ItemVisibility::Public, is_prelude));
Ok(())
}
} else {
let mut trait_hashmap = HashMap::new();
trait_hashmap.insert(trait_id, (mod_def, Visibility::Public, is_prelude));
trait_hashmap.insert(trait_id, (mod_def, ItemVisibility::Public, is_prelude));
map.insert(name, trait_hashmap);
Ok(())
}
Expand Down
11 changes: 6 additions & 5 deletions compiler/noirc_frontend/src/hir/def_map/namespace.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use super::{item_scope::Visibility, ModuleDefId};
use super::ModuleDefId;
use crate::ItemVisibility;

// This works exactly the same as in r-a, just simplified
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub struct PerNs {
pub types: Option<(ModuleDefId, Visibility, bool)>,
pub values: Option<(ModuleDefId, Visibility, bool)>,
pub types: Option<(ModuleDefId, ItemVisibility, bool)>,
pub values: Option<(ModuleDefId, ItemVisibility, bool)>,
}

impl PerNs {
pub fn types(t: ModuleDefId) -> PerNs {
PerNs { types: Some((t, Visibility::Public, false)), values: None }
PerNs { types: Some((t, ItemVisibility::Public, false)), values: None }
}

pub fn take_types(self) -> Option<ModuleDefId> {
Expand All @@ -24,7 +25,7 @@ impl PerNs {
self.types.map(|it| it.0).into_iter().chain(self.values.map(|it| it.0))
}

pub fn iter_items(self) -> impl Iterator<Item = (ModuleDefId, Visibility, bool)> {
pub fn iter_items(self) -> impl Iterator<Item = (ModuleDefId, ItemVisibility, bool)> {
self.types.into_iter().chain(self.values)
}

Expand Down
18 changes: 8 additions & 10 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ use crate::{
};
use crate::{
ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionDefinition,
FunctionReturnType, FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param,
Path, PathKind, Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind,
FunctionReturnType, Generics, ItemVisibility, LValue, NoirStruct, NoirTypeAlias, Param, Path,
PathKind, Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind,
UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
UnresolvedTypeExpression, Visibility, ERROR_IDENT,
};
Expand Down Expand Up @@ -237,7 +237,7 @@ impl<'a> Resolver<'a> {
is_open: false,
is_internal: false,
is_unconstrained: false,
visibility: FunctionVisibility::Public, // Trait functions are always public
visibility: ItemVisibility::Public, // Trait functions are always public
generics: generics.clone(),
parameters: vecmap(parameters, |(name, typ)| Param {
visibility: Visibility::Private,
Expand Down Expand Up @@ -1320,7 +1320,7 @@ impl<'a> Resolver<'a> {
&mut self,
func: FuncId,
span: Span,
visibility: FunctionVisibility,
visibility: ItemVisibility,
) {
let function_module = self.interner.function_module(func);
let current_module = self.path_resolver.module_id();
Expand All @@ -1330,8 +1330,8 @@ impl<'a> Resolver<'a> {
let current_module = current_module.local_id;
let name = self.interner.function_name(&func).to_string();
match visibility {
FunctionVisibility::Public => (),
FunctionVisibility::Private => {
ItemVisibility::Public => (),
ItemVisibility::Private => {
if !same_crate
|| !self.module_descendent_of_target(
krate,
Expand All @@ -1342,7 +1342,7 @@ impl<'a> Resolver<'a> {
self.errors.push(ResolverError::PrivateFunctionCalled { span, name });
}
}
FunctionVisibility::PublicCrate => {
ItemVisibility::PublicCrate => {
if !same_crate {
self.errors.push(ResolverError::NonCrateFunctionCalled { span, name });
}
Expand Down Expand Up @@ -1451,9 +1451,7 @@ impl<'a> Resolver<'a> {
self.interner.add_function_dependency(current_item, id);
}

if self.interner.function_visibility(id)
!= FunctionVisibility::Public
{
if self.interner.function_visibility(id) != ItemVisibility::Public {
let span = hir_ident.location.span;
self.check_can_reference_function(
id,
Expand Down
11 changes: 5 additions & 6 deletions compiler/noirc_frontend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,13 @@ pub mod macros_api {
pub use crate::hir::def_map::ModuleDefId;
pub use crate::{
hir::Context as HirContext, BlockExpression, CallExpression, CastExpression, Distinctness,
Expression, ExpressionKind, FunctionReturnType, Ident, IndexExpression, LetStatement,
Literal, MemberAccessExpression, MethodCallExpression, NoirFunction, Path, PathKind,
Pattern, Statement, UnresolvedType, UnresolvedTypeData, Visibility,
Expression, ExpressionKind, FunctionReturnType, Ident, IndexExpression, ItemVisibility,
LetStatement, Literal, MemberAccessExpression, MethodCallExpression, NoirFunction, Path,
PathKind, Pattern, Statement, UnresolvedType, UnresolvedTypeData, Visibility,
};
pub use crate::{
ForLoopStatement, ForRange, FunctionDefinition, FunctionVisibility, ImportStatement,
NoirStruct, Param, PrefixExpression, Signedness, StatementKind, StructType, Type, TypeImpl,
UnaryOp,
ForLoopStatement, ForRange, FunctionDefinition, ImportStatement, NoirStruct, Param,
PrefixExpression, Signedness, StatementKind, StructType, Type, TypeImpl, UnaryOp,
};

/// Methods to process the AST before and after type checking
Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::hir_def::{
};
use crate::token::{Attributes, SecondaryAttribute};
use crate::{
BinaryOpKind, ContractFunctionType, FunctionDefinition, FunctionVisibility, Generics, Shared,
BinaryOpKind, ContractFunctionType, FunctionDefinition, Generics, ItemVisibility, Shared,
TypeAlias, TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind,
};

Expand Down Expand Up @@ -236,7 +236,7 @@ pub struct FunctionModifiers {
pub name: String,

/// Whether the function is `pub` or not.
pub visibility: FunctionVisibility,
pub visibility: ItemVisibility,

pub attributes: Attributes,

Expand All @@ -259,7 +259,7 @@ impl FunctionModifiers {
pub fn new() -> Self {
Self {
name: String::new(),
visibility: FunctionVisibility::Public,
visibility: ItemVisibility::Public,
attributes: Attributes::empty(),
is_unconstrained: false,
is_internal: None,
Expand Down Expand Up @@ -799,7 +799,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) -> FunctionVisibility {
pub fn function_visibility(&self, func: FuncId) -> ItemVisibility {
self.function_modifiers[&func].visibility
}

Expand Down
12 changes: 6 additions & 6 deletions compiler/noirc_frontend/src/parser/parser/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::parser::labels::ParsingRuleLabel;
use crate::parser::spanned;
use crate::token::{Keyword, Token};
use crate::{
Distinctness, FunctionDefinition, FunctionReturnType, FunctionVisibility, Ident, NoirFunction,
Distinctness, FunctionDefinition, FunctionReturnType, Ident, ItemVisibility, NoirFunction,
Param, Visibility,
};

Expand Down Expand Up @@ -53,24 +53,24 @@ pub(super) fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunct
}

/// visibility_modifier: 'pub(crate)'? 'pub'? ''
fn visibility_modifier() -> impl NoirParser<FunctionVisibility> {
fn visibility_modifier() -> impl NoirParser<ItemVisibility> {
let is_pub_crate = (keyword(Keyword::Pub)
.then_ignore(just(Token::LeftParen))
.then_ignore(keyword(Keyword::Crate))
.then_ignore(just(Token::RightParen)))
.map(|_| FunctionVisibility::PublicCrate);
.map(|_| ItemVisibility::PublicCrate);

let is_pub = keyword(Keyword::Pub).map(|_| FunctionVisibility::Public);
let is_pub = keyword(Keyword::Pub).map(|_| ItemVisibility::Public);

let is_private = empty().map(|_| FunctionVisibility::Private);
let is_private = empty().map(|_| ItemVisibility::Private);

choice((is_pub_crate, is_pub, is_private))
}

/// function_modifiers: 'unconstrained'? (visibility)? 'open'?
///
/// returns (is_unconstrained, visibility, is_open) for whether each keyword was present
fn function_modifiers() -> impl NoirParser<(bool, FunctionVisibility, bool)> {
fn function_modifiers() -> impl NoirParser<(bool, ItemVisibility, bool)> {
keyword(Keyword::Unconstrained)
.or_not()
.then(visibility_modifier())
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_frontend/src/parser/parser/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
ParserErrorReason, TopLevelStatement,
},
token::{Keyword, Token},
Expression, FunctionVisibility, NoirTrait, NoirTraitImpl, TraitBound, TraitImplItem, TraitItem,
Expression, ItemVisibility, NoirTrait, NoirTraitImpl, TraitBound, TraitImplItem, TraitItem,
UnresolvedTraitConstraint, UnresolvedType,
};

Expand Down Expand Up @@ -123,12 +123,12 @@ fn trait_implementation_body() -> impl NoirParser<Vec<TraitImplItem>> {
if f.def().is_internal
|| f.def().is_unconstrained
|| f.def().is_open
|| f.def().visibility != FunctionVisibility::Private
|| f.def().visibility != ItemVisibility::Private
{
emit(ParserError::with_reason(ParserErrorReason::TraitImplFunctionModifiers, span));
}
// Trait impl functions are always public
f.def_mut().visibility = FunctionVisibility::Public;
f.def_mut().visibility = ItemVisibility::Public;
TraitImplItem::Function(f)
});

Expand Down
Loading