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): Add default and override of methods #2585

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,4 @@ impl Default for Foo {


fn main(x: Field, y: Field) {
let first = Foo::default(x,y);
assert(first.bar == x);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dup_trait_implementation_2"
type = "bin"
authors = [""]
compiler_version = "0.9.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = 1
y = 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
trait Default {
}

struct Foo {
bar: Field,
}

// Duplicate trait implementations should not compile
impl Default for Foo {
}

// Duplicate trait implementations should not compile
impl Default for Foo {
}


fn main(x: Field, y: Field) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "dup_trait_implementation_3"
type = "bin"
authors = [""]
compiler_version = "0.9.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = 1
y = 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
trait Default {
}

struct MyStruct {
}

jfecher marked this conversation as resolved.
Show resolved Hide resolved
type MyType = MyStruct;

// Duplicate trait implementations should not compile
impl Default for MyStruct {
}

// Duplicate trait implementations should not compile
impl Default for MyType {
}


fn main(x: Field, y: Field) {
}
128 changes: 118 additions & 10 deletions crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use std::vec;

/// Stores all of the unresolved functions in a particular file/mod
#[derive(Clone)]
pub struct UnresolvedFunctions {
pub file_id: FileId,
pub functions: Vec<(LocalModuleId, FuncId, NoirFunction)>,
Expand All @@ -43,12 +44,21 @@
pub struct_def: NoirStruct,
}

#[derive(Clone)]
pub struct UnresolvedTrait {
pub file_id: FileId,
pub module_id: LocalModuleId,
pub trait_def: NoirTrait,
}

pub struct UnresolvedTraitImpl {
pub file_id: FileId,
pub module_id: LocalModuleId,
pub the_trait: UnresolvedTrait,
pub methods: UnresolvedFunctions,
pub trait_impl_ident: Ident, // for error reporting
}

#[derive(Clone)]
pub struct UnresolvedTypeAlias {
pub file_id: FileId,
Expand All @@ -74,7 +84,7 @@
pub(crate) collected_traits: BTreeMap<TraitId, UnresolvedTrait>,
pub(crate) collected_globals: Vec<UnresolvedGlobal>,
pub(crate) collected_impls: ImplMap,
pub(crate) collected_traits_impls: ImplMap,
pub(crate) collected_traits_impls: TraitImplMap,
}

/// Maps the type and the module id in which the impl is defined to the functions contained in that
Expand All @@ -87,6 +97,8 @@
type ImplMap =
HashMap<(UnresolvedType, LocalModuleId), Vec<(UnresolvedGenerics, Span, UnresolvedFunctions)>>;

type TraitImplMap = HashMap<(UnresolvedType, LocalModuleId, TraitId), UnresolvedTraitImpl>;

impl DefCollector {
fn new(def_map: CrateDefMap) -> DefCollector {
DefCollector {
Expand All @@ -97,8 +109,8 @@
collected_type_aliases: BTreeMap::new(),
collected_traits: BTreeMap::new(),
collected_impls: HashMap::new(),
collected_traits_impls: HashMap::new(),
collected_globals: vec![],
collected_traits_impls: HashMap::new(),
}
}

Expand Down Expand Up @@ -205,7 +217,7 @@
// impl since that determines the module we should collect into.
collect_impls(context, crate_id, &def_collector.collected_impls, errors);

collect_impls(context, crate_id, &def_collector.collected_traits_impls, errors);
collect_trait_impls(context, crate_id, &def_collector.collected_traits_impls, errors);

// Lower each function in the crate. This is now possible since imports have been resolved
let file_func_ids = resolve_free_functions(
Expand All @@ -225,13 +237,8 @@
errors,
);

let file_trait_impls_ids = resolve_impls(
&mut context.def_interner,
crate_id,
&context.def_maps,
def_collector.collected_traits_impls,
errors,
);
let file_trait_impls_ids =
resolve_trait_impls(context, def_collector.collected_traits_impls, crate_id, errors);

type_check_globals(&mut context.def_interner, file_global_ids, errors);

Expand Down Expand Up @@ -306,6 +313,58 @@
}
}

fn collect_trait_impls(
context: &mut Context,
crate_id: CrateId,
collected_impls: &TraitImplMap,
errors: &mut Vec<FileDiagnostic>,
) {
let interner = &mut context.def_interner;
let def_maps = &mut context.def_maps;

// TODO: To follow the semantics of Rust, we must allow the impl if either
// 1. The type is a struct and it's defined in the current crate
// 2. The trait is defined in the current crate
for ((unresolved_type, module_id, _), trait_impl) in collected_impls {
let path_resolver =
StandardPathResolver::new(ModuleId { local_id: *module_id, krate: crate_id });

for (_, func_id, ast) in &trait_impl.methods.functions {
let file = def_maps[&crate_id].file_id(*module_id);

let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);
resolver.add_generics(&ast.def.generics);
let typ = resolver.resolve_type(unresolved_type.clone());

// Add the method to the struct's namespace
if let Some(struct_type) = get_struct_type(&typ) {
extend_errors(errors, trait_impl.file_id, resolver.take_errors());

let struct_type = struct_type.borrow();
let type_module = struct_type.id.local_module_id();

let module = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0];

let result = module.declare_function(ast.name_ident().clone(), *func_id);

if let Err((first_def, second_def)) = result {
let err = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::Function,
first_def,
second_def,
};
errors.push(err.into_file_diagnostic(trait_impl.file_id));
}
} else {
let span = trait_impl.trait_impl_ident.span();
let trait_ident = trait_impl.the_trait.trait_def.name.clone();
let error = DefCollectorErrorKind::NonStructTraitImpl { trait_ident, span };
errors.push(error.into_file_diagnostic(trait_impl.file_id));
}
}
}
}

