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

Mir borrowck does not generate lifetime variables for 'static lifetimes during opaque type resolution #87483

Merged
merged 1 commit into from
Jul 30, 2021

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jul 26, 2021

Fixes #87455

This situation was unreachable before #87287 as we used to just grab the resolved opaque type from typeck and replaced all regions with new inference vars. After #87287 we let the InferCx in mir borrowck figure out the opaque type all by itself (which it already did before, but it only used the result to sanity check with the typeck result).

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(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 Jul 26, 2021
@rust-log-analyzer

This comment has been minimized.

@teor2345
Copy link
Contributor

We're seeing this ICE in beta 1.55, which was just released on 28 July:

error: internal compiler error: unexpected concrete region in borrowck: ReStatic
...
rustc 1.55.0-beta.1 (739f8f0 2021-07-28) running on x86_64-unknown-linux-gnu

https://github.com/ZcashFoundation/zebra/pull/2525/checks?check_run_id=3187561535#step:10:1310

Let me know if you'd like a full bug report or an attempt at minimising the code.

@lqd
Copy link
Member

lqd commented Jul 29, 2021

@teor2345 do you know if this PR fixes the ICE you're seeing ?

I would assume so, but it would be nice to have confirmation. I'll ask CI for build artifacts and when the try build completes, you'll be able to use rustup-toolchain-install-master to install the built artifacts and check.

@bors try

@oli-obk the fix looks good to me, you can r=me if you want or wait for david's review. We may want to beta-nominate this PR since #87287 seems to have just slipped into beta.

@bors
Copy link
Contributor

bors commented Jul 29, 2021

⌛ Trying commit 2953a2f with merge 848106bdde5cd5ef50b0d2f597a45f2aee1eeed1...

@bors
Copy link
Contributor

bors commented Jul 29, 2021

☀️ Try build successful - checks-actions
Build commit: 848106bdde5cd5ef50b0d2f597a45f2aee1eeed1 (848106bdde5cd5ef50b0d2f597a45f2aee1eeed1)

@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 29, 2021

@bors r=lqd

@bors
Copy link
Contributor

bors commented Jul 29, 2021

📌 Commit 2953a2f has been approved by lqd

@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 Jul 29, 2021
@oli-obk oli-obk added beta-accepted Accepted for backporting to the compiler in the beta channel. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jul 29, 2021
@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 29, 2021

beta nominating and auto-accepting, it will be shown to the compiler team next week

@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 29, 2021
@lqd
Copy link
Member

lqd commented Jul 29, 2021

r? @lqd

(just so that this doesn't show on david's todo list)

@rust-highfive rust-highfive assigned lqd and unassigned davidtwco Jul 29, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 29, 2021
Mir borrowck does not generate lifetime variables for 'static lifetimes during opaque type resolution

Fixes rust-lang#87455

This situation was unreachable before rust-lang#87287 as we used to just grab the resolved opaque type from typeck and replaced all regions with new inference vars. After rust-lang#87287 we let the `InferCx` in mir borrowck figure out the opaque type all by itself (which it already did before, but it only used the result to sanity check with the typeck result).
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this pull request Jul 29, 2021
Mir borrowck does not generate lifetime variables for 'static lifetimes during opaque type resolution

Fixes rust-lang#87455

This situation was unreachable before rust-lang#87287 as we used to just grab the resolved opaque type from typeck and replaced all regions with new inference vars. After rust-lang#87287 we let the `InferCx` in mir borrowck figure out the opaque type all by itself (which it already did before, but it only used the result to sanity check with the typeck result).
@teor2345
Copy link
Contributor

@teor2345 do you know if this PR fixes the ICE you're seeing ?

I would assume so, but it would be nice to have confirmation. I'll ask CI for build artifacts and when the try build completes, you'll be able to use rustup-toolchain-install-master to install the built artifacts and check.

@lqd this PR fixes the ICE we're seeing.

cargo +848106bdde5cd5ef50b0d2f597a45f2aee1eeed1 test built and ran successfully, using rustc 1.56.0-nightly (848106bdd 2021-07-29).

I also checked that the same test build fails on rustc 1.56.0-nightly (b70888601 2021-07-28), as expected.

@bors
Copy link
Contributor

bors commented Jul 30, 2021

⌛ Testing commit 2953a2f with merge f739552...

@bors
Copy link
Contributor

bors commented Jul 30, 2021

☀️ Test successful - checks-actions
Approved by: lqd
Pushing f739552 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 30, 2021
@bors bors merged commit f739552 into rust-lang:master Jul 30, 2021
@rustbot rustbot added this to the 1.56.0 milestone Jul 30, 2021
@oli-obk oli-obk deleted the tait_ice branch July 30, 2021 11:46
@pnkfelix pnkfelix mentioned this pull request Aug 20, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 21, 2021
…acrum

Backport PR 87483

Backport of PR rust-lang#87483: "Mir borrowck does not generate lifetime variables for 'static lifetimes during opaque type resolution"

Fix rust-lang#87455: "ICE: unexpected concrete region in borrowck: ReStatic"
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 27, 2021
@Mark-Simulacrum
Copy link
Member

Backported in #88190

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. merged-by-bors This PR was explicitly merged by bors. 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.

ICE: unexpected concrete region in borrowck: ReStatic
10 participants