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

Always unwind through a Resume and other fixes #46320

Merged
merged 7 commits into from
Dec 3, 2017
Merged

Always unwind through a Resume and other fixes #46320

merged 7 commits into from
Dec 3, 2017

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Nov 27, 2017 •
edited
Loading

Should fix most of the small MIR borrowck issues.

r? @nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 28, 2017

☔ The latest upstream changes (presumably #46142) made this pull request unmergeable. Please resolve the merge conflicts.

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 28, 2017
@arielb1 arielb1 changed the title [WIP] Always unwind through a Resume and other fixes Always unwind through a Resume and other fixes Nov 29, 2017
@arielb1
Copy link
Contributor Author

arielb1 commented Nov 29, 2017

Ready for review

@bors
Copy link
Contributor

bors commented Nov 30, 2017

☔ The latest upstream changes (presumably #46041) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis
nikomatsakis approved these changes Nov 30, 2017
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Found the tests after all.

src/librustc_mir/borrow_check.rs Outdated
PrefixSet::Shallow
};

self.prefixes(lvalue, prefix_set).any(|prefix| prefix == root_lvalue)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tests?

@nikomatsakis
Copy link
Contributor

r=me post rebase

@arielb1
Copy link
Contributor Author

arielb1 commented Nov 30, 2017

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 30, 2017

📌 Commit 5da76cc has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 1, 2017

☔ The latest upstream changes (presumably #46425) made this pull request unmergeable. Please resolve the merge conflicts.

@arielb1
fix handling of borrows at a function's end
af8c233
@arielb1
initialize the destination in unit statements
a6e24fc 
Fixes rust-lang#46159.
@arielb1
don't track borrows for empty regions
25416c7 
Region inference can create borrows for an empty region if the borrow is
dead. In that case, there's no reason to track the borrow, but because
there's no such thing as an EndRegion(ReEmpty) these borrows used to live
for the entire function.

Fixes rust-lang#46161.
@arielb1
add a pass to remove no-op landing pads
485476c
@arielb1
funnel all unwind paths through a single Resume block
9dc3967 
This simplifies analysis and borrow-checking because liveness at the
resume point can always be simply propagated.

Later on, the "dead" Resumes are removed.
@arielb1
add and unignore tests
6594799
@arielb1
Copy link
Contributor Author

arielb1 commented Dec 3, 2017

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 3, 2017

📌 Commit 6594799 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 3, 2017

⌛ Testing commit 6594799 with merge 036469c...

bors added a commit that referenced this pull request Dec 3, 2017
@bors
Auto merge of #46320 - arielb1:always-resume, r=nikomatsakis
036469c 
Always unwind through a Resume and other fixes

Should fix most of the small MIR borrowck issues.

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Dec 3, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Dec 3, 2017

Two of the new tests failed with WASM.

[01:38:26] ---- [mir-opt] mir-opt/match_false_edges.rs stdout ----
[01:38:26] 	thread '[mir-opt] mir-opt/match_false_edges.rs' panicked at 'Did not find expected line, error: Mismatch in lines
[01:38:26] Currnt block:  bb9: { // binding1 and guard
[01:38:26] Expected Line: "        _6 = const guard() -> bb10;      // scope 2 at /checkout/src/test/mir-opt/match_false_edges.rs:25:20: 25:27"
[01:38:26] Actual Line: "     _6 = const guard() -> [return: bb10, unwind: bb1];"
...
[01:38:26] ---- [mir-opt] mir-opt/validate_2.rs stdout ----
[01:38:26] 	thread '[mir-opt] mir-opt/validate_2.rs' panicked at 'Did not find expected line, error: Mismatch in lines
[01:38:26] Currnt block: None
[01:38:26] Expected Line: "        drop(_1) -> bb2;                 // scope 0 at /checkout/src/test/mir-opt/validate_2.rs:16:2: 16:2"
[01:38:26] Actual Line: "        drop(_1) -> [return: bb2, unwind: bb3];"

@arielb1
Copy link
Contributor Author

arielb1 commented Dec 3, 2017

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 3, 2017

📌 Commit ff0b84d has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 3, 2017

⌛ Testing commit ff0b84d with merge 9da2112...

bors added a commit that referenced this pull request Dec 3, 2017
@bors
Auto merge of #46320 - arielb1:always-resume, r=nikomatsakis
9da2112 
Always unwind through a Resume and other fixes

Should fix most of the small MIR borrowck issues.

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Dec 3, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 9da2112 to master...

@bors bors merged commit ff0b84d into rust-lang:master Dec 3, 2017
@bors bors mentioned this pull request Dec 3, 2017
Merged
@matthewjasper matthewjasper mentioned this pull request Aug 3, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arielb1 @bors @nikomatsakis @kennytm