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

resolve: Fix one more ICE in import validation #57185

Merged
merged 1 commit into from
Dec 30, 2018

Conversation

petrochenkov
Copy link
Contributor

So if you have an unresolved import

mod m {
    use foo::bar;
}

error recovery will insert a special item with Def::Err definition into module m, so other things depending on bar won't produce extra errors.

The issue was that erroneous bar was overwriting legitimate bars coming from globs, e.g.

mod m {
    use baz::*; // imports real existing `bar`
    use foo::bar;
}

causing some unwanted diagnostics talking about "unresolved items", and producing inconsistent resolutions like #57015.
This PR stops overwriting real successful resolutions with Def::Errs.

Fixes #57015

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 29, 2018
@petrochenkov
Copy link
Contributor Author

Beta-nominating as a regression fix, the assert was introduced in stable Rust 1.31 (in #55884).

@petrochenkov petrochenkov added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 29, 2018
@petrochenkov
Copy link
Contributor Author

I'm glad I added all those asserts in #55884.
So many interesting corner cases were found (mostly in code with errors though).

@cramertj
Copy link
Member

r? @petrochenkov

@petrochenkov
Copy link
Contributor Author

@cramertj
This is my PR :)

r? @estebank

@estebank
Copy link
Contributor

r=me once travis' done

@cramertj
Copy link
Member

cramertj commented Dec 29, 2018

@petrochenkov lol, sorry XD -- in my defense it looked like sometihng you'd be qualified to review!

@Centril
Copy link
Contributor

Centril commented Dec 29, 2018

@bors r=estebank

@bors
Copy link
Contributor

bors commented Dec 29, 2018

📌 Commit ddb550a has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 29, 2018
@bors
Copy link
Contributor

bors commented Dec 30, 2018

⌛ Testing commit ddb550a with merge 171c1fc...

bors added a commit that referenced this pull request Dec 30, 2018
resolve: Fix one more ICE in import validation

So if you have an unresolved import
```rust
mod m {
    use foo::bar;
}
```
error recovery will insert a special item with `Def::Err` definition into module `m`, so other things depending on `bar` won't produce extra errors.

The issue was that erroneous `bar` was overwriting legitimate `bar`s coming from globs, e.g.
```rust
mod m {
    use baz::*; // imports real existing `bar`
    use foo::bar;
}
```
causing some unwanted diagnostics talking about "unresolved items", and producing inconsistent resolutions like #57015.
This PR stops overwriting real successful resolutions with `Def::Err`s.

Fixes #57015
@bors
Copy link
Contributor

bors commented Dec 30, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing 171c1fc to master...

@bors bors merged commit ddb550a into rust-lang:master Dec 30, 2018
@bors bors mentioned this pull request Dec 30, 2018
@nagisa nagisa added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jan 3, 2019
@pietroalbini pietroalbini added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 3, 2019
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 3, 2019
bors added a commit that referenced this pull request Jan 4, 2019
[beta] Rollup backports

Cherry-picked:

* #57053: Fix alignment for array indexing
* #57181: resolve: Fix another ICE in import validation
* #57185: resolve: Fix one more ICE in import validation
* #57282: Wf-check the output type of a function in MIR-typeck
* #55318: Ensure that Rustdoc discovers all necessary auto trait bounds
* #56838: Call poly_project_and_unify_type on types that contain inference types

Rolled up:

* #57300: [beta] Update RLS to include 100% CPU on hover bugfix
* #57301: beta: bootstrap from latest stable (1.31.1)
* #57292: [BETA] Update cargo

r? @ghost
@petrochenkov petrochenkov deleted the impice4 branch June 5, 2019 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

attempting to address path clarity warnings for 2018 edition with StructOpt macro led to ICE
8 participants