From 219423eb87fa12bd8cca2a6fd2ce4c06e308783c Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Thu, 4 Jan 2024 11:56:08 +0100 Subject: [PATCH] fix: handle multiple imports in the same file (#3903) # Description ## Problem\* Resolves #3889 ## Summary\* An import of an imported object in the same file would not work because the imports are added to the module after they are resolved. To fix this we simply process the imports one-by-one. ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../src/hir/def_collector/dc_crate.rs | 60 +++++++++---------- .../src/hir/resolution/import.rs | 37 ++++++------ .../regression_3889/Nargo.toml | 7 +++ .../regression_3889/Prover.toml | 10 ++++ .../regression_3889/src/main.nr | 23 +++++++ 5 files changed, 85 insertions(+), 52 deletions(-) create mode 100644 test_programs/execution_success/regression_3889/Nargo.toml create mode 100644 test_programs/execution_success/regression_3889/Prover.toml create mode 100644 test_programs/execution_success/regression_3889/src/main.nr 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 ae061792125..8ada3faf756 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_imports, ImportDirective}; +use crate::hir::resolution::import::{resolve_import, ImportDirective}; use crate::hir::resolution::resolver::Resolver; use crate::hir::resolution::{ collect_impls, collect_trait_impls, path_resolver, resolve_free_functions, resolve_globals, @@ -259,37 +259,33 @@ impl DefCollector { ); } - // Resolve unresolved imports collected from the crate - let (resolved, unresolved_imports) = - resolve_imports(crate_id, def_collector.collected_imports, &context.def_maps); - - { - let current_def_map = context.def_maps.get(&crate_id).unwrap(); - errors.extend(vecmap(unresolved_imports, |(error, module_id)| { - let file_id = current_def_map.file_id(module_id); - let error = DefCollectorErrorKind::PathResolutionError(error); - (error.into(), file_id) - })); - }; - - // Populate module namespaces according to the imports used - let current_def_map = context.def_maps.get_mut(&crate_id).unwrap(); - for resolved_import in resolved { - 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].import( - name.clone(), - ns, - resolved_import.is_prelude, - ); - - if let Err((first_def, second_def)) = result { - let err = DefCollectorErrorKind::Duplicate { - typ: DuplicateType::Import, - first_def, - second_def, - }; - errors.push((err.into(), root_file_id)); + // Resolve unresolved imports collected from the crate, one by one. + for collected_import in def_collector.collected_imports { + match resolve_import(crate_id, collected_import, &context.def_maps) { + Ok(resolved_import) => { + // Populate module namespaces according to the imports used + let current_def_map = context.def_maps.get_mut(&crate_id).unwrap(); + + 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] + .import(name.clone(), ns, resolved_import.is_prelude); + + if let Err((first_def, second_def)) = result { + let err = DefCollectorErrorKind::Duplicate { + typ: DuplicateType::Import, + first_def, + second_def, + }; + errors.push((err.into(), root_file_id)); + } + } + } + Err((error, module_id)) => { + let current_def_map = context.def_maps.get(&crate_id).unwrap(); + let file_id = current_def_map.file_id(module_id); + let error = DefCollectorErrorKind::PathResolutionError(error); + errors.push((error.into(), file_id)); } } } diff --git a/compiler/noirc_frontend/src/hir/resolution/import.rs b/compiler/noirc_frontend/src/hir/resolution/import.rs index 41fdac746bd..e6ac33053a0 100644 --- a/compiler/noirc_frontend/src/hir/resolution/import.rs +++ b/compiler/noirc_frontend/src/hir/resolution/import.rs @@ -1,4 +1,3 @@ -use iter_extended::partition_results; use noirc_errors::{CustomDiagnostic, Span}; use crate::graph::CrateId; @@ -51,29 +50,27 @@ impl From for CustomDiagnostic { } } -pub fn resolve_imports( +pub fn resolve_import( crate_id: CrateId, - imports_to_resolve: Vec, + import_directive: ImportDirective, def_maps: &BTreeMap, -) -> (Vec, Vec<(PathResolutionError, LocalModuleId)>) { +) -> Result { let def_map = &def_maps[&crate_id]; - partition_results(imports_to_resolve, |import_directive| { - 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 name = resolve_path_name(&import_directive); - Ok(ResolvedImport { - name, - resolved_namespace, - module_scope, - is_prelude: import_directive.is_prelude, - }) + 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 name = resolve_path_name(&import_directive); + Ok(ResolvedImport { + name, + resolved_namespace, + module_scope, + is_prelude: import_directive.is_prelude, }) } diff --git a/test_programs/execution_success/regression_3889/Nargo.toml b/test_programs/execution_success/regression_3889/Nargo.toml new file mode 100644 index 00000000000..d212d24473f --- /dev/null +++ b/test_programs/execution_success/regression_3889/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "regression_3889" +version = "0.1.0" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_3889/Prover.toml b/test_programs/execution_success/regression_3889/Prover.toml new file mode 100644 index 00000000000..a81ab67fe3e --- /dev/null +++ b/test_programs/execution_success/regression_3889/Prover.toml @@ -0,0 +1,10 @@ +[works] +a = "5" + +[fails] +a = "6" + + +[also_fails] +a = "7" + diff --git a/test_programs/execution_success/regression_3889/src/main.nr b/test_programs/execution_success/regression_3889/src/main.nr new file mode 100644 index 00000000000..10b8ecabee3 --- /dev/null +++ b/test_programs/execution_success/regression_3889/src/main.nr @@ -0,0 +1,23 @@ +mod Foo { + struct NewType{ + a: Field, + } +} + +mod Bar { + use crate::Foo::NewType as BarStruct; + use crate::Foo::NewType; +} + +mod Baz { + struct Works { + a: Field, + } + use crate::Bar::BarStruct; + use crate::Bar::NewType; +} + + +fn main(works: Baz::Works, fails: Baz::BarStruct, also_fails: Bar::NewType) -> pub Field { + works.a + fails.a + also_fails.a +}