Skip to content

Commit

Permalink
fix(traits): dont use paths to check for crate
Browse files Browse the repository at this point in the history
  • Loading branch information
alexvitkov authored and ymadzhunkov committed Sep 28, 2023
1 parent 7826249 commit b8b9a81
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 40 deletions.
47 changes: 22 additions & 25 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::errors::{DefCollectorErrorKind, DuplicateType};
use crate::graph::CrateId;
use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId};
use crate::hir::resolution::errors::ResolverError;
use crate::hir::resolution::import::{get_path_crate, PathResolutionError};
use crate::hir::resolution::import::PathResolutionError;
use crate::hir::resolution::path_resolver::PathResolver;
use crate::hir::resolution::resolver::Resolver;
use crate::hir::resolution::{
Expand All @@ -12,7 +12,7 @@ use crate::hir::resolution::{
};
use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker};
use crate::hir::Context;
use crate::hir_def::traits::{TraitConstant, TraitFunction, TraitImpl, TraitType};
use crate::hir_def::traits::{Trait, TraitConstant, TraitFunction, TraitImpl, TraitType};
use crate::node_interner::{
FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId,
};
Expand Down Expand Up @@ -412,21 +412,20 @@ fn collect_impls(

fn collect_trait_impl_methods(
interner: &mut NodeInterner,
current_def_map: &mut CrateDefMap,
def_maps: &mut BTreeMap<CrateId, CrateDefMap>,
crate_id: CrateId,
trait_id: TraitId,
trait_impl: &mut UnresolvedTraitImpl,
) -> Vec<(CompilationError, FileId)> {
let crate_id = current_def_map.krate();

// In this Vec methods[i] corresponds to trait.methods[i]. If the impl has no implementation
// for a particular method, the default implementation will be added at that slot.
let mut ordered_methods = Vec::new();

let the_trait = interner.get_trait(trait_id);

// check whether the trait implementation is in the same crate as either the trait or the type
let mut errors = check_trait_impl_crate_coherence(trait_impl, crate_id, current_def_map);

let mut errors =
check_trait_impl_crate_coherence(interner, &the_trait, trait_impl, crate_id, def_maps);
// set of function ids that have a corresponding method in the trait
let mut func_ids_in_trait = HashSet::new();

Expand Down Expand Up @@ -523,13 +522,10 @@ fn collect_trait_impl(
}
};

match trait_impl.trait_id {
Some(trait_id) => {

if let Some(trait_id) = trait_impl.trait_id {
errors.extend(collect_trait_impl_methods(
interner,
def_maps.get_mut(&crate_id).unwrap(),
trait_id,
trait_impl,
interner, def_maps, crate_id, trait_id, trait_impl,
));
for (_, func_id, ast) in &trait_impl.methods.functions {
let file = def_maps[&crate_id].file_id(trait_impl.module_id);
Expand All @@ -548,7 +544,7 @@ fn collect_trait_impl(
*func_id,
ast.name_ident(),
) {
Ok(()) => {}
Ok(()) => {},
Err(err) => {
errors.push((err.into(), trait_impl.file_id));
}
Expand All @@ -562,9 +558,6 @@ fn collect_trait_impl(
}
}
}
None => {}
}

errors
}

Expand All @@ -580,21 +573,25 @@ fn collect_trait_impls(
}

fn check_trait_impl_crate_coherence(
interner: &mut NodeInterner,
the_trait: &Trait,
trait_impl: &UnresolvedTraitImpl,
current_crate: CrateId,
current_def_map: &CrateDefMap,
def_maps: &BTreeMap<CrateId, CrateDefMap>,
) -> Vec<(CompilationError, FileId)> {
let mut errors: Vec<(CompilationError, FileId)> = vec![];
let trait_crate = get_path_crate(current_crate, current_def_map, &trait_impl.trait_path);

let object_crate = match &trait_impl.object_type.typ {
crate::UnresolvedTypeData::Named(path, _) => {
get_path_crate(current_crate, current_def_map, path)
}
_ => None,
let module = ModuleId { krate: current_crate, local_id: trait_impl.module_id };
let file = def_maps[&current_crate].file_id(trait_impl.module_id);
let path_resolver = StandardPathResolver::new(module);
let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file);

let object_crate = match resolver.resolve_type(trait_impl.object_type.clone()) {
Type::Struct(struct_type, _) => struct_type.borrow().id.krate(),
_ => CrateId::Dummy,
};

if Some(current_crate) != trait_crate && Some(current_crate) != object_crate {
if current_crate != the_trait.crate_id && current_crate != object_crate {
let error = DefCollectorErrorKind::TraitImplOrphaned {
span: trait_impl.object_type.span.expect("object type must have a span"),
};
Expand Down
15 changes: 0 additions & 15 deletions compiler/noirc_frontend/src/hir/resolution/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,3 @@ fn resolve_external_dep(

resolve_path_to_ns(&dep_directive, dep_def_map, def_maps, allow_contracts)
}

pub fn get_path_crate(
current_crate: CrateId,
current_def_map: &CrateDefMap,
path: &Path,
) -> Option<CrateId> {
match path.kind {
PathKind::Dep => {
let crate_name = &path.segments.first().unwrap();
current_def_map.extern_prelude.get(&crate_name.0.contents).map(|module| module.krate)
}
PathKind::Crate => Some(current_crate),
PathKind::Plain => Some(current_crate),
}
}

0 comments on commit b8b9a81

Please sign in to comment.