Skip to content

Commit

Permalink
feat(traits): Add default and override of methods
Browse files Browse the repository at this point in the history
This commit removes some checks on UnresolvedTypes.

It adds the following validity checks and tests for them:

    Duplicate trait blocks should not compile
    Duplicate trait impl blocks should not compile
    Duplicate method definitions in impl blocks should not compile
    Impl blocks invalid types should not compile
  • Loading branch information
ymadzhunkov committed Sep 7, 2023
1 parent 747c604 commit 7b3ae7e
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);

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

0 comments on commit 7b3ae7e

Please sign in to comment.