diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 0c53bff4a54..90aa4baee7c 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -4,7 +4,7 @@ use crate::graph::CrateId; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; use crate::hir::resolution::errors::ResolverError; -use crate::hir::resolution::import::{resolve_import, ImportDirective}; +use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution}; use crate::hir::resolution::{ collect_impls, collect_trait_impls, path_resolver, resolve_free_functions, resolve_globals, resolve_impls, resolve_structs, resolve_trait_by_path, resolve_trait_impls, resolve_traits, @@ -56,8 +56,11 @@ impl UnresolvedFunctions { for bound in &mut func.def.where_clause { match resolve_trait_by_path(def_maps, module, bound.trait_bound.trait_path.clone()) { - Ok(trait_id) => { + Ok((trait_id, warning)) => { bound.trait_bound.trait_id = Some(trait_id); + if let Some(warning) = warning { + errors.push(DefCollectorErrorKind::PathResolutionError(warning)); + } } Err(err) => { errors.push(err); @@ -281,6 +284,13 @@ impl DefCollector { for collected_import in def_collector.collected_imports { match resolve_import(crate_id, &collected_import, &context.def_maps) { Ok(resolved_import) => { + if let Some(error) = resolved_import.error { + errors.push(( + DefCollectorErrorKind::PathResolutionError(error).into(), + root_file_id, + )); + } + // Populate module namespaces according to the imports used let current_def_map = context.def_maps.get_mut(&crate_id).unwrap(); @@ -299,9 +309,9 @@ impl DefCollector { } } } - Err((error, module_id)) => { + Err(error) => { let current_def_map = context.def_maps.get(&crate_id).unwrap(); - let file_id = current_def_map.file_id(module_id); + let file_id = current_def_map.file_id(collected_import.module_id); let error = DefCollectorErrorKind::PathResolutionError(error); errors.push((error.into(), file_id)); } @@ -409,12 +419,13 @@ fn inject_prelude( Path { segments: segments.clone(), kind: crate::PathKind::Dep, span: Span::default() }; if !crate_id.is_stdlib() { - if let Ok(module_def) = path_resolver::resolve_path( + if let Ok(PathResolution { module_def_id, error }) = path_resolver::resolve_path( &context.def_maps, ModuleId { krate: crate_id, local_id: crate_root }, path, ) { - let module_id = module_def.as_module().expect("std::prelude should be a module"); + assert!(error.is_none(), "Tried to add private item to prelude"); + let module_id = module_def_id.as_module().expect("std::prelude should be a module"); let prelude = context.module(module_id).scope().names(); for path in prelude { diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 3c6c0582292..d5b0c612f90 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -25,7 +25,7 @@ pub enum ResolverError { #[error("path is not an identifier")] PathIsNotIdent { span: Span }, #[error("could not resolve path")] - PathResolutionError(PathResolutionError), + PathResolutionError(#[from] PathResolutionError), #[error("Expected")] Expected { span: Span, expected: String, got: String }, #[error("Duplicate field in constructor")] @@ -72,10 +72,6 @@ pub enum ResolverError { NumericConstantInFormatString { name: String, span: Span }, #[error("Closure environment must be a tuple or unit type")] InvalidClosureEnvironment { typ: Type, span: Span }, - #[error("{name} is private and not visible from the current module")] - PrivateFunctionCalled { name: String, span: Span }, - #[error("{name} is not visible from the current crate")] - NonCrateFunctionCalled { name: String, span: Span }, #[error("Nested slices are not supported")] NestedSlices { span: Span }, #[error("#[recursive] attribute is only allowed on entry points to a program")] @@ -290,13 +286,6 @@ impl From for Diagnostic { ResolverError::InvalidClosureEnvironment { span, typ } => Diagnostic::simple_error( format!("{typ} is not a valid closure environment type"), "Closure environment must be a tuple or unit type".to_string(), span), - // This will be upgraded to an error in future versions - ResolverError::PrivateFunctionCalled { span, name } => Diagnostic::simple_warning( - format!("{name} is private and not visible from the current module"), - format!("{name} is private"), span), - ResolverError::NonCrateFunctionCalled { span, name } => Diagnostic::simple_warning( - format!("{name} is not visible from the current crate"), - format!("{name} is only visible within its crate"), span), ResolverError::NestedSlices { span } => Diagnostic::simple_error( "Nested slices are not supported".into(), "Try to use a constant sized array instead".into(), diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 9c8418daf80..ade97e2cf42 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -1,10 +1,11 @@ use noirc_errors::{CustomDiagnostic, Span}; +use thiserror::Error; use crate::graph::CrateId; use std::collections::BTreeMap; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId, PerNs}; -use crate::{Ident, Path, PathKind}; +use crate::{Ident, ItemVisibility, Path, PathKind}; #[derive(Debug, Clone)] pub struct ImportDirective { @@ -14,12 +15,30 @@ pub struct ImportDirective { pub is_prelude: bool, } -pub type PathResolution = Result; +struct NamespaceResolution { + module_id: ModuleId, + namespace: PerNs, + error: Option, +} + +type NamespaceResolutionResult = Result; + +pub struct PathResolution { + pub module_def_id: ModuleDefId, + + pub error: Option, +} + +pub(crate) type PathResolutionResult = Result; -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Error)] pub enum PathResolutionError { + #[error("Could not resolve '{0}' in path")] Unresolved(Ident), + #[error("Contract variable '{0}' referenced from outside the contract")] ExternalContractUsed(Ident), + #[error("{0} is private and not visible from the current module")] + Private(Ident), } #[derive(Debug)] @@ -31,21 +50,26 @@ pub struct ResolvedImport { // The module which we must add the resolved namespace to pub module_scope: LocalModuleId, pub is_prelude: bool, + pub error: Option, } impl From for CustomDiagnostic { fn from(error: PathResolutionError) -> Self { - match error { - PathResolutionError::Unresolved(ident) => CustomDiagnostic::simple_error( - format!("Could not resolve '{ident}' in path"), - String::new(), - ident.span(), - ), + match &error { + PathResolutionError::Unresolved(ident) => { + CustomDiagnostic::simple_error(error.to_string(), String::new(), ident.span()) + } PathResolutionError::ExternalContractUsed(ident) => CustomDiagnostic::simple_error( - format!("Contract variable '{ident}' referenced from outside the contract"), + error.to_string(), "Contracts may only be referenced from within a contract".to_string(), ident.span(), ), + // This will be upgraded to an error in future versions + PathResolutionError::Private(ident) => CustomDiagnostic::simple_warning( + error.to_string(), + format!("{ident} is private"), + ident.span(), + ), } } } @@ -54,27 +78,49 @@ pub fn resolve_import( crate_id: CrateId, import_directive: &ImportDirective, def_maps: &BTreeMap, -) -> Result { - let def_map = &def_maps[&crate_id]; - +) -> Result { let allow_contracts = allow_referencing_contracts(def_maps, crate_id, import_directive.module_id); let module_scope = import_directive.module_id; - let resolved_namespace = - resolve_path_to_ns(import_directive, def_map, def_maps, allow_contracts) - .map_err(|error| (error, module_scope))?; + let NamespaceResolution { + module_id: resolved_module, + namespace: resolved_namespace, + mut error, + } = resolve_path_to_ns(import_directive, crate_id, crate_id, def_maps, allow_contracts)?; let name = resolve_path_name(import_directive); + + let visibility = resolved_namespace + .values + .or(resolved_namespace.types) + .map(|(_, visibility, _)| visibility) + .expect("Found empty namespace"); + + error = error.or_else(|| { + if can_reference_module_id( + def_maps, + crate_id, + import_directive.module_id, + resolved_module, + visibility, + ) { + None + } else { + Some(PathResolutionError::Private(name.clone())) + } + }); + Ok(ResolvedImport { name, resolved_namespace, module_scope, is_prelude: import_directive.is_prelude, + error, }) } -pub(super) fn allow_referencing_contracts( +fn allow_referencing_contracts( def_maps: &BTreeMap, krate: CrateId, local_id: LocalModuleId, @@ -82,27 +128,40 @@ pub(super) fn allow_referencing_contracts( ModuleId { krate, local_id }.module(def_maps).is_contract } -pub fn resolve_path_to_ns( +fn resolve_path_to_ns( import_directive: &ImportDirective, - def_map: &CrateDefMap, + crate_id: CrateId, + importing_crate: CrateId, def_maps: &BTreeMap, allow_contracts: bool, -) -> PathResolution { +) -> NamespaceResolutionResult { let import_path = &import_directive.path.segments; + let def_map = &def_maps[&crate_id]; match import_directive.path.kind { crate::ast::PathKind::Crate => { // Resolve from the root of the crate - resolve_path_from_crate_root(def_map, import_path, def_maps, allow_contracts) - } - crate::ast::PathKind::Dep => { - resolve_external_dep(def_map, import_directive, def_maps, allow_contracts) + resolve_path_from_crate_root( + crate_id, + importing_crate, + import_path, + def_maps, + allow_contracts, + ) } + crate::ast::PathKind::Dep => resolve_external_dep( + def_map, + import_directive, + def_maps, + allow_contracts, + importing_crate, + ), crate::ast::PathKind::Plain => { // Plain paths are only used to import children modules. It's possible to allow import of external deps, but maybe this distinction is better? // In Rust they can also point to external Dependencies, if no children can be found with the specified name resolve_name_in_module( - def_map, + crate_id, + importing_crate, import_path, import_directive.module_id, def_maps, @@ -113,45 +172,60 @@ pub fn resolve_path_to_ns( } fn resolve_path_from_crate_root( - def_map: &CrateDefMap, + crate_id: CrateId, + importing_crate: CrateId, + import_path: &[Ident], def_maps: &BTreeMap, allow_contracts: bool, -) -> PathResolution { - resolve_name_in_module(def_map, import_path, def_map.root, def_maps, allow_contracts) +) -> NamespaceResolutionResult { + resolve_name_in_module( + crate_id, + importing_crate, + import_path, + def_maps[&crate_id].root, + def_maps, + allow_contracts, + ) } fn resolve_name_in_module( - def_map: &CrateDefMap, + krate: CrateId, + importing_crate: CrateId, import_path: &[Ident], starting_mod: LocalModuleId, def_maps: &BTreeMap, allow_contracts: bool, -) -> PathResolution { - let mut current_mod = &def_map.modules[starting_mod.0]; +) -> NamespaceResolutionResult { + let def_map = &def_maps[&krate]; + let mut current_mod_id = ModuleId { krate, local_id: starting_mod }; + let mut current_mod = &def_map.modules[current_mod_id.local_id.0]; // There is a possibility that the import path is empty // In that case, early return if import_path.is_empty() { - let mod_id = ModuleId { krate: def_map.krate, local_id: starting_mod }; - return Ok(PerNs::types(mod_id.into())); + return Ok(NamespaceResolution { + module_id: current_mod_id, + namespace: PerNs::types(current_mod_id.into()), + error: None, + }); } - let mut import_path = import_path.iter(); - let first_segment = import_path.next().expect("ice: could not fetch first segment"); + let first_segment = import_path.first().expect("ice: could not fetch first segment"); let mut current_ns = current_mod.find_name(first_segment); if current_ns.is_none() { return Err(PathResolutionError::Unresolved(first_segment.clone())); } - for segment in import_path { - let typ = match current_ns.take_types() { - None => return Err(PathResolutionError::Unresolved(segment.clone())), - Some(typ) => typ, + let mut warning: Option = None; + for (last_segment, current_segment) in import_path.iter().zip(import_path.iter().skip(1)) { + let (typ, visibility) = match current_ns.types { + None => return Err(PathResolutionError::Unresolved(last_segment.clone())), + Some((typ, visibility, _)) => (typ, visibility), }; // In the type namespace, only Mod can be used in a path. - let new_module_id = match typ { + current_mod_id = match typ { ModuleDefId::ModuleId(id) => id, ModuleDefId::FunctionId(_) => panic!("functions cannot be in the type namespace"), // TODO: If impls are ever implemented, types can be used in a path @@ -161,22 +235,37 @@ fn resolve_name_in_module( ModuleDefId::GlobalId(_) => panic!("globals cannot be in the type namespace"), }; - current_mod = &def_maps[&new_module_id.krate].modules[new_module_id.local_id.0]; + warning = warning.or_else(|| { + if can_reference_module_id( + def_maps, + importing_crate, + starting_mod, + current_mod_id, + visibility, + ) { + None + } else { + Some(PathResolutionError::Private(last_segment.clone())) + } + }); + + current_mod = &def_maps[¤t_mod_id.krate].modules[current_mod_id.local_id.0]; // Check if namespace - let found_ns = current_mod.find_name(segment); + let found_ns = current_mod.find_name(current_segment); if found_ns.is_none() { - return Err(PathResolutionError::Unresolved(segment.clone())); + return Err(PathResolutionError::Unresolved(current_segment.clone())); } + // Check if it is a contract and we're calling from a non-contract context if current_mod.is_contract && !allow_contracts { - return Err(PathResolutionError::ExternalContractUsed(segment.clone())); + return Err(PathResolutionError::ExternalContractUsed(current_segment.clone())); } current_ns = found_ns; } - Ok(current_ns) + Ok(NamespaceResolution { module_id: current_mod_id, namespace: current_ns, error: warning }) } fn resolve_path_name(import_directive: &ImportDirective) -> Ident { @@ -191,11 +280,11 @@ fn resolve_external_dep( directive: &ImportDirective, def_maps: &BTreeMap, allow_contracts: bool, -) -> PathResolution { + importing_crate: CrateId, +) -> NamespaceResolutionResult { // Use extern_prelude to get the dep - // let path = &directive.path.segments; - // + // Fetch the root module from the prelude let crate_name = path.first().unwrap(); let dep_module = current_def_map @@ -218,7 +307,49 @@ fn resolve_external_dep( is_prelude: false, }; - let dep_def_map = def_maps.get(&dep_module.krate).unwrap(); + resolve_path_to_ns(&dep_directive, dep_module.krate, importing_crate, def_maps, allow_contracts) +} + +// Issue an error if the given private function is being called from a non-child module, or +// if the given pub(crate) function is being called from another crate +fn can_reference_module_id( + def_maps: &BTreeMap, + importing_crate: CrateId, + current_module: LocalModuleId, + target_module: ModuleId, + visibility: ItemVisibility, +) -> bool { + // Note that if the target module is in a different crate from the current module then we will either + // return true as the target module is public or return false as it is private without looking at the `CrateDefMap` in either case. + let same_crate = target_module.krate == importing_crate; + let target_crate_def_map = &def_maps[&target_module.krate]; + + match visibility { + ItemVisibility::Public => true, + ItemVisibility::PublicCrate => same_crate, + ItemVisibility::Private => { + same_crate + && module_descendent_of_target( + target_crate_def_map, + target_module.local_id, + current_module, + ) + } + } +} + +// Returns true if `current` is a (potentially nested) child module of `target`. +// This is also true if `current == target`. +fn module_descendent_of_target( + def_map: &CrateDefMap, + target: LocalModuleId, + current: LocalModuleId, +) -> bool { + if current == target { + return true; + } - resolve_path_to_ns(&dep_directive, dep_def_map, def_maps, allow_contracts) + def_map.modules[current.0] + .parent + .map_or(false, |parent| module_descendent_of_target(def_map, target, parent)) } diff --git a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs index 4c5fa3bceef..e19af3c732f 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -1,11 +1,9 @@ -use super::import::{ - allow_referencing_contracts, resolve_path_to_ns, ImportDirective, PathResolutionError, -}; +use super::import::{resolve_import, ImportDirective, PathResolution, PathResolutionResult}; use crate::Path; use std::collections::BTreeMap; use crate::graph::CrateId; -use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId}; +use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; pub trait PathResolver { /// Resolve the given path returning the resolved ModuleDefId. @@ -13,7 +11,7 @@ pub trait PathResolver { &self, def_maps: &BTreeMap, path: Path, - ) -> Result; + ) -> PathResolutionResult; fn local_module_id(&self) -> LocalModuleId; @@ -36,7 +34,7 @@ impl PathResolver for StandardPathResolver { &self, def_maps: &BTreeMap, path: Path, - ) -> Result { + ) -> PathResolutionResult { resolve_path(def_maps, self.module_id, path) } @@ -55,17 +53,15 @@ pub fn resolve_path( def_maps: &BTreeMap, module_id: ModuleId, path: Path, -) -> Result { +) -> PathResolutionResult { // lets package up the path into an ImportDirective and resolve it using that let import = ImportDirective { module_id: module_id.local_id, path, alias: None, is_prelude: false }; - let allow_referencing_contracts = - allow_referencing_contracts(def_maps, module_id.krate, module_id.local_id); + let resolved_import = resolve_import(module_id.krate, &import, def_maps)?; - let def_map = &def_maps[&module_id.krate]; - let ns = resolve_path_to_ns(&import, def_map, def_maps, allow_referencing_contracts)?; + let namespace = resolved_import.resolved_namespace; + let id = + namespace.values.or(namespace.types).map(|(id, _, _)| id).expect("Found empty namespace"); - let function = ns.values.map(|(id, _, _)| id); - let id = function.or_else(|| ns.types.map(|(id, _, _)| id)); - Ok(id.expect("Found empty namespace")) + Ok(PathResolution { module_def_id: id, error: resolved_import.error }) } diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 3fbde8a890b..8c468b6c2ba 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -25,7 +25,7 @@ use std::collections::{BTreeMap, HashSet}; use std::rc::Rc; use crate::graph::CrateId; -use crate::hir::def_map::{LocalModuleId, ModuleDefId, TryFromModuleDefId, MAIN_FUNCTION}; +use crate::hir::def_map::{ModuleDefId, TryFromModuleDefId, MAIN_FUNCTION}; use crate::hir_def::stmt::{HirAssignStatement, HirForStatement, HirLValue, HirPattern}; use crate::node_interner::{ DefinitionId, DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, NodeInterner, StmtId, @@ -56,6 +56,7 @@ use crate::hir_def::{ }; use super::errors::{PubPosition, ResolverError}; +use super::import::PathResolution; const SELF_TYPE_NAME: &str = "Self"; @@ -677,10 +678,14 @@ impl<'a> Resolver<'a> { // If we cannot find a local generic of the same name, try to look up a global match self.path_resolver.resolve(self.def_maps, path.clone()) { - Ok(ModuleDefId::GlobalId(id)) => { + Ok(PathResolution { module_def_id: ModuleDefId::GlobalId(id), error }) => { if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, id); } + + if let Some(error) = error { + self.push_err(error.into()); + } Some(Type::Constant(self.eval_global_as_array_length(id, path))) } _ => None, @@ -1319,59 +1324,6 @@ impl<'a> Resolver<'a> { } } - // Issue an error if the given private function is being called from a non-child module, or - // if the given pub(crate) function is being called from another crate - fn check_can_reference_function( - &mut self, - func: FuncId, - span: Span, - visibility: ItemVisibility, - ) { - let function_module = self.interner.function_module(func); - let current_module = self.path_resolver.module_id(); - - let same_crate = function_module.krate == current_module.krate; - let krate = function_module.krate; - let current_module = current_module.local_id; - let name = self.interner.function_name(&func).to_string(); - match visibility { - ItemVisibility::Public => (), - ItemVisibility::Private => { - if !same_crate - || !self.module_descendent_of_target( - krate, - function_module.local_id, - current_module, - ) - { - self.errors.push(ResolverError::PrivateFunctionCalled { span, name }); - } - } - ItemVisibility::PublicCrate => { - if !same_crate { - self.errors.push(ResolverError::NonCrateFunctionCalled { span, name }); - } - } - } - } - - // Returns true if `current` is a (potentially nested) child module of `target`. - // This is also true if `current == target`. - fn module_descendent_of_target( - &self, - krate: CrateId, - target: LocalModuleId, - current: LocalModuleId, - ) -> bool { - if current == target { - return true; - } - - self.def_maps[&krate].modules[current.0] - .parent - .map_or(false, |parent| self.module_descendent_of_target(krate, target, parent)) - } - fn resolve_local_variable(&mut self, hir_ident: HirIdent, var_scope_index: usize) { let mut transitive_capture_index: Option = None; @@ -1465,15 +1417,6 @@ impl<'a> Resolver<'a> { if let Some(current_item) = self.current_item { self.interner.add_function_dependency(current_item, id); } - - if self.interner.function_visibility(id) != ItemVisibility::Public { - let span = hir_ident.location.span; - self.check_can_reference_function( - id, - span, - self.interner.function_visibility(id), - ); - } } DefinitionKind::Global(global_id) => { if let Some(current_item) = self.current_item { @@ -1912,7 +1855,7 @@ impl<'a> Resolver<'a> { } if let Ok(ModuleDefId::TraitId(trait_id)) = - self.path_resolver.resolve(self.def_maps, trait_bound.trait_path.clone()) + self.resolve_path(trait_bound.trait_path.clone()) { let the_trait = self.interner.get_trait(trait_id); if let Some(method) = @@ -1947,7 +1890,13 @@ impl<'a> Resolver<'a> { } fn resolve_path(&mut self, path: Path) -> Result { - self.path_resolver.resolve(self.def_maps, path).map_err(ResolverError::PathResolutionError) + let path_resolution = self.path_resolver.resolve(self.def_maps, path)?; + + if let Some(error) = path_resolution.error { + self.push_err(error.into()); + } + + Ok(path_resolution.module_def_id) } fn resolve_block(&mut self, block_expr: BlockExpression) -> HirExpression { diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 04da558a642..a7669f57e33 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -21,6 +21,7 @@ use crate::{ use super::{ functions, get_module_mut, get_struct_type, + import::{PathResolution, PathResolutionError}, path_resolver::{PathResolver, StandardPathResolver}, resolver::Resolver, take_errors, @@ -274,7 +275,15 @@ fn collect_trait_impl( let module = ModuleId { local_id: trait_impl.module_id, krate: crate_id }; trait_impl.trait_id = match resolve_trait_by_path(def_maps, module, trait_impl.trait_path.clone()) { - Ok(trait_id) => Some(trait_id), + Ok((trait_id, warning)) => { + if let Some(warning) = warning { + errors.push(( + DefCollectorErrorKind::PathResolutionError(warning).into(), + trait_impl.file_id, + )); + } + Some(trait_id) + } Err(error) => { errors.push((error.into(), trait_impl.file_id)); None @@ -362,11 +371,13 @@ pub(crate) fn resolve_trait_by_path( def_maps: &BTreeMap, module: ModuleId, path: Path, -) -> Result { +) -> Result<(TraitId, Option), DefCollectorErrorKind> { let path_resolver = StandardPathResolver::new(module); match path_resolver.resolve(def_maps, path.clone()) { - Ok(ModuleDefId::TraitId(trait_id)) => Ok(trait_id), + Ok(PathResolution { module_def_id: ModuleDefId::TraitId(trait_id), error }) => { + Ok((trait_id, error)) + } Ok(_) => Err(DefCollectorErrorKind::NotATrait { not_a_trait_name: path }), Err(_) => Err(DefCollectorErrorKind::TraitNotFound { trait_path: path }), } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index ab759f454e5..84c5b56ceac 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -398,7 +398,9 @@ mod test { use crate::graph::CrateId; use crate::hir::def_map::{ModuleData, ModuleId}; - use crate::hir::resolution::import::PathResolutionError; + use crate::hir::resolution::import::{ + PathResolution, PathResolutionError, PathResolutionResult, + }; use crate::hir_def::expr::HirIdent; use crate::hir_def::stmt::HirLetStatement; use crate::hir_def::stmt::HirPattern::Identifier; @@ -598,12 +600,13 @@ mod test { &self, _def_maps: &BTreeMap, path: Path, - ) -> Result { + ) -> PathResolutionResult { // Not here that foo::bar and hello::foo::bar would fetch the same thing let name = path.segments.last().unwrap(); self.0 .get(&name.0.contents) .cloned() + .map(|module_def_id| PathResolution { module_def_id, error: None }) .ok_or_else(move || PathResolutionError::Unresolved(name.clone())) }