Skip to content

Commit

Permalink
fix: handle multiple imports in the same file (#3903)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
guipublic committed Jan 4, 2024
1 parent e5e52a8 commit 219423e
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 52 deletions.
60 changes: 28 additions & 32 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
}
}
}
Expand Down
37 changes: 17 additions & 20 deletions compiler/noirc_frontend/src/hir/resolution/import.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use iter_extended::partition_results;
use noirc_errors::{CustomDiagnostic, Span};

use crate::graph::CrateId;
Expand Down Expand Up @@ -51,29 +50,27 @@ impl From<PathResolutionError> for CustomDiagnostic {
}
}

pub fn resolve_imports(
pub fn resolve_import(
crate_id: CrateId,
imports_to_resolve: Vec<ImportDirective>,
import_directive: ImportDirective,
def_maps: &BTreeMap<CrateId, CrateDefMap>,
) -> (Vec<ResolvedImport>, Vec<(PathResolutionError, LocalModuleId)>) {
) -> Result<ResolvedImport, (PathResolutionError, LocalModuleId)> {
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,
})
}

Expand Down
7 changes: 7 additions & 0 deletions test_programs/execution_success/regression_3889/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_3889"
version = "0.1.0"
type = "bin"
authors = [""]

[dependencies]
10 changes: 10 additions & 0 deletions test_programs/execution_success/regression_3889/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[works]
a = "5"

[fails]
a = "6"


[also_fails]
a = "7"

23 changes: 23 additions & 0 deletions test_programs/execution_success/regression_3889/src/main.nr
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 219423e

Please sign in to comment.