Skip to content

Commit

Permalink
feat(traits): Add default and override of methods (#2585)
Browse files Browse the repository at this point in the history
Co-authored-by: Yordan Madzhunkov <[email protected]>
  • Loading branch information
yordanmadzhunkov and ymadzhunkov authored Sep 7, 2023
1 parent 6110638 commit 98c3ba9
Show file tree
Hide file tree
Showing 17 changed files with 257 additions and 131 deletions.
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 {
}

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::rc::Rc;
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 UnresolvedStruct {
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 struct DefCollector {
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 @@ pub struct DefCollector {
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 @@ impl DefCollector {
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 DefCollector {
// 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 @@ impl DefCollector {
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_impls(
}
}

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 @@ -601,6 +660,55 @@ fn resolve_impls(
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(),
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);
}

methods.append(&mut impl_methods);
}

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

0 comments on commit 98c3ba9

Please sign in to comment.