Skip to content

Commit

Permalink
Rollup merge of #124840 - bvanjoi:fix-124490, r=petrochenkov
Browse files Browse the repository at this point in the history
resolve: mark it undetermined if single import is not has any bindings

- Fixes #124490
- Fixes #125013

This issue arises from incorrect resolution updates, for example:

```rust
mod a {
    pub mod b {
        pub mod c {}
    }
}

use a::*;

use b::c;
use c as b;

fn main() {}
```

1. In the first loop, binding `(root, b)` is refer to `root::a::b` due to `use a::*`.
    1. However, binding `(root, c)` isn't defined by `use b::c` during this stage because `use c as b` falls under the `single_imports` of `(root, b)`, where the `imported_module` hasn't been computed yet. This results in marking the `path_res` for `b` as `Indeterminate`.
    2. Then, the `imported_module` for `use c as b` will be recorded.
2. In the second loop, `use b::c` will be processed again:
    1. Firstly, it attempts to find the `path_res` for `(root, b)`.
    2. It will iterate through the `single_imports` of `use b::c`, encounter `use c as b`, attempt to resolve `c` in `root`, and ultimately return `Err(Undetermined)`, thus passing the iterator.
    3. Use the binding `(root, b)` -> `root::a::b` introduced by `use a::*` and ultimately return `root::a::b` as the `path_res` of `b`.
    4. Then define the binding `(root, c)` -> `root::a::b::c`.
3. Then process `use c as b`, update the resolution for `(root, b)` to refer to `root::a::b::c`, ultimately causing inconsistency.

In my view, step `2.2` has an issue where it should exit early, similar to the behavior when there's no `imported_module`. Therefore, I've added an attribute called `indeterminate` to `ImportData`. This will help us handle only those single imports that have at least one determined binding.

r? ``@petrochenkov``
  • Loading branch information
matthiaskrgr authored Jun 5, 2024
2 parents db8aca4 + f67a0eb commit 69a8c13
Show file tree
Hide file tree
Showing 11 changed files with 142 additions and 41 deletions.
29 changes: 27 additions & 2 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// if it can then our result is not determined and can be invalidated.
for single_import in &resolution.single_imports {
let Some(import_vis) = single_import.vis.get() else {
// This branch handles a cycle in single imports, which occurs
// when we've previously captured the `vis` value during an import
// process.
//
// For example:
// ```
// use a::b;
// use b as a;
// ```
// 1. Steal the `vis` in `use a::b` and attempt to locate `a` in the
// current module.
// 2. Encounter the import `use b as a`, which is a `single_import` for `a`,
// and try to find `b` in the current module.
// 3. Re-encounter the `use a::b` import since it's a `single_import` of `b`.
// This leads to entering this branch.
continue;
};
if !self.is_accessible_from(import_vis, parent_scope.module) {
Expand All @@ -979,15 +994,25 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// named imports.
continue;
}

let Some(module) = single_import.imported_module.get() else {
return Err((Undetermined, Weak::No));
};
let ImportKind::Single { source: ident, .. } = single_import.kind else {
let ImportKind::Single { source: ident, source_bindings, .. } = &single_import.kind
else {
unreachable!();
};
if binding.map_or(false, |binding| binding.module().is_some())
&& source_bindings.iter().all(|binding| matches!(binding.get(), Err(Undetermined)))
{
// This branch allows the binding to be defined or updated later,
// avoiding module inconsistency between the resolve process and the finalize process.
// See more details in #124840
return Err((Undetermined, Weak::No));
}
match self.resolve_ident_in_module(
module,
ident,
*ident,
ns,
&single_import.parent_scope,
None,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
(old_glob @ true, false) | (old_glob @ false, true) => {
let (glob_binding, nonglob_binding) =
if old_glob { (old_binding, binding) } else { (binding, old_binding) };
if glob_binding.res() != nonglob_binding.res()
&& key.ns == MacroNS
if key.ns == MacroNS
&& nonglob_binding.expansion != LocalExpnId::ROOT
&& glob_binding.res() != nonglob_binding.res()
{
resolution.binding = Some(this.ambiguity(
AmbiguityKind::GlobVsExpanded,
Expand Down
16 changes: 0 additions & 16 deletions tests/crashes/124490.rs

This file was deleted.

5 changes: 0 additions & 5 deletions tests/crashes/125013-1.rs

This file was deleted.

16 changes: 0 additions & 16 deletions tests/crashes/125013-2.rs

This file was deleted.

14 changes: 14 additions & 0 deletions tests/ui/imports/cycle-import-in-diff-module-0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ check-pass

// https://github.com/rust-lang/rust/pull/124840#issuecomment-2098148587

mod a {
pub(crate) use crate::S;
}
mod b {
pub struct S;
}
use self::a::S;
use self::b::*;

fn main() {}
14 changes: 14 additions & 0 deletions tests/ui/imports/cycle-import-in-diff-module-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ check-pass

// similar `cycle-import-in-diff-module-0.rs`

mod a {
pub(crate) use crate::s;
}
mod b {
pub mod s {}
}
use self::b::*;
use self::a::s;

fn main() {}
17 changes: 17 additions & 0 deletions tests/ui/imports/shadow-glob-module-resolution-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// https://github.com/rust-lang/rust/issues/124490

mod a {
pub mod b {
pub mod c {}
}
}

use a::*;

use b::c;
//~^ ERROR: cannot determine resolution for the import
//~| ERROR: cannot determine resolution for the import
//~| ERROR: unresolved import `b::c`
use c as b;

fn main() {}
23 changes: 23 additions & 0 deletions tests/ui/imports/shadow-glob-module-resolution-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error: cannot determine resolution for the import
--> $DIR/shadow-glob-module-resolution-1.rs:11:5
|
LL | use b::c;
| ^^^^

error: cannot determine resolution for the import
--> $DIR/shadow-glob-module-resolution-1.rs:11:5
|
LL | use b::c;
| ^^^^
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error[E0432]: unresolved import `b::c`
--> $DIR/shadow-glob-module-resolution-1.rs:11:5
|
LL | use b::c;
| ^^^^

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0432`.
19 changes: 19 additions & 0 deletions tests/ui/imports/shadow-glob-module-resolution-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// https://github.com/rust-lang/rust/issues/125013

mod a {
pub mod b {
pub mod c {
pub trait D {}
}
}
}

use a::*;

use e as b;
//~^ ERROR: unresolved import `e`
use b::c::D as e;
//~^ ERROR: cannot determine resolution for the import
//~| ERROR: cannot determine resolution for the import

fn main() { }
26 changes: 26 additions & 0 deletions tests/ui/imports/shadow-glob-module-resolution-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error: cannot determine resolution for the import
--> $DIR/shadow-glob-module-resolution-2.rs:15:5
|
LL | use b::c::D as e;
| ^^^^^^^^^^^^

error: cannot determine resolution for the import
--> $DIR/shadow-glob-module-resolution-2.rs:15:5
|
LL | use b::c::D as e;
| ^^^^^^^^^^^^
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error[E0432]: unresolved import `e`
--> $DIR/shadow-glob-module-resolution-2.rs:13:5
|
LL | use e as b;
| -^^^^^
| |
| no `e` in the root
| help: a similar name exists in the module: `a`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0432`.

0 comments on commit 69a8c13

Please sign in to comment.