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: visibility for type aliases #6058

Merged
merged 12 commits into from
Sep 25, 2024
6 changes: 4 additions & 2 deletions compiler/noirc_frontend/src/ast/type_alias.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{Ident, UnresolvedGenerics, UnresolvedType};
use super::{Ident, ItemVisibility, UnresolvedGenerics, UnresolvedType};
use iter_extended::vecmap;
use noirc_errors::Span;
use std::fmt::Display;
Expand All @@ -9,6 +9,7 @@ pub struct NoirTypeAlias {
pub name: Ident,
pub generics: UnresolvedGenerics,
pub typ: UnresolvedType,
pub visibility: ItemVisibility,
pub span: Span,
}

Expand All @@ -17,9 +18,10 @@ impl NoirTypeAlias {
name: Ident,
generics: UnresolvedGenerics,
typ: UnresolvedType,
visibility: ItemVisibility,
span: Span,
) -> NoirTypeAlias {
NoirTypeAlias { name, generics, typ, span }
NoirTypeAlias { name, generics, typ, visibility, span }
}
}

Expand Down
93 changes: 93 additions & 0 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1131,13 +1131,106 @@ impl<'context> Elaborator<'context> {
self.file = alias.file_id;
self.local_module = alias.module_id;

let name = &alias.type_alias_def.name;
let visibility = alias.type_alias_def.visibility;
let span = alias.type_alias_def.typ.span;

let generics = self.add_generics(&alias.type_alias_def.generics);
self.current_item = Some(DependencyId::Alias(alias_id));
let typ = self.resolve_type(alias.type_alias_def.typ);

if visibility != ItemVisibility::Private {
self.check_aliased_type_is_not_more_private(name, visibility, &typ, span);
}

self.interner.set_type_alias(alias_id, typ, generics);
self.generics.clear();
}

fn check_aliased_type_is_not_more_private(
&mut self,
name: &Ident,
visibility: ItemVisibility,
typ: &Type,
span: Span,
) {
match typ {
Type::Struct(struct_type, generics) => {
let struct_type = struct_type.borrow();
let struct_module_id = struct_type.id.module_id();

// We only check this in types in the same crate. If it's in a different crate
// then it's either accessible (all good) or it's not, in which case a different
// error will happen somewhere else, but no need to error again here.
if struct_module_id.krate == self.crate_id {
// Go to the struct's parent module
let module_data = self.get_module(struct_module_id);
let parent_local_id =
module_data.parent.expect("Expected struct module parent to exist");
let parent_module_id =
ModuleId { krate: struct_module_id.krate, local_id: parent_local_id };
let parent_module_data = self.get_module(parent_module_id);

// Find the struct in the parent module so we can know its visibility
let per_ns = parent_module_data.find_name(&struct_type.name);
if let Some((_, aliased_visibility, _)) = per_ns.types {
if aliased_visibility < visibility {
self.push_err(ResolverError::TypeIsMorePrivateThenItem {
typ: struct_type.name.to_string(),
item: name.to_string(),
span,
});
}
}
}

for generic in generics {
self.check_aliased_type_is_not_more_private(name, visibility, generic, span);
}
}
Type::Tuple(types) => {
for typ in types {
self.check_aliased_type_is_not_more_private(name, visibility, typ, span);
}
}
Type::Alias(alias_type, generics) => {
self.check_aliased_type_is_not_more_private(
name,
visibility,
&alias_type.borrow().get_type(generics),
span,
);
}
Type::Function(args, return_type, env, _) => {
for arg in args {
self.check_aliased_type_is_not_more_private(name, visibility, arg, span);
}
self.check_aliased_type_is_not_more_private(name, visibility, return_type, span);
self.check_aliased_type_is_not_more_private(name, visibility, env, span);
}
Type::MutableReference(typ) | Type::Array(_, typ) | Type::Slice(typ) => {
self.check_aliased_type_is_not_more_private(name, visibility, typ, span);
}
Type::InfixExpr(left, _op, right) => {
self.check_aliased_type_is_not_more_private(name, visibility, left, span);
self.check_aliased_type_is_not_more_private(name, visibility, right, span);
}
Type::FieldElement
| Type::Integer(..)
| Type::Bool
| Type::String(..)
| Type::FmtString(..)
| Type::Unit
| Type::Quoted(..)
| Type::TypeVariable(..)
| Type::Forall(..)
| Type::TraitAsType(..)
| Type::Constant(..)
| Type::NamedGeneric(..)
| Type::Error => (),
}
}

