Skip to content

add StorageDead handling #45936

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 3 commits into from
Nov 15, 2017
Merged

Conversation

mikhail-m1
Copy link
Contributor

fix #45642
r? @arielb1

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 12, 2017
// here, in dataflow vector
zero_to_one(sets.gen_set.words_mut(), *move_index);
match stmt.kind {
// skip move out for StorageDead
Copy link
Contributor

@arielb1 arielb1 Nov 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you comment here why we are skipping the move out for StorageDead?

// this analysis only tries to find moves explicitly
// written by the user, so we ignore the move-outs
// created by `StorageDead` and at the beginning
// of a function.

Also, it might be a better idea to handle the move outs for both StorageDead and the implicit move-out at the beginning of a function. Your call.

Copy link
Contributor

@arielb1 arielb1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with a better comment for StorageDead and the double move-out removed.

@@ -231,8 +231,13 @@ pub(crate) fn drop_flag_effects_for_location<'a, 'gcx, 'tcx, F>(
}
}
}
mir::StatementKind::StorageDead(local) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed - we already iterate over move-outs the statement before.

@arielb1
Copy link
Contributor

arielb1 commented Nov 13, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 13, 2017

📌 Commit 5a501cc has been approved by arielb1

@kennytm kennytm 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 Nov 13, 2017
@bors
Copy link
Collaborator

bors commented Nov 15, 2017

⌛ Testing commit 5a501cc8802cb3119e9180b23ff019d13b5997b8 with merge b8123036d2957f059bac389ebbffac65eee0a936...

@bors
Copy link
Collaborator

bors commented Nov 15, 2017

💔 Test failed - status-travis

@mikhail-m1
Copy link
Contributor Author

rebaseing

@mikhail-m1 mikhail-m1 force-pushed the mir-borrowck-storage-dead branch from 5a501cc to 9e35fd2 Compare November 15, 2017 09:31
@arielb1
Copy link
Contributor

arielb1 commented Nov 15, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 15, 2017

📌 Commit 9e35fd2 has been approved by arielb1

@bors
Copy link
Collaborator

bors commented Nov 15, 2017

⌛ Testing commit 9e35fd2 with merge 88a28ff...

bors added a commit that referenced this pull request Nov 15, 2017
@bors
Copy link
Collaborator

bors commented Nov 15, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 88a28ff to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MIR borrowck: variables are still initialized after their storage ends
4 participants