Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(resolve): prevent infinite loop when glob-import self #110264

Closed
wants to merge 1 commit into from
Closed

fix(resolve): prevent infinite loop when glob-import self #110264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions compiler/rustc_resolve/src/ident.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Some(_) => continue,
None => return Err((Undetermined, Weak::Yes)),
};

if ptr::eq(module, glob_import.parent_scope.module) {
// do not glob-import a module into itself
continue;
}

let tmp_parent_scope;
let (mut adjusted_parent_scope, mut ident) =
(parent_scope, ident.normalize_to_macros_2_0());
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/underscore-imports/issue-110164.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use self::*;
//~^ ERROR unresolved import `self::*`
use crate::*;
//~^ ERROR unresolved import `crate::*`
use _::a;
//~^ ERROR expected identifier, found reserved identifier `_`
//~| ERROR unresolved import `_`
use _::*;
//~^ ERROR expected identifier, found reserved identifier `_`
//~| ERROR unresolved import `_`

fn main() {
use _::a;
//~^ ERROR expected identifier, found reserved identifier `_`
//~| ERROR unresolved import `_`
use _::*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does use _::* trigger the "glob-import a module into itself" in the first place?
I'd expect _ to resolve into Res::Err here.

Is it possible to reproduce the ICE without _::*, just with regular well-formed imports?

Copy link
Contributor Author

@bvanjoi bvanjoi Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to reproduce the ICE without _::*, just with regular well-formed imports?

Will not occur, other cases will hit map_err

I'd expect _ to resolve into Res::Err here.

When executing new_key, special processing is performed for _ to ensure that _ each time the key is not the same, it is intended to solve the case of xxx as _

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, fascinating.
The underscore_disambiguator += 1 should only apply when we define names, not when we are retrieving already added definitions.

Copy link
Contributor

@petrochenkov petrochenkov Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding BindingKey::new method that will be used everywhere by default, and rename new_key to new_disambiguated_key and use it only when adding use foo as _ imports to a module (in fn add_import I guess, but I'm not sure).

Note that if new_key is called for a single import twice in different contexts, then the resulting disambiguators are not consistent with each other, and it's something that is suspicious at least.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is independent from glob imports and we should do it first.

The glob change also looks reasonable, maybe it fixes some other actual issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I will open a new PR to rewrite new_key, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, new PR.

//~^ ERROR expected identifier, found reserved identifier `_`
//~| ERROR unresolved import `_`
}
71 changes: 71 additions & 0 deletions tests/ui/underscore-imports/issue-110164.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
error: expected identifier, found reserved identifier `_`
--> $DIR/issue-110164.rs:5:5
|
LL | use _::a;
| ^ expected identifier, found reserved identifier

error: expected identifier, found reserved identifier `_`
--> $DIR/issue-110164.rs:8:5
|
LL | use _::*;
| ^ expected identifier, found reserved identifier

error: expected identifier, found reserved identifier `_`
--> $DIR/issue-110164.rs:13:9
|
LL | use _::a;
| ^ expected identifier, found reserved identifier

error: expected identifier, found reserved identifier `_`
--> $DIR/issue-110164.rs:16:9
|
LL | use _::*;
| ^ expected identifier, found reserved identifier

error[E0432]: unresolved import `self::*`
--> $DIR/issue-110164.rs:1:5
|
LL | use self::*;
| ^^^^^^^ cannot glob-import a module into itself

error[E0432]: unresolved import `crate::*`
--> $DIR/issue-110164.rs:3:5
|
LL | use crate::*;
| ^^^^^^^^ cannot glob-import a module into itself

error[E0432]: unresolved import `_`
--> $DIR/issue-110164.rs:8:5
|
LL | use _::*;
| ^ maybe a missing crate `_`?
|
= help: consider adding `extern crate _` to use the `_` crate

error[E0432]: unresolved import `_`
--> $DIR/issue-110164.rs:5:5
|
LL | use _::a;
| ^ maybe a missing crate `_`?
|
= help: consider adding `extern crate _` to use the `_` crate

error[E0432]: unresolved import `_`
--> $DIR/issue-110164.rs:13:9
|
LL | use _::a;
| ^ maybe a missing crate `_`?
|
= help: consider adding `extern crate _` to use the `_` crate

error[E0432]: unresolved import `_`
--> $DIR/issue-110164.rs:16:9
|
LL | use _::*;
| ^ maybe a missing crate `_`?
|
= help: consider adding `extern crate _` to use the `_` crate

error: aborting due to 10 previous errors

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