Skip to content

Commit

Permalink
Rollup merge of #112495 - bvanjoi:fix-109153, r=petrochenkov
Browse files Browse the repository at this point in the history
fix(resolve): update shadowed_glob more precision

- Fixes #109153
- Fixes #109962

## Why does it panic?

We use #109153 as an illustration.

The process of `resolve_imports` is:

| Iter | resolve     | resolution of **`(Mod(root), Ident(bar) in type ns)`** |
| -    | -           | -      |
| 0 | `use foo::*`   | `binding` -> foo::bar, `shallowed_glob` -> `None` |
| 1 | `use bar::bar` | `binding` -> foo::bar::bar, `shallowed_glob` -> foo::bar    |
| 2 | `use bar::*`   | `binding` -> foo::bar::bar, `shallowed_glob` -> foo::bar::bar::bar |

So during `finalize_import`, the `root::bar` in `use bar::bar` had been pointed to `foo::bar::bar::bar`, which is different from the `initial_module` valued of `foo::bar`, therefore, the panic had been triggered.

## Try to solve it

~I think #109153 should check-pass rather than throw an ambiguous error. Following this idea, there are two ways to solve this problem:~

~1. Give up the `initial_module` and update `import.imported_module` after each resolution update. However, I think this method may have too much impact.~
~2. Do not update the `shadowed_glob` when it is defined.~

~To be honest, I am not sure if this is the right way to solve this ICE. Perhaps there is a better resolution.~

Edit: we had made the `resolution.shadowed_glob` update more detailed.

r? `@petrochenkov`
  • Loading branch information
matthiaskrgr authored Jun 14, 2023
2 parents 6fc50da + f7330eb commit 98f6e96
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 2 deletions.
16 changes: 15 additions & 1 deletion compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
} else {
resolution.binding = Some(nonglob_binding);
}
resolution.shadowed_glob = Some(glob_binding);

if let Some(old_binding) = resolution.shadowed_glob {
assert!(old_binding.is_glob_import());
if glob_binding.res() != old_binding.res() {
resolution.shadowed_glob = Some(this.ambiguity(
AmbiguityKind::GlobVsGlob,
old_binding,
glob_binding,
));
} else if !old_binding.vis.is_at_least(binding.vis, this.tcx) {
resolution.shadowed_glob = Some(glob_binding);
}
} else {
resolution.shadowed_glob = Some(glob_binding);
}
}
(false, false) => {
return Err(old_binding);
Expand Down
4 changes: 3 additions & 1 deletion tests/ui/resolve/issue-105069.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,19 @@ error[E0659]: `V` is ambiguous
LL | use V;
| ^ ambiguous name
|
= note: ambiguous because of multiple potential import sources
= note: ambiguous because of multiple glob imports of a name in the same module
note: `V` could refer to the variant imported here
--> $DIR/issue-105069.rs:1:5
|
LL | use self::A::*;
| ^^^^^^^^^^
= help: consider adding an explicit import of `V` to disambiguate
note: `V` could also refer to the variant imported here
--> $DIR/issue-105069.rs:3:5
|
LL | use self::B::*;
| ^^^^^^^^^^
= help: consider adding an explicit import of `V` to disambiguate

error: aborting due to previous error

Expand Down
14 changes: 14 additions & 0 deletions tests/ui/resolve/issue-109153.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use foo::*;

mod foo {
pub mod bar {
pub mod bar {
pub mod bar {}
}
}
}

use bar::bar; //~ ERROR `bar` is ambiguous
use bar::*;

fn main() { }
23 changes: 23 additions & 0 deletions tests/ui/resolve/issue-109153.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error[E0659]: `bar` is ambiguous
--> $DIR/issue-109153.rs:11:5
|
LL | use bar::bar;
| ^^^ ambiguous name
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `bar` could refer to the module imported here
--> $DIR/issue-109153.rs:1:5
|
LL | use foo::*;
| ^^^^^^
= help: consider adding an explicit import of `bar` to disambiguate
note: `bar` could also refer to the module imported here
--> $DIR/issue-109153.rs:12:5
|
LL | use bar::*;
| ^^^^^^
= help: consider adding an explicit import of `bar` to disambiguate

error: aborting due to previous error

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

0 comments on commit 98f6e96

Please sign in to comment.