fn collect_struct_definitions(&mut self, structs: &BTreeMap<StructId, UnresolvedStruct>) {
// This is necessary to avoid cloning the entire struct map
// when adding checks after each struct field is resolved.
Expand Down
22 changes: 19 additions & 3 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ impl<'a> ModCollector<'a> {
let doc_comments = type_alias.doc_comments;
let type_alias = type_alias.item;
let name = type_alias.name.clone();
let visibility = type_alias.visibility;

// And store the TypeId -> TypeAlias mapping somewhere it is reachable
let unresolved = UnresolvedTypeAlias {
Expand All @@ -327,8 +328,19 @@ impl<'a> ModCollector<'a> {
context.def_interner.set_doc_comments(ReferenceId::Alias(type_alias_id), doc_comments);

// Add the type alias to scope so its path can be looked up later
let result = self.def_collector.def_map.modules[self.module_id.0]
.declare_type_alias(name.clone(), type_alias_id);
let result = self.def_collector.def_map.modules[self.module_id.0].declare_type_alias(
name.clone(),
visibility,
type_alias_id,
);

let parent_module_id = ModuleId { krate, local_id: self.module_id };
context.def_interner.usage_tracker.add_unused_item(
parent_module_id,
name.clone(),
UnusedItem::TypeAlias(type_alias_id),
visibility,
);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::Duplicate {
Expand Down Expand Up @@ -526,7 +538,11 @@ impl<'a> ModCollector<'a> {
TraitItem::Type { name } => {
if let Err((first_def, second_def)) = self.def_collector.def_map.modules
[trait_id.0.local_id.0]
.declare_type_alias(name.clone(), TypeAliasId::dummy_id())
.declare_type_alias(
name.clone(),
ItemVisibility::Public,
TypeAliasId::dummy_id(),
)
{
let error = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::TraitAssociatedType,
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/hir/def_map/module_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ impl ModuleData {
pub fn declare_type_alias(
&mut self,
name: Ident,
visibility: ItemVisibility,
id: TypeAliasId,
) -> Result<(), (Ident, Ident)> {
self.declare(name, ItemVisibility::Public, id.into(), None)
self.declare(name, visibility, id.into(), None)
}

pub fn declare_trait(
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ pub enum ResolverError {
MutatingComptimeInNonComptimeContext { name: String, span: Span },
#[error("Failed to parse `{statement}` as an expression")]
InvalidInternedStatementInExpr { statement: String, span: Span },
#[error("Type `{typ}` is more private than item `{item}`")]
TypeIsMorePrivateThenItem { typ: String, item: String, span: Span },
}

impl ResolverError {
Expand Down Expand Up @@ -529,6 +531,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
*span,
)
},
ResolverError::TypeIsMorePrivateThenItem { typ, item, span } => {
Diagnostic::simple_warning(
format!("Type `{typ}` is more private than item `{item}`"),
String::new(),
*span,
)
},
}
}
}
23 changes: 15 additions & 8 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,21 @@ fn contract(
fn type_alias_definition() -> impl NoirParser<TopLevelStatementKind> {
use self::Keyword::Type;

let p = ignore_then_commit(keyword(Type), ident());
let p = then_commit(p, function::generics());
let p = then_commit_ignore(p, just(Token::Assign));
let p = then_commit(p, parse_type());

p.map_with_span(|((name, generics), typ), span| {
TopLevelStatementKind::TypeAlias(NoirTypeAlias { name, generics, typ, span })
})
item_visibility()
.then_ignore(keyword(Type))
.then(ident())
.then(function::generics())
.then_ignore(just(Token::Assign))
.then(parse_type())
.map_with_span(|(((visibility, name), generics), typ), span| {
TopLevelStatementKind::TypeAlias(NoirTypeAlias {
name,
generics,
typ,
visibility,
span,
})
})
}

fn self_parameter() -> impl NoirParser<Param> {
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2659,8 +2659,8 @@ fn incorrect_generic_count_on_struct_impl() {
#[test]
fn incorrect_generic_count_on_type_alias() {
let src = r#"
struct Foo {}
type Bar = Foo<i32>;
pub struct Foo {}
pub type Bar = Foo<i32>;
fn main() {}
"#;

Expand Down
74 changes: 74 additions & 0 deletions compiler/noirc_frontend/src/tests/unused_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,77 @@ fn silences_unused_variable_warning() {
"#;
assert_no_errors(src);
}

#[test]
fn errors_on_unused_type_alias() {
let src = r#"
type Foo = Field;
type Bar = Field;
pub fn bar(_: Bar) {}
fn main() {}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
&errors[0].0
else {
panic!("Expected an unused item error");
};

assert_eq!(ident.to_string(), "Foo");
assert_eq!(*item_type, "type alias");
}

#[test]
fn errors_if_type_alias_aliases_more_private_type() {
let src = r#"
struct Foo {}
pub type Bar = Foo;
pub fn no_unused_warnings(_b: Bar) {
let _ = Foo {};
}
fn main() {}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem {
typ, item, ..
}) = &errors[0].0
else {
panic!("Expected an unused item error");
};

assert_eq!(typ, "Foo");
assert_eq!(item, "Bar");
}

#[test]
fn errors_if_type_alias_aliases_more_private_type_in_generic() {
let src = r#"
pub struct Generic<T> { value: T }
struct Foo {}
pub type Bar = Generic<Foo>;
pub fn no_unused_warnings(_b: Bar) {
let _ = Foo {};
let _ = Generic { value: 1 };
}
fn main() {}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem {
typ, item, ..
}) = &errors[0].0
else {
panic!("Expected an unused item error");
};

assert_eq!(typ, "Foo");
assert_eq!(item, "Bar");
}
4 changes: 3 additions & 1 deletion compiler/noirc_frontend/src/usage_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
ast::{Ident, ItemVisibility},
hir::def_map::ModuleId,
macros_api::StructId,
node_interner::{FuncId, TraitId},
node_interner::{FuncId, TraitId, TypeAliasId},
};

#[derive(Debug)]
Expand All @@ -13,6 +13,7 @@ pub enum UnusedItem {
Function(FuncId),
Struct(StructId),
Trait(TraitId),
TypeAlias(TypeAliasId),
}

impl UnusedItem {
Expand All @@ -22,6 +23,7 @@ impl UnusedItem {
UnusedItem::Function(_) => "function",
UnusedItem::Struct(_) => "struct",
UnusedItem::Trait(_) => "trait",
UnusedItem::TypeAlias(_) => "type alias",
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions docs/docs/noir/concepts/data_types/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ type Bad2 = Bad1;
// ^^^^^^^^^^^ 'Bad2' recursively depends on itself: Bad2 -> Bad1 -> Bad2
```

By default, like functions, type aliases are private to the module the exist in. You can use `pub`
to make the type alias public or `pub(crate)` to make it public to just its crate:

```rust
// This type alias is now public
pub type Id = u8;
```

## Wildcard Type
Noir can usually infer the type of the variable from the context, so specifying the type of a variable is only required when it cannot be inferred. However, specifying a complex type can be tedious, especially when it has multiple generic arguments. Often some of the generic types can be inferred from the context, and Noir only needs a hint to properly infer the other types. We can partially specify a variable's type by using `_` as a marker, indicating where we still want the compiler to infer the type.

Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/meta/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use crate::hash::poseidon2::Poseidon2Hasher;

// A derive function is one that given a struct definition can
// create us a quoted trait impl from it.
type DeriveFunction = fn(StructDefinition) -> Quoted;
pub type DeriveFunction = fn(StructDefinition) -> Quoted;

// We'll keep a global HANDLERS map to keep track of the derive handler for each trait
comptime mut global HANDLERS: UHashMap<TraitDefinition, DeriveFunction, BuildHasherDefault<Poseidon2Hasher>> = UHashMap::default();
Expand Down
Loading