From de95935699c42daf78008928b43e91bc44a08fb3 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 13 Feb 2024 10:24:10 -0500 Subject: [PATCH 1/3] wip starting to reduce usage of dummy id --- compiler/noirc_frontend/src/graph/mod.rs | 6 +++--- compiler/noirc_frontend/src/hir/def_map/mod.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/graph/mod.rs b/compiler/noirc_frontend/src/graph/mod.rs index 452aef74b36..52d04289946 100644 --- a/compiler/noirc_frontend/src/graph/mod.rs +++ b/compiler/noirc_frontend/src/graph/mod.rs @@ -20,9 +20,9 @@ pub enum CrateId { } impl CrateId { - pub fn dummy_id() -> CrateId { - CrateId::Dummy - } + // pub fn dummy_id() -> CrateId { + // CrateId::Dummy + // } pub fn is_stdlib(&self) -> bool { matches!(self, CrateId::Stdlib(_)) diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 8c985e88e0b..9b62bfbd899 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -37,7 +37,7 @@ impl LocalModuleId { #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct ModuleId { - pub krate: CrateId, + pub krate: Option, pub local_id: LocalModuleId, } From 653e6138fbc8b7fe19347b31d3a8dd3af064785a Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 13 Feb 2024 14:30:27 -0500 Subject: [PATCH 2/3] wip propagating options --- compiler/noirc_frontend/src/graph/mod.rs | 1 - .../src/hir/def_collector/dc_crate.rs | 20 ++-- .../src/hir/def_collector/dc_mod.rs | 18 ++-- .../noirc_frontend/src/hir/def_map/mod.rs | 10 +- compiler/noirc_frontend/src/hir/mod.rs | 38 ++++---- .../src/hir/resolution/errors.rs | 2 + .../src/hir/resolution/functions.rs | 4 +- .../src/hir/resolution/globals.rs | 2 +- .../src/hir/resolution/impls.rs | 95 ++++++++++--------- .../noirc_frontend/src/hir/resolution/mod.rs | 8 +- .../src/hir/resolution/structs.rs | 4 +- .../src/hir/resolution/traits.rs | 80 +++++++++------- .../src/hir/resolution/type_aliases.rs | 2 +- compiler/noirc_frontend/src/hir_def/traits.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 8 +- 15 files changed, 160 insertions(+), 134 deletions(-) diff --git a/compiler/noirc_frontend/src/graph/mod.rs b/compiler/noirc_frontend/src/graph/mod.rs index 52d04289946..c624a16e49e 100644 --- a/compiler/noirc_frontend/src/graph/mod.rs +++ b/compiler/noirc_frontend/src/graph/mod.rs @@ -16,7 +16,6 @@ pub enum CrateId { Root(usize), Crate(usize), Stdlib(usize), - Dummy, } impl CrateId { 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 0d1dd1b4337..f12c80afb17 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -46,7 +46,7 @@ impl UnresolvedFunctions { pub fn resolve_trait_bounds_trait_ids( &mut self, def_maps: &BTreeMap, - crate_id: CrateId, + crate_id: Option, ) -> Vec { let mut errors = Vec::new(); @@ -80,7 +80,7 @@ pub struct UnresolvedStruct { pub struct UnresolvedTrait { pub file_id: FileId, pub module_id: LocalModuleId, - pub crate_id: CrateId, + pub crate_id: Option, pub trait_def: NoirTrait, pub method_ids: HashMap, pub fns_with_default_impl: UnresolvedFunctions, @@ -228,7 +228,7 @@ impl DefCollector { let dep_def_root = context.def_map(&dep.crate_id).expect("ice: def map was just created").root; - let module_id = ModuleId { krate: dep.crate_id, local_id: dep_def_root }; + let module_id = ModuleId { krate: Some(dep.crate_id), local_id: dep_def_root }; // Add this crate as a dependency by linking it's root module def_map.extern_prelude.insert(dep.as_name(), module_id); } @@ -248,7 +248,7 @@ impl DefCollector { ast, root_file_id, crate_root, - crate_id, + Some(crate_id), context, )); @@ -256,10 +256,10 @@ impl DefCollector { // Add the current crate to the collection of DefMaps context.def_maps.insert(crate_id, def_collector.def_map); - inject_prelude(crate_id, context, crate_root, &mut def_collector.collected_imports); + inject_prelude(Some(crate_id), context, crate_root, &mut def_collector.collected_imports); for submodule in submodules { inject_prelude( - crate_id, + Some(crate_id), context, LocalModuleId(submodule), &mut def_collector.collected_imports, @@ -310,7 +310,7 @@ impl DefCollector { errors.extend(resolve_type_aliases( context, def_collector.collected_type_aliases, - crate_id, + Some(crate_id), )); errors.extend(resolve_traits(context, def_collector.collected_traits, crate_id)); @@ -361,7 +361,7 @@ impl DefCollector { functions.extend(resolve_trait_impls( context, def_collector.collected_traits_impls, - crate_id, + Some(crate_id), &mut errors, )); @@ -382,7 +382,7 @@ impl DefCollector { } fn inject_prelude( - crate_id: CrateId, + crate_id: Option, context: &Context, crate_root: LocalModuleId, collected_imports: &mut Vec, @@ -395,7 +395,7 @@ fn inject_prelude( let path = Path { segments: segments.clone(), kind: crate::PathKind::Dep, span: Span::default() }; - if !crate_id.is_stdlib() { + if !crate_id.map(|id| id.is_stdlib()).unwrap_or(false) { if let Ok(module_def) = path_resolver::resolve_path( &context.def_maps, ModuleId { krate: crate_id, local_id: crate_root }, diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 77224cc311c..b4091cf2650 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -39,7 +39,7 @@ pub fn collect_defs( ast: SortedModule, file_id: FileId, module_id: LocalModuleId, - crate_id: CrateId, + crate_id: Option, context: &mut Context, ) -> Vec<(CompilationError, FileId)> { let mut collector = ModCollector { def_collector, file_id, module_id }; @@ -115,7 +115,7 @@ impl<'a> ModCollector<'a> { errors } - fn collect_impls(&mut self, context: &mut Context, impls: Vec, krate: CrateId) { + fn collect_impls(&mut self, context: &mut Context, impls: Vec, krate: Option) { let module_id = ModuleId { krate, local_id: self.module_id }; for r#impl in impls { @@ -142,7 +142,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, impls: Vec, - krate: CrateId, + krate: Option, ) { for trait_impl in impls { let trait_name = trait_impl.trait_name.clone(); @@ -178,7 +178,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, trait_impl: &NoirTraitImpl, - krate: CrateId, + krate: Option, ) -> UnresolvedFunctions { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new(), trait_id: None }; @@ -201,7 +201,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, functions: Vec, - krate: CrateId, + krate: Option, ) -> Vec<(CompilationError, FileId)> { let mut unresolved_functions = UnresolvedFunctions { file_id: self.file_id, functions: Vec::new(), trait_id: None }; @@ -257,7 +257,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, types: Vec, - krate: CrateId, + krate: Option, ) -> Vec<(CompilationError, FileId)> { let mut definition_errors = vec![]; for struct_definition in types { @@ -343,7 +343,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, traits: Vec, - krate: CrateId, + krate: Option, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; for trait_definition in traits { @@ -491,7 +491,7 @@ impl<'a> ModCollector<'a> { fn collect_submodules( &mut self, context: &mut Context, - crate_id: CrateId, + crate_id: Option, submodules: Vec, file_id: FileId, ) -> Vec<(CompilationError, FileId)> { @@ -523,7 +523,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, mod_name: &Ident, - crate_id: CrateId, + crate_id: Option, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; let child_file_id = diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 9b62bfbd899..491a037ac6b 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -43,13 +43,17 @@ pub struct ModuleId { impl ModuleId { pub fn dummy_id() -> ModuleId { - ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() } + ModuleId { krate: None, local_id: LocalModuleId::dummy_id() } } } impl ModuleId { - pub fn module(self, def_maps: &BTreeMap) -> &ModuleData { - &def_maps[&self.krate].modules()[self.local_id.0] + pub fn module(self, def_maps: &BTreeMap) -> Option<&ModuleData> { + if let Some(krate) = self.krate { + &def_maps[&krate].modules()[self.local_id.0] + } else { + None + } } } diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index 4d3800f1a50..1e23d990758 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -87,8 +87,8 @@ impl Context<'_, '_> { /// It is perfectly valid for the compiler to look /// up a CrateDefMap and it is not available. /// This is how the compiler knows to compile a Crate. - pub fn def_map(&self, crate_id: &CrateId) -> Option<&CrateDefMap> { - self.def_maps.get(crate_id) + pub fn def_map(&self, crate_id: Option<&CrateId>) -> Option<&CrateDefMap> { + crate_id.and_then(|id| self.def_maps.get(id)) } /// Return the CrateId for each crate that has been compiled @@ -110,13 +110,13 @@ impl Context<'_, '_> { self.def_interner.function_name(id) } - pub fn fully_qualified_function_name(&self, crate_id: &CrateId, id: &FuncId) -> String { + pub fn fully_qualified_function_name(&self, crate_id: Option<&CrateId>, id: &FuncId) -> String { let def_map = self.def_map(crate_id).expect("The local crate should be analyzed already"); let name = self.def_interner.function_name(id); let module_id = self.def_interner.function_module(*id); - let module = self.module(module_id); + let module = self.module(module_id).expect("The local module should be analyzed already"); let parent = def_map.get_module_path_with_separator(module_id.local_id.0, module.parent, "::"); @@ -133,21 +133,21 @@ impl Context<'_, '_> { /// /// For example, if you project contains a `main.nr` and `foo.nr` and you provide the `main_crate_id` and the /// `bar_struct_id` where the `Bar` struct is inside `foo.nr`, this function would return `foo::Bar` as a [String]. - pub fn fully_qualified_struct_path(&self, crate_id: &CrateId, id: StructId) -> String { + pub fn fully_qualified_struct_path(&self, crate_id: Option<&CrateId>, id: StructId) -> String { let module_id = id.module_id(); let child_id = module_id.local_id.0; let def_map = - self.def_map(&module_id.krate).expect("The local crate should be analyzed already"); + self.def_map((&module_id.krate).as_ref()).expect("The local crate should be analyzed already"); - let module = self.module(module_id); + let module = self.module(module_id).expect("The local module should be analyzed already"); let module_path = def_map.get_module_path_with_separator(child_id, module.parent, "::"); - if &module_id.krate == crate_id { + if (&module_id.krate).as_ref() == crate_id { module_path } else { let crates = self - .find_dependencies(crate_id, &module_id.krate) + .find_dependencies(crate_id, (&module_id.krate).as_ref()) .expect("The Struct was supposed to be defined in a dependency"); crates.join("::") + "::" + &module_path } @@ -160,14 +160,14 @@ impl Context<'_, '_> { /// Returns the path from crate_id to target_crate_id fn find_dependencies( &self, - crate_id: &CrateId, - target_crate_id: &CrateId, + crate_id: Option<&CrateId>, + target_crate_id: Option<&CrateId>, ) -> Option> { - for dep in &self.crate_graph[crate_id].dependencies { - if &dep.crate_id == target_crate_id { + for dep in crate_id.map(|id| &self.crate_graph[id].dependencies).unwrap_or(&vec![]) { + if Some(&dep.crate_id) == target_crate_id { return Some(vec![dep.name.to_string()]); } - if let Some(mut path) = self.find_dependencies(&dep.crate_id, target_crate_id) { + if let Some(mut path) = self.find_dependencies(Some(&dep.crate_id), target_crate_id) { path.insert(0, dep.name.to_string()); return Some(path); } @@ -182,7 +182,7 @@ impl Context<'_, '_> { /// Returns the FuncId of the 'main' function in a crate. /// - Expects check_crate to be called beforehand /// - Panics if no main function is found - pub fn get_main_function(&self, crate_id: &CrateId) -> Option { + pub fn get_main_function(&self, crate_id: Option<&CrateId>) -> Option { // Find the local crate, one should always be present let local_crate = self.def_map(crate_id).unwrap(); @@ -194,7 +194,7 @@ impl Context<'_, '_> { /// will return all functions marked with #[test]. pub fn get_all_test_functions_in_crate_matching( &self, - crate_id: &CrateId, + crate_id: Option<&CrateId>, pattern: FunctionNameMatch, ) -> Vec<(String, TestFunction)> { let interner = &self.def_interner; @@ -217,7 +217,7 @@ impl Context<'_, '_> { .collect() } - pub fn get_all_exported_functions_in_crate(&self, crate_id: &CrateId) -> Vec<(String, FuncId)> { + pub fn get_all_exported_functions_in_crate(&self, crate_id: Option<&CrateId>) -> Vec<(String, FuncId)> { let interner = &self.def_interner; let def_map = self.def_map(crate_id).expect("The local crate should be analyzed already"); @@ -231,13 +231,13 @@ impl Context<'_, '_> { } /// Return a Vec of all `contract` declarations in the source code and the functions they contain - pub fn get_all_contracts(&self, crate_id: &CrateId) -> Vec { + pub fn get_all_contracts(&self, crate_id: Option<&CrateId>) -> Vec { self.def_map(crate_id) .expect("The local crate should be analyzed already") .get_all_contracts(&self.def_interner) } - fn module(&self, module_id: def_map::ModuleId) -> &def_map::ModuleData { + fn module(&self, module_id: def_map::ModuleId) -> Option<&def_map::ModuleData> { module_id.module(&self.def_maps) } } diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index d2fe67da38c..9a7a0ef53ca 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -90,6 +90,8 @@ pub enum ResolverError { LowLevelFunctionOutsideOfStdlib { ident: Ident }, #[error("Dependency cycle found, '{item}' recursively depends on itself: {cycle} ")] DependencyCycle { span: Span, item: String, cycle: String }, + #[error("Internal error: missing crate ID")] + MissingCrateId(ModuleId), } impl ResolverError { diff --git a/compiler/noirc_frontend/src/hir/resolution/functions.rs b/compiler/noirc_frontend/src/hir/resolution/functions.rs index e63de9b9173..656d010a3f7 100644 --- a/compiler/noirc_frontend/src/hir/resolution/functions.rs +++ b/compiler/noirc_frontend/src/hir/resolution/functions.rs @@ -19,7 +19,7 @@ use super::{path_resolver::StandardPathResolver, resolver::Resolver}; #[allow(clippy::too_many_arguments)] pub(crate) fn resolve_function_set( interner: &mut NodeInterner, - crate_id: CrateId, + crate_id: Option, def_maps: &BTreeMap, mut unresolved_functions: UnresolvedFunctions, self_type: Option, @@ -61,7 +61,7 @@ pub(crate) fn resolve_function_set( pub(crate) fn resolve_free_functions( interner: &mut NodeInterner, - crate_id: CrateId, + crate_id: Option, def_maps: &BTreeMap, collected_functions: Vec, self_type: Option, diff --git a/compiler/noirc_frontend/src/hir/resolution/globals.rs b/compiler/noirc_frontend/src/hir/resolution/globals.rs index 9fb31271727..d53833a3d52 100644 --- a/compiler/noirc_frontend/src/hir/resolution/globals.rs +++ b/compiler/noirc_frontend/src/hir/resolution/globals.rs @@ -26,7 +26,7 @@ impl ResolvedGlobals { pub(crate) fn resolve_globals( context: &mut Context, globals: Vec, - crate_id: CrateId, + crate_id: Option, ) -> ResolvedGlobals { let mut errors: Vec<(CompilationError, FileId)> = vec![]; let globals = vecmap(globals, |global| { diff --git a/compiler/noirc_frontend/src/hir/resolution/impls.rs b/compiler/noirc_frontend/src/hir/resolution/impls.rs index 4aa70f00cfc..36840fecbf8 100644 --- a/compiler/noirc_frontend/src/hir/resolution/impls.rs +++ b/compiler/noirc_frontend/src/hir/resolution/impls.rs @@ -25,13 +25,19 @@ use super::{ /// of the module defined by its type. pub(crate) fn collect_impls( context: &mut Context, - crate_id: CrateId, + opt_crate_id: Option, collected_impls: &ImplMap, ) -> Vec<(CompilationError, FileId)> { let interner = &mut context.def_interner; let def_maps = &mut context.def_maps; let mut errors: Vec<(CompilationError, FileId)> = vec![]; + if opt_crate_id.is_none() { + return errors + } else { + let crate_id = opt_crate_id.unwrap(); + } + for ((unresolved_type, module_id), methods) in collected_impls { let path_resolver = StandardPathResolver::new(ModuleId { local_id: *module_id, krate: crate_id }); @@ -60,17 +66,18 @@ pub(crate) fn collect_impls( // Grab the module defined by the struct type. Note that impls are a case // where the module the methods are added to is not the same as the module // they are resolved in. - let module = get_module_mut(def_maps, struct_type.id.module_id()); - - for (_, method_id, method) in &unresolved.functions { - // If this method was already declared, remove it from the module so it cannot - // be accessed with the `TypeName::method` syntax. We'll check later whether the - // object types in each method overlap or not. If they do, we issue an error. - // If not, that is specialization which is allowed. - if module.declare_function(method.name_ident().clone(), *method_id).is_err() { - module.remove_function(method.name_ident()); + if let Some(module) = get_module_mut(def_maps, struct_type.id.module_id()) { + for (_, method_id, method) in &unresolved.functions { + // If this method was already declared, remove it from the module so it cannot + // be accessed with the `TypeName::method` syntax. We'll check later whether the + // object types in each method overlap or not. If they do, we issue an error. + // If not, that is specialization which is allowed. + if module.declare_function(method.name_ident().clone(), *method_id).is_err() { + module.remove_function(method.name_ident()); + } } } + // Prohibit defining impls for primitive types if we're not in the stdlib } else if typ != Type::Error && !crate_id.is_stdlib() { let span = *span; @@ -84,7 +91,7 @@ pub(crate) fn collect_impls( pub(crate) fn resolve_impls( interner: &mut NodeInterner, - crate_id: CrateId, + crate_id: Option, def_maps: &BTreeMap, collected_impls: ImplMap, errors: &mut Vec<(CompilationError, FileId)>, @@ -95,41 +102,43 @@ pub(crate) fn resolve_impls( let path_resolver = StandardPathResolver::new(ModuleId { local_id: module_id, krate: crate_id }); - let file = def_maps[&crate_id].file_id(module_id); - - for (generics, _, functions) in methods { - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); - resolver.add_generics(&generics); - let generics = resolver.get_generics().to_vec(); - let self_type = resolver.resolve_type(unresolved_type.clone()); - - let mut file_func_ids = functions::resolve_function_set( - interner, - crate_id, - def_maps, - functions, - Some(self_type.clone()), - None, - generics, - errors, - ); - if self_type != Type::Error { - for (file_id, method_id) in &file_func_ids { - let method_name = interner.function_name(method_id).to_owned(); - - if let Some(first_fn) = - interner.add_method(&self_type, method_name.clone(), *method_id, false) - { - let error = ResolverError::DuplicateDefinition { - name: method_name, - first_span: interner.function_ident(&first_fn).span(), - second_span: interner.function_ident(method_id).span(), - }; - errors.push((error.into(), *file_id)); + if let Some(crate_id) = opt_crate_id { + let file = def_maps[&crate_id].file_id(module_id); + + for (generics, _, functions) in methods { + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + resolver.add_generics(&generics); + let generics = resolver.get_generics().to_vec(); + let self_type = resolver.resolve_type(unresolved_type.clone()); + + let mut file_func_ids = functions::resolve_function_set( + interner, + Some(crate_id), + def_maps, + functions, + Some(self_type.clone()), + None, + generics, + errors, + ); + if self_type != Type::Error { + for (file_id, method_id) in &file_func_ids { + let method_name = interner.function_name(method_id).to_owned(); + + if let Some(first_fn) = + interner.add_method(&self_type, method_name.clone(), *method_id, false) + { + let error = ResolverError::DuplicateDefinition { + name: method_name, + first_span: interner.function_ident(&first_fn).span(), + second_span: interner.function_ident(method_id).span(), + }; + errors.push((error.into(), *file_id)); + } } } + file_method_ids.append(&mut file_func_ids); } - file_method_ids.append(&mut file_func_ids); } } diff --git a/compiler/noirc_frontend/src/hir/resolution/mod.rs b/compiler/noirc_frontend/src/hir/resolution/mod.rs index 8c16a9cca80..0dbbf692a43 100644 --- a/compiler/noirc_frontend/src/hir/resolution/mod.rs +++ b/compiler/noirc_frontend/src/hir/resolution/mod.rs @@ -46,8 +46,12 @@ fn take_errors(file_id: FileId, resolver: Resolver<'_>) -> Vec<(CompilationError fn get_module_mut( def_maps: &mut BTreeMap, module: ModuleId, -) -> &mut ModuleData { - &mut def_maps.get_mut(&module.krate).unwrap().modules[module.local_id.0] +) -> Option<&mut ModuleData> { + if let Some(krate) = module.krate { + Some(&mut def_maps.get_mut(&krate).unwrap().modules[module.local_id.0]) + } else { + None + } } fn get_struct_type(typ: &Type) -> Option<&Shared> { diff --git a/compiler/noirc_frontend/src/hir/resolution/structs.rs b/compiler/noirc_frontend/src/hir/resolution/structs.rs index ed7aa86e718..dc455a5b269 100644 --- a/compiler/noirc_frontend/src/hir/resolution/structs.rs +++ b/compiler/noirc_frontend/src/hir/resolution/structs.rs @@ -21,7 +21,7 @@ use super::{errors::ResolverError, path_resolver::StandardPathResolver, resolver pub(crate) fn resolve_structs( context: &mut Context, structs: BTreeMap, - crate_id: CrateId, + crate_id: Option, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; // This is necessary to avoid cloning the entire struct map @@ -67,7 +67,7 @@ pub(crate) fn resolve_structs( fn resolve_struct_fields( context: &mut Context, - krate: CrateId, + krate: Option, type_id: StructId, unresolved: UnresolvedStruct, ) -> (Generics, Vec<(Ident, Type)>, Vec) { diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 8f966be312b..daf7d26a9ee 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -33,7 +33,7 @@ use super::{ pub(crate) fn resolve_traits( context: &mut Context, traits: BTreeMap, - crate_id: CrateId, + crate_id: Option, ) -> Vec<(CompilationError, FileId)> { for (trait_id, unresolved_trait) in &traits { context.def_interner.push_empty_trait(*trait_id, unresolved_trait); @@ -64,7 +64,7 @@ pub(crate) fn resolve_traits( // This check needs to be after the trait's methods are set since // the interner may set `interner.ordering_type` based on the result type // of the Cmp trait, if this is it. - if crate_id.is_stdlib() { + if crate_id.map(|id| id.is_stdlib()).unwrap_or(false) { context.def_interner.try_add_operator_trait(trait_id); } } @@ -73,7 +73,7 @@ pub(crate) fn resolve_traits( fn resolve_trait_types( _context: &mut Context, - _crate_id: CrateId, + _crate_id: Option, _unresolved_trait: &UnresolvedTrait, ) -> (Vec, Vec<(CompilationError, FileId)>) { // TODO @@ -81,7 +81,7 @@ fn resolve_trait_types( } fn resolve_trait_constants( _context: &mut Context, - _crate_id: CrateId, + _crate_id: Option, _unresolved_trait: &UnresolvedTrait, ) -> (Vec, Vec<(CompilationError, FileId)>) { // TODO @@ -91,7 +91,7 @@ fn resolve_trait_constants( fn resolve_trait_methods( context: &mut Context, trait_id: TraitId, - crate_id: CrateId, + crate_id: Option, unresolved_trait: &UnresolvedTrait, trait_generics: &Generics, ) -> (Vec, Vec<(CompilationError, FileId)>) { @@ -102,7 +102,7 @@ fn resolve_trait_methods( local_id: unresolved_trait.module_id, krate: crate_id, }); - let file = def_maps[&crate_id].file_id(unresolved_trait.module_id); + let file = def_maps[&crate_id.expect("crate resolved at this point")].file_id(unresolved_trait.module_id); let mut functions = vec![]; let mut resolver_errors = vec![]; @@ -177,7 +177,7 @@ fn resolve_trait_methods( fn collect_trait_impl_methods( interner: &mut NodeInterner, def_maps: &BTreeMap, - crate_id: CrateId, + crate_id: Option, trait_id: TraitId, trait_impl: &mut UnresolvedTraitImpl, ) -> Vec<(CompilationError, FileId)> { @@ -265,7 +265,7 @@ fn collect_trait_impl_methods( fn collect_trait_impl( context: &mut Context, - crate_id: CrateId, + opt_crate_id: Option, trait_impl: &mut UnresolvedTraitImpl, ) -> Vec<(CompilationError, FileId)> { let interner = &mut context.def_interner; @@ -286,26 +286,30 @@ fn collect_trait_impl( errors .extend(collect_trait_impl_methods(interner, def_maps, crate_id, trait_id, trait_impl)); - let path_resolver = StandardPathResolver::new(module); - let file = def_maps[&crate_id].file_id(trait_impl.module_id); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); - resolver.add_generics(&trait_impl.generics); - let typ = resolver.resolve_type(unresolved_type); - errors.extend(take_errors(trait_impl.file_id, resolver)); - - if let Some(struct_type) = get_struct_type(&typ) { - let struct_type = struct_type.borrow(); - let module = get_module_mut(def_maps, struct_type.id.module_id()); - - for (_, method_id, method) in &trait_impl.methods.functions { - // If this method was already declared, remove it from the module so it cannot - // be accessed with the `TypeName::method` syntax. We'll check later whether the - // object types in each method overlap or not. If they do, we issue an error. - // If not, that is specialization which is allowed. - if module.declare_function(method.name_ident().clone(), *method_id).is_err() { - module.remove_function(method.name_ident()); + if let Some(crate_id) = opt_crate_id { + let path_resolver = StandardPathResolver::new(module); + let file = def_maps[&crate_id].file_id(trait_impl.module_id); + let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + resolver.add_generics(&trait_impl.generics); + let typ = resolver.resolve_type(unresolved_type); + errors.extend(take_errors(trait_impl.file_id, resolver)); + + if let Some(struct_type) = get_struct_type(&typ) { + let struct_type = struct_type.borrow(); + let opt_module = get_module_mut(def_maps, struct_type.id.module_id()); + + for (_, method_id, method) in &trait_impl.methods.functions { + // If this method was already declared, remove it from the module so it cannot + // be accessed with the `TypeName::method` syntax. We'll check later whether the + // object types in each method overlap or not. If they do, we issue an error. + // If not, that is specialization which is allowed. + if let Some(module) = opt_module && module.declare_function(method.name_ident().clone(), *method_id).is_err() { + module.remove_function(method.name_ident()); + } } } + } else { + errors.extend(std::iter::one((CompilationError::ResolverError(ResolverError::MissingCrateId(module)), FileId::dummy()))); } } errors @@ -313,7 +317,7 @@ fn collect_trait_impl( pub(crate) fn collect_trait_impls( context: &mut Context, - crate_id: CrateId, + crate_id: Option, collected_impls: &mut [UnresolvedTraitImpl], ) -> Vec<(CompilationError, FileId)> { collected_impls @@ -326,20 +330,24 @@ fn check_trait_impl_crate_coherence( interner: &mut NodeInterner, trait_id: TraitId, trait_impl: &UnresolvedTraitImpl, - current_crate: CrateId, + current_crate: Option, def_maps: &BTreeMap, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; let module = ModuleId { krate: current_crate, local_id: trait_impl.module_id }; - let file = def_maps[¤t_crate].file_id(trait_impl.module_id); - let path_resolver = StandardPathResolver::new(module); - let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); + if let Some(current_crate) = opt_current_crate { + let file = def_maps[¤t_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, - }; + let object_crate = match resolver.resolve_type(trait_impl.object_type.clone()) { + Type::Struct(struct_type, _) => struct_type.borrow().id.krate(), + _ => None, + }; + } else { + let object_crate = None; + } let the_trait = interner.get_trait(trait_id); if current_crate != the_trait.crate_id && current_crate != object_crate { @@ -368,7 +376,7 @@ pub(crate) fn resolve_trait_by_path( pub(crate) fn resolve_trait_impls( context: &mut Context, traits: Vec, - crate_id: CrateId, + crate_id: Option, errors: &mut Vec<(CompilationError, FileId)>, ) -> Vec<(FileId, FuncId)> { let interner = &mut context.def_interner; diff --git a/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs b/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs index f66f6c8dfa7..53be08f360d 100644 --- a/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs +++ b/compiler/noirc_frontend/src/hir/resolution/type_aliases.rs @@ -14,7 +14,7 @@ use std::collections::BTreeMap; pub(crate) fn resolve_type_aliases( context: &mut Context, type_aliases: BTreeMap, - crate_id: CrateId, + crate_id: Option, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; for (type_id, unresolved_typ) in type_aliases { diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs index 16b9899039f..6fbc8254ec5 100644 --- a/compiler/noirc_frontend/src/hir_def/traits.rs +++ b/compiler/noirc_frontend/src/hir_def/traits.rs @@ -40,7 +40,7 @@ pub struct Trait { /// struct traits are equal. pub id: TraitId, - pub crate_id: CrateId, + pub crate_id: Option, pub methods: Vec, diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 0051c1b4f5f..5a59962466b 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -333,14 +333,14 @@ impl StructId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> StructId { - StructId(ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() }) + StructId(ModuleId { krate: None, local_id: LocalModuleId::dummy_id() }) } pub fn module_id(self) -> ModuleId { self.0 } - pub fn krate(self) -> CrateId { + pub fn krate(self) -> Option { self.0.krate } @@ -366,7 +366,7 @@ impl TraitId { // This can be anything, as the program will ultimately fail // after resolution pub fn dummy_id() -> TraitId { - TraitId(ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() }) + TraitId(ModuleId { krate: None, local_id: LocalModuleId::dummy_id() }) } } @@ -577,7 +577,7 @@ impl NodeInterner { pub fn new_struct( &mut self, typ: &UnresolvedStruct, - krate: CrateId, + krate: Option, local_id: LocalModuleId, file_id: FileId, ) -> StructId { From d81c1ddac0bdad1715264e79e163da0b1dd49976 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Thu, 22 Feb 2024 15:45:34 -0500 Subject: [PATCH 3/3] wip propagating option --- .../src/hir/def_collector/dc_crate.rs | 2 +- .../src/hir/def_collector/dc_mod.rs | 2 +- .../noirc_frontend/src/hir/def_map/mod.rs | 4 +- .../src/hir/resolution/import.rs | 59 +++++++++++-------- .../src/hir/resolution/path_resolver.rs | 20 ++++--- .../src/hir/resolution/resolver.rs | 19 +++--- 6 files changed, 62 insertions(+), 44 deletions(-) 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 f12c80afb17..5350c87a198 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -402,7 +402,7 @@ fn inject_prelude( path, ) { let module_id = module_def.as_module().expect("std::prelude should be a module"); - let prelude = context.module(module_id).scope().names(); + let prelude = context.module(module_id).map(scope).unwrap_or_else(|| ItemScope::default()).names(); for path in prelude { let mut segments = segments.clone(); diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index b4091cf2650..d8e23b3588d 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -612,7 +612,7 @@ impl<'a> ModCollector<'a> { // the struct name. if add_to_parent_scope { let mod_id = ModuleId { - krate: self.def_collector.def_map.krate, + krate: Some(self.def_collector.def_map.krate), local_id: LocalModuleId(module_id), }; diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 01ccd6fc1ca..4274e3a9c4b 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -50,7 +50,7 @@ impl ModuleId { impl ModuleId { pub fn module(self, def_maps: &BTreeMap) -> Option<&ModuleData> { if let Some(krate) = self.krate { - &def_maps[&krate].modules()[self.local_id.0] + Some(&def_maps[&krate].modules()[self.local_id.0]) } else { None } @@ -85,7 +85,7 @@ impl CrateDefMap { // expect the same crate to be processed twice. It would not // make the implementation wrong, if the same crate was processed twice, it just makes it slow. let mut errors: Vec<(CompilationError, FileId)> = vec![]; - if context.def_map(&crate_id).is_some() { + if context.def_map(Some(&crate_id)).is_some() { return errors; } diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index e6ac33053a0..c1754e1cc21 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -20,6 +20,7 @@ pub type PathResolution = Result; pub enum PathResolutionError { Unresolved(Ident), ExternalContractUsed(Ident), + DummyCrateId(ModuleId, Path), } #[derive(Debug)] @@ -58,7 +59,7 @@ pub fn resolve_import( let def_map = &def_maps[&crate_id]; let allow_contracts = - allow_referencing_contracts(def_maps, crate_id, import_directive.module_id); + allow_referencing_contracts(def_maps, Some(crate_id), import_directive.module_id); let module_scope = import_directive.module_id; let resolved_namespace = @@ -76,10 +77,10 @@ pub fn resolve_import( pub(super) fn allow_referencing_contracts( def_maps: &BTreeMap, - krate: CrateId, + krate: Option, local_id: LocalModuleId, ) -> bool { - ModuleId { krate, local_id }.module(def_maps).is_contract + ModuleId { krate, local_id }.module(def_maps).map(is_contract).unwrap_or(false) } pub fn resolve_path_to_ns( @@ -88,7 +89,7 @@ pub fn resolve_path_to_ns( def_maps: &BTreeMap, allow_contracts: bool, ) -> PathResolution { - let import_path = &import_directive.path.segments; + let import_path = import_directive.path; match import_directive.path.kind { crate::ast::PathKind::Crate => { @@ -114,7 +115,7 @@ pub fn resolve_path_to_ns( fn resolve_path_from_crate_root( def_map: &CrateDefMap, - import_path: &[Ident], + import_path: Path, def_maps: &BTreeMap, allow_contracts: bool, ) -> PathResolution { @@ -123,7 +124,7 @@ fn resolve_path_from_crate_root( fn resolve_name_in_module( def_map: &CrateDefMap, - import_path: &[Ident], + import_path: Path, starting_mod: LocalModuleId, def_maps: &BTreeMap, allow_contracts: bool, @@ -132,19 +133,19 @@ fn resolve_name_in_module( // 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 }; + if import_path.segments.is_empty() { + let mod_id = ModuleId { krate: Some(def_map.krate), local_id: starting_mod }; return Ok(PerNs::types(mod_id.into())); } - let mut import_path = import_path.iter(); - let first_segment = import_path.next().expect("ice: could not fetch first segment"); + let mut import_path_iter = import_path.segments.iter(); + let first_segment = import_path_iter.next().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 { + for segment in import_path_iter { let typ = match current_ns.take_types() { None => return Err(PathResolutionError::Unresolved(segment.clone())), Some(typ) => typ, @@ -161,19 +162,23 @@ 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]; - - // Check if namespace - let found_ns = current_mod.find_name(segment); - - if found_ns.is_none() { - return Err(PathResolutionError::Unresolved(segment.clone())); + if let Some(krate) = new_module_id.krate { + current_mod = &def_maps[&krate].modules[new_module_id.local_id.0]; + + // Check if namespace + let found_ns = current_mod.find_name(segment); + + if found_ns.is_none() { + return Err(PathResolutionError::Unresolved(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())); + } + current_ns = found_ns; + } else { + return Err(PathResolutionError::DummyCrateId(new_module_id, import_path)) } - // 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())); - } - current_ns = found_ns; } Ok(current_ns) @@ -218,7 +223,11 @@ fn resolve_external_dep( is_prelude: false, }; - let dep_def_map = def_maps.get(&dep_module.krate).unwrap(); + if let Some(krate) = dep_module.krate { + let dep_def_map = def_maps.get(&krate).unwrap(); - resolve_path_to_ns(&dep_directive, dep_def_map, def_maps, allow_contracts) + resolve_path_to_ns(&dep_directive, dep_def_map, def_maps, allow_contracts) + } else { + Err(PathResolutionError::DummyCrateId(*dep_module, path)) + } } diff --git a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs index 4c5fa3bceef..e97b0572095 100644 --- a/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -59,13 +59,17 @@ pub fn resolve_path( // 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 def_map = &def_maps[&module_id.krate]; - let ns = resolve_path_to_ns(&import, def_map, def_maps, allow_referencing_contracts)?; - - let function = ns.values.map(|(id, _, _)| id); - let id = function.or_else(|| ns.types.map(|(id, _, _)| id)); - Ok(id.expect("Found empty namespace")) + if let Some(krate) = module_id.krate { + let allow_referencing_contracts = + allow_referencing_contracts(def_maps, Some(krate), module_id.local_id); + let def_map = &def_maps[&krate]; + let ns = resolve_path_to_ns(&import, def_map, def_maps, allow_referencing_contracts)?; + + let function = ns.values.map(|(id, _, _)| id); + let id = function.or_else(|| ns.types.map(|(id, _, _)| id)); + Ok(id.expect("Found empty namespace")) + } else { + Err(PathResolutionError::DummyCrateId(module_id, path)) + } } diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 7f9e48353a7..007c5639861 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -138,7 +138,7 @@ impl<'a> Resolver<'a> { file: FileId, ) -> Resolver<'a> { let module_id = path_resolver.module_id(); - let in_contract = module_id.module(def_maps).is_contract; + let in_contract = module_id.module(def_maps).map(is_contract).unwrap_or(false); Self { path_resolver, @@ -934,7 +934,7 @@ impl<'a> Resolver<'a> { } let is_low_level_function = func.attributes().function.as_ref().map_or(false, |func| func.is_low_level()); - if !self.path_resolver.module_id().krate.is_stdlib() && is_low_level_function { + if !self.path_resolver.module_id().krate.map(is_stdlib).unwrap_or(false) && is_low_level_function { let error = ResolverError::LowLevelFunctionOutsideOfStdlib { ident: func.name_ident().clone() }; self.push_err(error); @@ -1253,7 +1253,7 @@ impl<'a> Resolver<'a> { return Some(self.resolve_expression(assert_message_expr)); } - let is_in_stdlib = self.path_resolver.module_id().krate.is_stdlib(); + let is_in_stdlib = self.path_resolver.module_id().krate.map(is_stdlib).unwrap_or(false); let assert_msg_call_path = if is_in_stdlib { ExpressionKind::Variable(Path { segments: vec![Ident::from("resolve_assert_message")], @@ -1345,7 +1345,7 @@ impl<'a> Resolver<'a> { // This is also true if `current == target`. fn module_descendent_of_target( &self, - krate: CrateId, + opt_krate: Option, target: LocalModuleId, current: LocalModuleId, ) -> bool { @@ -1353,9 +1353,14 @@ impl<'a> Resolver<'a> { return true; } - self.def_maps[&krate].modules[current.0] - .parent - .map_or(false, |parent| self.module_descendent_of_target(krate, target, parent)) + if let Some(krate) = opt_krate { + self.def_maps[&krate].modules[current.0] + .parent + .map_or(false, |parent| self.module_descendent_of_target(Some(krate), target, parent)) + + } else { + current == target + } } fn resolve_local_variable(&mut self, hir_ident: HirIdent, var_scope_index: usize) {