fn get_struct_type(typ: &Type) -> Option<&Shared<StructType>> {
match typ {
Type::Struct(definition, _) => Some(definition),
Expand Down Expand Up @@ -495,9 +554,9 @@
for (trait_id, unresolved_trait) in traits {
let mut items: Vec<TraitItemType> = vec![];
// Resolve order
// 1. Trait Types ( Trait contants can have a trait type, therefore types before constants)

Check warning on line 557 in crates/noirc_frontend/src/hir/def_collector/dc_crate.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (contants)
items.append(&mut resolve_trait_types(context, crate_id, &unresolved_trait, errors));
// 2. Trait Constants ( Trait's methods can use trait types & constants, threfore they should be after)

Check warning on line 559 in crates/noirc_frontend/src/hir/def_collector/dc_crate.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (threfore)
items.append(&mut resolve_trait_constants(context, crate_id, &unresolved_trait, errors));
// 3. Trait Methods
items.append(&mut resolve_trait_methods(context, crate_id, &unresolved_trait, errors));
Expand Down Expand Up @@ -601,6 +660,55 @@
file_method_ids
}

fn resolve_trait_impls(
context: &mut Context,
traits: TraitImplMap,
crate_id: CrateId,
errors: &mut Vec<FileDiagnostic>,
) -> Vec<(FileId, FuncId)> {
let interner = &mut context.def_interner;
let mut methods = Vec::<(FileId, FuncId)>::new();

for ((unresolved_type, _, trait_id), trait_impl) in traits {
let local_mod_id = trait_impl.module_id;
let module_id = ModuleId { krate: crate_id, local_id: local_mod_id };
let path_resolver = StandardPathResolver::new(module_id);

let self_type = {
let mut resolver =
Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id);
resolver.resolve_type(unresolved_type.clone())
};

let mut impl_methods = resolve_function_set(
interner,
crate_id,
&context.def_maps,
trait_impl.methods.clone(),
jfecher marked this conversation as resolved.
Show resolved Hide resolved
Some(self_type.clone()),
vec![], // TODO
errors,
);

let trait_definition_ident = &trait_impl.trait_impl_ident;
let key = (self_type.clone(), trait_id);
if let Some(prev_trait_impl_ident) = interner.get_previous_trait_implementation(&key) {
let err = DefCollectorErrorKind::Duplicate {
typ: DuplicateType::TraitImplementation,
first_def: prev_trait_impl_ident.clone(),
second_def: trait_definition_ident.clone(),
};
errors.push(err.into_file_diagnostic(trait_impl.methods.file_id));
} else {
let _func_ids =
interner.add_trait_implementaion(&key, trait_definition_ident, &trait_impl.methods);

Check warning on line 704 in crates/noirc_frontend/src/hir/def_collector/dc_crate.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (implementaion)
}

methods.append(&mut impl_methods);
}

methods
}
fn resolve_free_functions(
interner: &mut NodeInterner,
crate_id: CrateId,
Expand Down
Loading