From 19fefde9179a6ee0c66572d112b5d1d25b1ddcc3 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 6 Apr 2023 15:22:12 -0500 Subject: [PATCH 1/2] Separate items defined within a module from those just imported into the module --- .../src/hir/def_collector/dc_crate.rs | 12 ++-- .../src/hir/def_collector/dc_mod.rs | 15 ++--- .../src/hir/def_map/item_scope.rs | 33 ++-------- crates/noirc_frontend/src/hir/def_map/mod.rs | 17 +++-- .../src/hir/def_map/module_data.rs | 62 ++++++++++++++++++- .../src/hir/resolution/import.rs | 4 +- 6 files changed, 84 insertions(+), 59 deletions(-) diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs index e932f91f75a..b61376f39ae 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -138,8 +138,7 @@ impl DefCollector { let name = resolved_import.name; for ns in resolved_import.resolved_namespace.iter_defs() { let result = current_def_map.modules[resolved_import.module_scope.0] - .scope - .add_item_to_namespace(name.clone(), ns); + .import(name.clone(), ns); if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateImport { first_def, second_def }; @@ -224,14 +223,13 @@ fn collect_impls( extend_errors(errors, unresolved.file_id, resolver.take_errors()); if let Some(type_module) = get_local_id_from_type(&typ) { - // Grab the scope defined by the struct type. Note that impls are a case - // where the scope the methods are added to is not the same as the scope + // 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 scope = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0].scope; + let module = &mut def_maps.get_mut(&crate_id).unwrap().modules[type_module.0]; - // .define_func_def(name, func_id); for (_, method_id, method) in &unresolved.functions { - let result = scope.define_func_def(method.name_ident().clone(), *method_id); + let result = module.declare_function(method.name_ident().clone(), *method_id); if let Err((first_def, second_def)) = result { let err = diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs index 1a58decda99..bee7f1755c8 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -75,9 +75,8 @@ impl<'a> ModCollector<'a> { let stmt_id = context.def_interner.push_empty_global(); // Add the statement to the scope so its path can be looked up later - let result = self.def_collector.def_map.modules[self.module_id.0] - .scope - .define_global(name, stmt_id); + let result = + self.def_collector.def_map.modules[self.module_id.0].declare_global(name, stmt_id); if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateGlobal { first_def, second_def }; @@ -137,8 +136,7 @@ impl<'a> ModCollector<'a> { // Add function to scope/ns of the module let result = self.def_collector.def_map.modules[self.module_id.0] - .scope - .define_func_def(name, func_id); + .declare_function(name, func_id); if let Err((first_def, second_def)) = result { let error = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; @@ -167,9 +165,8 @@ impl<'a> ModCollector<'a> { }; // Add the struct to scope so its path can be looked up later - let result = self.def_collector.def_map.modules[self.module_id.0] - .scope - .define_struct_def(name, id); + let result = + self.def_collector.def_map.modules[self.module_id.0].declare_struct(name, id); if let Err((first_def, second_def)) = result { let err = DefCollectorErrorKind::DuplicateFunction { first_def, second_def }; @@ -288,7 +285,7 @@ impl<'a> ModCollector<'a> { }; if let Err((first_def, second_def)) = - modules[self.module_id.0].scope.define_module_def(mod_name.to_owned(), mod_id) + modules[self.module_id.0].declare_child_module(mod_name.to_owned(), mod_id) { let err = DefCollectorErrorKind::DuplicateModuleDecl { first_def, second_def }; errors.push(err.into_file_diagnostic(self.file_id)); diff --git a/crates/noirc_frontend/src/hir/def_map/item_scope.rs b/crates/noirc_frontend/src/hir/def_map/item_scope.rs index 6a819550e72..52201f7ade3 100644 --- a/crates/noirc_frontend/src/hir/def_map/item_scope.rs +++ b/crates/noirc_frontend/src/hir/def_map/item_scope.rs @@ -1,8 +1,5 @@ use super::{namespace::PerNs, ModuleDefId, ModuleId}; -use crate::{ - node_interner::{FuncId, StmtId, StructId}, - Ident, -}; +use crate::{node_interner::FuncId, Ident}; use std::collections::{hash_map::Entry, HashMap}; #[derive(Debug, PartialEq, Eq, Copy, Clone)] @@ -55,30 +52,6 @@ impl ItemScope { } } - pub fn define_module_def( - &mut self, - name: Ident, - mod_id: ModuleId, - ) -> Result<(), (Ident, Ident)> { - self.add_definition(name, mod_id.into()) - } - - pub fn define_func_def(&mut self, name: Ident, local_id: FuncId) -> Result<(), (Ident, Ident)> { - self.add_definition(name, local_id.into()) - } - - pub fn define_struct_def( - &mut self, - name: Ident, - local_id: StructId, - ) -> Result<(), (Ident, Ident)> { - self.add_definition(name, ModuleDefId::TypeId(local_id)) - } - - pub fn define_global(&mut self, name: Ident, stmt_id: StmtId) -> Result<(), (Ident, Ident)> { - self.add_definition(name, ModuleDefId::GlobalId(stmt_id)) - } - pub fn find_module_with_name(&self, mod_name: &Ident) -> Option<&ModuleId> { let (module_def, _) = self.types.get(mod_name)?; match module_def { @@ -86,6 +59,7 @@ impl ItemScope { _ => None, } } + pub fn find_func_with_name(&self, func_name: &Ident) -> Option { let (module_def, _) = self.values.get(func_name)?; match module_def { @@ -93,6 +67,7 @@ impl ItemScope { _ => None, } } + pub fn find_name(&self, name: &Ident) -> PerNs { PerNs { types: self.types.get(name).cloned(), values: self.values.get(name).cloned() } } @@ -100,9 +75,11 @@ impl ItemScope { pub fn definitions(&self) -> Vec { self.defs.clone() } + pub fn types(&self) -> &HashMap { &self.types } + pub fn values(&self) -> &HashMap { &self.values } diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index abf07c8d2a8..25e0488a7b6 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -110,7 +110,7 @@ impl CrateDefMap { // This function accepts an Ident, so we attach a dummy span to // "main". Equality is implemented only on the contents. - root_module.scope.find_func_with_name(&MAIN_FUNCTION.into()) + root_module.find_func_with_name(&MAIN_FUNCTION.into()) } pub fn root_file_id(&self) -> FileId { @@ -129,8 +129,10 @@ impl CrateDefMap { interner: &'a NodeInterner, ) -> impl Iterator + 'a { self.modules.iter().flat_map(|(_, module)| { - let functions = module.scope.values().values().filter_map(|(id, _)| id.as_function()); - functions.filter(|id| interner.function_meta(id).attributes == Some(Attribute::Test)) + module + .value_definitions() + .filter_map(|id| id.as_function()) + .filter(|id| interner.function_meta(id).attributes == Some(Attribute::Test)) }) } @@ -141,13 +143,8 @@ impl CrateDefMap { .iter() .filter_map(|(id, module)| { if module.is_contract { - let functions = module - .scope - .values() - .values() - .filter_map(|(id, _)| id.as_function()) - .collect(); - + let functions = + module.value_definitions().filter_map(|id| id.as_function()).collect(); let name = self.get_module_path(id, module.parent); Some(Contract { name, functions }) } else { diff --git a/crates/noirc_frontend/src/hir/def_map/module_data.rs b/crates/noirc_frontend/src/hir/def_map/module_data.rs index d437a4d1b6c..20906885ad9 100644 --- a/crates/noirc_frontend/src/hir/def_map/module_data.rs +++ b/crates/noirc_frontend/src/hir/def_map/module_data.rs @@ -2,9 +2,12 @@ use std::collections::HashMap; use fm::FileId; -use crate::Ident; +use crate::{ + node_interner::{FuncId, StmtId, StructId}, + Ident, +}; -use super::{ItemScope, LocalModuleId}; +use super::{ItemScope, LocalModuleId, ModuleDefId, ModuleId, PerNs}; /// Contains the actual contents of a module: its parent (if one exists), /// children, and scope with all definitions defined within the scope. @@ -12,7 +15,13 @@ use super::{ItemScope, LocalModuleId}; pub struct ModuleData { pub parent: Option, pub children: HashMap, - pub scope: ItemScope, + + /// Contains all definitions visible to the current module. This includes + /// all definitions in self.definitions as well as all imported definitions. + scope: ItemScope, + + /// Contains only the definitions directly defined in the current module + definitions: ItemScope, pub origin: ModuleOrigin, @@ -30,10 +39,57 @@ impl ModuleData { parent, children: HashMap::new(), scope: ItemScope::default(), + definitions: ItemScope::default(), origin, is_contract, } } + + fn declare(&mut self, name: Ident, item_id: ModuleDefId) -> Result<(), (Ident, Ident)> { + self.scope.add_definition(name.clone(), item_id)?; + + // definitions is a subset of self.scope so it is expected if self.scope.define_func_def + // returns without error, so will self.definitions.define_func_def. + self.definitions.add_definition(name, item_id) + } + + pub fn declare_function(&mut self, name: Ident, id: FuncId) -> Result<(), (Ident, Ident)> { + self.declare(name, id.into()) + } + + pub fn declare_global(&mut self, name: Ident, id: StmtId) -> Result<(), (Ident, Ident)> { + self.declare(name, id.into()) + } + + pub fn declare_struct(&mut self, name: Ident, id: StructId) -> Result<(), (Ident, Ident)> { + self.declare(name, ModuleDefId::TypeId(id)) + } + + pub fn declare_child_module( + &mut self, + name: Ident, + child_id: ModuleId, + ) -> Result<(), (Ident, Ident)> { + self.declare(name, child_id.into()) + } + + pub fn find_func_with_name(&self, name: &Ident) -> Option { + self.scope.find_func_with_name(name) + } + + pub fn import(&mut self, name: Ident, id: ModuleDefId) -> Result<(), (Ident, Ident)> { + self.scope.add_item_to_namespace(name, id) + } + + pub fn find_name(&self, name: &Ident) -> PerNs { + self.scope.find_name(name) + } + + /// Return an iterator over all definitions defined within this module, + /// excluding any type definitions. + pub fn value_definitions(&self) -> impl Iterator + '_ { + self.definitions.values().values().map(|(id, _)| *id) + } } #[derive(Debug, PartialEq, Eq, Copy, Clone)] diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index 836333ee5d9..bd8f3e5e634 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -135,7 +135,7 @@ fn resolve_name_in_module( let mut import_path = import_path.iter(); let first_segment = import_path.next().expect("ice: could not fetch first segment"); - let mut current_ns = current_mod.scope.find_name(first_segment); + let mut current_ns = current_mod.find_name(first_segment); if current_ns.is_none() { return Err(PathResolutionError::Unresolved(first_segment.clone())); } @@ -158,7 +158,7 @@ fn resolve_name_in_module( current_mod = &def_maps[&new_module_id.krate].modules[new_module_id.local_id.0]; // Check if namespace - let found_ns = current_mod.scope.find_name(segment); + let found_ns = current_mod.find_name(segment); if found_ns.is_none() { return Err(PathResolutionError::Unresolved(segment.clone())); } From 1b70e3f13aea65148b3ecdfdb44bb9f518e2ca06 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 6 Apr 2023 16:00:02 -0500 Subject: [PATCH 2/2] Fix test --- crates/noirc_frontend/src/main.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/noirc_frontend/src/main.rs b/crates/noirc_frontend/src/main.rs index f68c52e3129..b1a5c0f950e 100644 --- a/crates/noirc_frontend/src/main.rs +++ b/crates/noirc_frontend/src/main.rs @@ -41,18 +41,15 @@ fn main() { // Get root module let root = def_map.root(); let module = def_map.modules().get(root.0).unwrap(); - for (name, (def_id, vis)) in module.scope.values() { - println!("func name is {:?}", name); + for def_id in module.value_definitions() { let func_id = match def_id { ModuleDefId::FunctionId(func_id) => func_id, _ => unreachable!(), }; // Get the HirFunction for that Id - let hir = context.def_interner.function(func_id); - + let hir = context.def_interner.function(&func_id); println!("func hir is {:?}", hir); - println!("func vis is {:?}", vis); } //