Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

mir: drop temps outside-in by scheduling the drops inside-out. #33239

Merged
merged 1 commit into from
May 11, 2016
Merged

mir: drop temps outside-in by scheduling the drops inside-out. #33239

merged 1 commit into from
May 11, 2016

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Apr 27, 2016

It was backwards all along, but only noticeable with multiple drops in one rvalue scope. Fixes #32433.

@rust-highfive
Copy link
Contributor

r? @arielb1

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

@arielb1
Copy link
Contributor

arielb1 commented Apr 27, 2016

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 27, 2016

📌 Commit d569228 has been approved by arielb1

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 27, 2016
@Manishearth
Rollup merge of rust-lang#33239 - eddyb:mir-temp-drops, r=arielb1
651ae26 
mir: drop temps outside-in by scheduling the drops inside-out.

It was backwards all along, but only noticeable with multiple drops in one rvalue scope. Fixes rust-lang#32433.
@Manishearth Manishearth mentioned this pull request Apr 29, 2016
Merged
@Manishearth
Copy link
Member

http://buildbot.rust-lang.org/builders/auto-linux-64-opt-mir/builds/513/steps/test/logs/stdio

@eddyb
Copy link
Member Author

eddyb commented Apr 29, 2016

Ahh right, I forgot to check it locally as I had two other PRs to work on. And Travis doesn't help with MIR.

@eddyb
Copy link
Member Author

eddyb commented Apr 29, 2016

@bors r-

bors added a commit that referenced this pull request Apr 29, 2016
@bors
Auto merge of #33272 - Manishearth:rollup, r=Manishearth
e07e34c 
Rollup of 4 pull requests

- Successful merges: #33239, #33248, #33253, #33258
- Failed merges:
@eddyb
Copy link
Member Author

eddyb commented May 7, 2016

I went back and checked my testcases from #32433. They're fixed, but the issue-23338-ensure-param-drop-order test still has a different case which MIR gets wrong, 6 needs to be dropped earlier, just after Make D(de_6, 7).

@eddyb eddyb mentioned this pull request May 7, 2016
Closed
@bors
Copy link
Collaborator

bors commented May 8, 2016

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

@eddyb eddyb force-pushed the mir-temp-drops branch from d569228 to 6974d30 Compare May 11, 2016 00:44
This was referenced May 11, 2016
Closed
Closed
@eddyb eddyb force-pushed the mir-temp-drops branch from 6974d30 to e940de6 Compare May 11, 2016 07:46
@eddyb
Copy link
Member Author

eddyb commented May 11, 2016

@bors r=arielb1

@bors
Copy link
Collaborator

bors commented May 11, 2016

📌 Commit e940de6 has been approved by arielb1

@bors
Copy link
Collaborator

bors commented May 11, 2016

⌛ Testing commit e940de6 with merge c049541...

bors added a commit that referenced this pull request May 11, 2016
@bors
Auto merge of #33239 - eddyb:mir-temp-drops, r=arielb1
c049541 
mir: drop temps outside-in by scheduling the drops inside-out.

It was backwards all along, but only noticeable with multiple drops in one rvalue scope. Fixes #32433.
@bors
Copy link
Collaborator

bors commented May 11, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-32cross-opt, auto-linux-64-cargotest, auto-linux-64-cross-armhf, auto-linux-64-cross-armsf, auto-linux-64-cross-freebsd, auto-linux-64-cross-netbsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild

@bors bors merged commit e940de6 into rust-lang:master May 11, 2016
@eddyb eddyb deleted the mir-temp-drops branch May 11, 2016 15:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@arielb1 arielb1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eddyb @rust-highfive @arielb1 @bors @Manishearth