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.

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

ICE when use-ing enum variant after glob-importing same-named enum #62767

Closed
oberien opened this issue Jul 17, 2019 · 6 comments · Fixed by #70236
Closed

ICE when use-ing enum variant after glob-importing same-named enum #62767

oberien opened this issue Jul 17, 2019 · 6 comments · Fixed by #70236
Labels
A-resolve Area: Name/path resolution done by `rustc_resolve` specifically C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oberien
Copy link
Contributor

oberien commented Jul 17, 2019

playpen

Reproduction code:

mod foo {
    pub enum Foo {
        Foo(i32),
    }
}
use foo::*;
use Foo::Foo;
thread 'rustc' panicked at 'assertion failed: directive.imported_module.get().is_none()', src/librustc_resolve/resolve_imports.rs:943:21
note: rustc 1.36.0 (a53f9df32 2019-07-03) running on x86_64-unknown-linux-gnu
note: compiler flags: -C codegen-units=1 -C debuginfo=2 --crate-type lib
note: some of the compiler flags provided by cargo are hidden

If I re-export pub use Foo::* in the module, it works as expected, having enum Foo in the type-namespace and variant Foo in the value-namespace (playpen).

@jonas-schievink jonas-schievink added A-resolve Area: Name/path resolution done by `rustc_resolve` specifically C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated labels Jul 17, 2019
@petrochenkov petrochenkov self-assigned this Jul 17, 2019
@nikomatsakis nikomatsakis added P-medium Medium priority and removed I-nominated labels Jul 25, 2019
@nikomatsakis
Copy link
Contributor

Compiler team check-in:

I'm marking this as P-medium for now as it is not classified as a regression, although it seems like a fairly major problem. @petrochenkov appears to be on it, though.

@nikomatsakis
Copy link
Contributor

Feel free to add the I-nominated label if you think it should be P-high

@estebank
Copy link
Contributor

The assertion was added late last year.

@petrochenkov could you take a look at this to see if it is valid for the invariant being checked not to hold in this case? If I comment out the assert the output is reasonable:

error[E0432]: unresolved import `Foo`
 --> file7.rs:8:5
  |
8 | use Foo::Foo;
  |     ^^^ `Foo` is a variant, not a module

warning: unused import: `foo::*`
 --> file7.rs:7:5
  |
7 | use foo::*;
  |     ^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0432, E0601.
For more information about an error, try `rustc --explain E0432`.

@petrochenkov
Copy link
Contributor

I'll see what happens (that's why I self-assigned).
The assert is there to catch the cases that resolve logic assumes to be impossible.
It may work ok if the assert is removed, but it may also mean some deeper issues, this needs a more detailed look.

@oberien
Copy link
Contributor Author

oberien commented Jul 25, 2019

The above error message without the assert looks a lot like #62768 and #62769.

@petrochenkov
Copy link
Contributor

but it may also mean some deeper issues

...which was exactly the case here!
Thank you for reporting.

Fixed in #70236.

@petrochenkov petrochenkov removed their assignment Mar 21, 2020
Centril added a commit to Centril/rust that referenced this issue Mar 23, 2020
…-morse

resolve: Avoid "self-confirming" import resolutions in one more case

So the idea behind "blacklisted bindings" is that we must ignore some name definitions during resolution because otherwise they cause infinite cycles.
E.g. import
```rust
use my_crate;
```
would refer to itself (on 2018 edition) without this blacklisting, because `use my_crate;` is the first name in scope when we are resolving `my_crate` here.

In this PR we are doing this blacklisting for the case
```rust
use same::same;
```
, namely blacklisting the second `same` when resolving the first `same`.
This was previously forgotten.

Fixes rust-lang#62767
@bors bors closed this as completed in 08dfd13 Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name/path resolution done by `rustc_resolve` specifically C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants