Skip to content

Extend rustc::middle::dataflow to allow filtering kills from flow-exits #24330

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

Merged
merged 2 commits into from
Apr 15, 2015

Conversation

pnkfelix
Copy link
Member

Extend rustc::middle::dataflow to allow filtering kills from flow-exits.

Fix borrowck analysis so that it will not treat a break that pops through an assignment

x = { ... break; ... }

as a kill of the "moved-out" bit for x.

Fix #24267.

[breaking-change], but really, its only breaking code that was already buggy.

@rust-highfive
Copy link
Contributor

r? @Aatch

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

@pnkfelix
Copy link
Member Author

oh I still need to add a regression test; I will do that shortly.

@pnkfelix
Copy link
Member Author

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned Aatch Apr 11, 2015
@pnkfelix
Copy link
Member Author

@nikomatsakis (also, I am a little worried about the inefficiency introduced by this filter. I briefly searched for a more efficient encoding, but this was the easiest thing to put in quickly. We can discuss more during the week.)

@pnkfelix
Copy link
Member Author

(i have now confirmed that this does pass make check)

@pnkfelix
Copy link
Member Author

(maybe I do indeed need to mark this as a [breaking-change], with a clear stmt that its fixing a soundness bug?)

@nikomatsakis
Copy link
Contributor

per our discussion, it would be nicer to distinguish "lexical" kills from "action" kills, and then only kill "lexical" kills when breaking out

@pnkfelix pnkfelix force-pushed the issue-24267 branch 2 times, most recently from f447d8a to 1116ebe Compare April 14, 2015 22:03
@pnkfelix
Copy link
Member Author

(this is ready for a re-review)

@nikomatsakis
Copy link
Contributor

r+ modulo nits about dead code from before -- looks great!

Revise rustc::middle::dataflow: one must select kill-kind when calling
add_kill. The current kill-kinds are (1.) kills associated with
ends-of-scopes and (2.) kills associated with the actual action of the
expression/pattern.

Then, use this to fix borrowck analysis so that it will not treat a
break that pops through an assignment `x = { ... break; ... }` as a
kill of the "moved-out" bit for `x`.

Fix rust-lang#24267.

(incorporated review feedback.)
@pnkfelix
Copy link
Member Author

@bors r=nikomatsakis 77bf827

@bors
Copy link
Collaborator

bors commented Apr 15, 2015

⌛ Testing commit 77bf827 with merge e3eaf2c...

@bors
Copy link
Collaborator

bors commented Apr 15, 2015

💔 Test failed - auto-linux-64-nopt-t

@alexcrichton
Copy link
Member

@bors: retry

On Wed, Apr 15, 2015 at 1:11 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-linux-64-nopt-t
http://buildbot.rust-lang.org/builders/auto-linux-64-nopt-t/builds/4478


Reply to this email directly or view it on GitHub
#24330 (comment).

@pnkfelix
Copy link
Member Author

@alexcrichton any info or idea what was up with that failure? Residual files on the bot from another PR or something?

@alexcrichton
Copy link
Member

Perhaps, I'm not quite sure...

@bors
Copy link
Collaborator

bors commented Apr 15, 2015

⌛ Testing commit 77bf827 with merge 07f807d...

bors added a commit that referenced this pull request Apr 15, 2015
Extend rustc::middle::dataflow to allow filtering kills from flow-exits.

Fix borrowck analysis so that it will not treat a break that pops through an assignment
```rust
x = { ... break; ... }
```
as a kill of the "moved-out" bit for `x`.

Fix #24267.

[breaking-change], but really, its only breaking code that was already buggy.
@bors
Copy link
Collaborator

bors commented Apr 15, 2015

@bors bors merged commit 77bf827 into rust-lang:master Apr 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

break stmt confuses intialization-knowledge of borrow checker
6 participants