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(traits): added checks for duplicated trait associated items (types, consts, functions) #2927

Merged
merged 16 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
10 changes: 3 additions & 7 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ fn collect_trait_impl_methods(

if overrides.len() > 1 {
let error = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::Function,
typ: DuplicateType::TraitAssociatedFunction,
first_def: overrides[0].2.name_ident().clone(),
second_def: overrides[1].2.name_ident().clone(),
};
Expand Down Expand Up @@ -542,7 +542,7 @@ fn collect_trait_impl(

if let Some(struct_type) = get_struct_type(&typ) {
errors.extend(take_errors(trait_impl.file_id, resolver));
let current_def_map = def_maps.get_mut(&crate_id).unwrap();
let current_def_map = def_maps.get_mut(&struct_type.borrow().id.krate()).unwrap();
match add_method_to_struct_namespace(
current_def_map,
struct_type,
Expand Down Expand Up @@ -801,11 +801,7 @@ fn resolve_trait_methods(
.iter()
.filter(|(_, _, q)| q.name() == name.0.contents)
.collect();
let default_impl = if !default_impl_list.is_empty() {
if default_impl_list.len() > 1 {
// TODO(nickysn): Add check for method duplicates in the trait and emit proper error messages. This is planned in a future PR.
panic!("Too many functions with the same name!");
}
let default_impl = if default_impl_list.len() == 1 {
Some(Box::new(default_impl_list[0].2.clone()))
} else {
None
Expand Down
86 changes: 68 additions & 18 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use noirc_errors::Location;
use crate::{
graph::CrateId,
hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
node_interner::TraitId,
node_interner::{TraitId, TypeAliasId},
parser::{SortedModule, SubModule},
FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl,
NoirTypeAlias, TraitImplItem, TraitItem, TypeImpl,
Expand Down Expand Up @@ -360,27 +360,77 @@ impl<'a> ModCollector<'a> {
trait_id: None,
};
for trait_item in &trait_definition.items {
// TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
if let TraitItem::Function {
name,
generics,
parameters,
return_type,
where_clause,
body: Some(body),
} = trait_item
{
let func_id = context.def_interner.push_empty_fn();

let impl_method = NoirFunction::normal(FunctionDefinition::normal(
match trait_item {
TraitItem::Function {
name,
generics,
parameters,
body,
where_clause,
return_type,
));
unresolved_functions.push_fn(self.module_id, func_id, impl_method);
where_clause,
body,
} => {
let func_id = context.def_interner.push_empty_fn();
match self.def_collector.def_map.modules[id.0.local_id.0]
.declare_function(name.clone(), func_id)
{
Ok(()) => {
// TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
if let Some(body) = body {
let impl_method =
NoirFunction::normal(FunctionDefinition::normal(
name,
generics,
parameters,
body,
where_clause,
return_type,
));
unresolved_functions.push_fn(
self.module_id,
func_id,
impl_method,
);
}
}
Err((first_def, second_def)) => {
let error = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::TraitAssociatedFunction,
first_def,
second_def,
};
errors.push((error.into(), self.file_id));
}
}
}
TraitItem::Constant { name, .. } => {
let stmt_id = context.def_interner.push_empty_global();

if let Err((first_def, second_def)) = self.def_collector.def_map.modules
[id.0.local_id.0]
.declare_global(name.clone(), stmt_id)
{
let error = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::TraitAssociatedConst,
first_def,
second_def,
};
errors.push((error.into(), self.file_id));
}
}
TraitItem::Type { name } => {
// TODO(nickysn or alexvitkov): implement context.def_interner.push_empty_type_alias and get an id, instead of using TypeAliasId::dummy_id()
if let Err((first_def, second_def)) = self.def_collector.def_map.modules
[id.0.local_id.0]
.declare_type_alias(name.clone(), TypeAliasId::dummy_id())
{
let error = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::TraitAssociatedType,
first_def,
second_def,
};
errors.push((error.into(), self.file_id));
}
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions compiler/noirc_frontend/src/hir/def_collector/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ pub enum DuplicateType {
Import,
Trait,
TraitImplementation,
TraitAssociatedType,
TraitAssociatedConst,
TraitAssociatedFunction,
}

#[derive(Error, Debug, Clone)]
Expand Down Expand Up @@ -79,6 +82,9 @@ impl fmt::Display for DuplicateType {
DuplicateType::Trait => write!(f, "trait definition"),
DuplicateType::TraitImplementation => write!(f, "trait implementation"),
DuplicateType::Import => write!(f, "import"),
DuplicateType::TraitAssociatedType => write!(f, "trait associated type"),
DuplicateType::TraitAssociatedConst => write!(f, "trait associated constant"),
DuplicateType::TraitAssociatedFunction => write!(f, "trait associated function"),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dup_trait_items_1"
type = "bin"
authors = [""]
compiler_version = "0.15.0"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait MyTrait {
fn SomeFunc();
fn SomeFunc();
}

fn main() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dup_trait_items_2"
type = "bin"
authors = [""]
compiler_version = "0.15.0"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait MyTrait {
let SomeConst: u32;
let SomeConst: Field;
}

fn main() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dup_trait_items_3"
type = "bin"
authors = [""]
compiler_version = "0.15.0"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait MyTrait {
type SomeType;
type SomeType;
}

fn main() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dup_trait_items_4"
type = "bin"
authors = [""]
compiler_version = "0.15.0"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait MyTrait {
let MyItem: u32;
fn MyItem();
}

fn main() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dup_trait_items_5"
type = "bin"
authors = [""]
compiler_version = "0.15.0"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait MyTrait {
fn MyItem();
let MyItem: u32;
}

fn main() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dup_trait_items_6"
type = "bin"
authors = [""]
compiler_version = "0.15.0"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
trait MyTrait {
fn SomeFunc() { };
fn SomeFunc() { };
}

struct MyStruct {
}

impl MyTrait for MyStruct {
fn SomeFunc() {
}
}

fn main() {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "trait_allowed_item_name_matches"
type = "bin"
authors = [""]
compiler_version = "0.15.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
trait Trait1 {
// types and consts with the same name are allowed
type Tralala;
let Tralala: u32;
}

trait Trait2 {
// consts and types with the same name are allowed
let Tralala: u32;
type Tralala;
}

trait Trait3 {
// types and functions with the same name are allowed
type Tralala;
fn Tralala();
}

trait Trait4 {
// functions and types with the same name are allowed
fn Tralala();
type Tralala;
}

fn main() {
}
Loading