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

Jump to bottom

Fix #78192 #78247

Merged
merged 1 commit into from
Oct 26, 2020
Merged

Fix #78192 #78247

merged 1 commit into from
Oct 26, 2020

Conversation

simonvandel
Copy link
Contributor

@simonvandel simonvandel commented Oct 22, 2020 •
edited
Loading

Check which places are marked dead.

Fixes #78192

@rust-highfive
Copy link
Collaborator

r? @estebank

(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 Oct 22, 2020
@estebank
Copy link
Contributor

r? @oli-obk who's more familiar with the nuance here.

@rust-highfive rust-highfive assigned oli-obk and unassigned estebank Oct 23, 2020
@estebank estebank added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 23, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Oct 25, 2020

While this does fix the bug, I'm not very happy with making the logic in this optimization more complex. I still think we should make this optimization work without backtracking, in which case it would be trivial to bail out when encountering StorageDead

@simonvandel
Copy link
Contributor Author

I have a branch locally with this optimization without backtracking. I can't guarantee when I will have time to finish it though. Should we create a issue to track the no-backtracking implementation, and move forward with the current pr?

@oli-obk
Copy link
Contributor

oli-obk commented Oct 25, 2020

Yea, an issue tracking it would be great. Merging this is ok imo with a fix somewhere on the horizon.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 25, 2020

📌 Commit 57d01a9 has been approved by oli-obk

@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 Oct 25, 2020
@simonvandel
Copy link
Contributor Author

Created issue #78368 to track the work on a non-backtracking implementation

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 25, 2020
@Dylan-DPC
Rollup merge of rust-lang#78247 - simonvandel:fix-78192, r=oli-obk
9fe1de1 
Fix rust-lang#78192

Check which places are marked dead.

Fixes rust-lang#78192
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Oct 25, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 26, 2020
@Dylan-DPC
Rollup merge of rust-lang#78247 - simonvandel:fix-78192, r=oli-obk
b829cef 
Fix rust-lang#78192

Check which places are marked dead.

Fixes rust-lang#78192
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Oct 26, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 26, 2020
@Dylan-DPC
Rollup merge of rust-lang#78247 - simonvandel:fix-78192, r=oli-obk
40d4db3 
Fix rust-lang#78192

Check which places are marked dead.

Fixes rust-lang#78192
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Oct 26, 2020
Closed
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Oct 26, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 26, 2020
@bors
Auto merge of rust-lang#78387 - Dylan-DPC:rollup-ch0st6z, r=Dylan-DPC
1cd97ca 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#74477 (`#[deny(unsafe_op_in_unsafe_fn)]` in sys/wasm)
 - rust-lang#77836 (transmute_copy: explain that alignment is handled correctly)
 - rust-lang#78126 (Properly define va_arg and va_list for aarch64-apple-darwin)
 - rust-lang#78137 (Initialize tracing subscriber in compiletest tool)
 - rust-lang#78161 (Add issue template link to IRLO)
 - rust-lang#78214 (Tweak match arm semicolon removal suggestion to account for futures)
 - rust-lang#78247 (Fix rust-lang#78192)
 - rust-lang#78252 (Add codegen test for rust-lang#45964)
 - rust-lang#78268 (Do not try to report on closures to avoid ICE)
 - rust-lang#78295 (Add some regression tests)

Failed merges:

r? `@ghost`
@bors bors merged commit 8646c2a into rust-lang:master Oct 26, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 26, 2020
@jonas-schievink
Copy link
Contributor

#78195 is already beta-accepted, so removing the backport nomination from this PR. It would reintroduce the unsoundness as mentioned here: #78192 (comment).

@jonas-schievink jonas-schievink removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
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
Milestone
1.49.0
Development

Successfully merging this pull request may close these issues.

InstCombine introduces an incorrect use of a local after its storage has ended
7 participants
@simonvandel @rust-highfive @estebank @oli-obk @bors @jonas-schievink @rustbot