Don't leak return value after panic in drop#78373
Conversation
|
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 7acf3f1b0d7330edcc908662e170dd6182074f8f with merge d610dce8ea41761b41e03160a39fca995dea0df5... |
|
☀️ Try build successful - checks-actions |
|
Queued d610dce8ea41761b41e03160a39fca995dea0df5 with parent b9a94c9, future comparison URL. |
|
Finished benchmarking try commit (d610dce8ea41761b41e03160a39fca995dea0df5): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
Since this adds code, I suspect it's always going to be a regression. I would ignore the incremental benchmarks here, which brings us down to roughly 5% regression on real world codebases. I think given this is a soundness fix (at least arguably) and certainly unintuitive behavior, we should land this (modulo review of the diff itself, which I have not done). |
davidtwco
left a comment
There was a problem hiding this comment.
Unsure if this is still only intended for checking perf, but LGTM; someone else should also take a look though.
|
r? @pnkfelix |
|
nominating for discussion at next T-compiler triage meeting. I am inclined to swallow the compiler performance regression, but I figure we will be better off talking about it as a team. Also, I think it will be best to land this early in the release cycle, i.e. soon after the next beta is cut.
|
|
@bors r+ rollup=never |
|
📌 Commit 7acf3f1b0d7330edcc908662e170dd6182074f8f has been approved by |
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
27e434f to
5e71b80
Compare
|
@bors r=pnkfelix |
0b6ab78 to
4fef391
Compare
|
@bors r=pnkfelix |
|
📌 Commit 4fef391 has been approved by |
|
🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened |
|
☀️ Test successful - checks-actions |
…ution-via-revert-of-pr-78373, r=matthewjasper
Revert 78373 ("dont leak return value after panic in drop")
Short term resolution for issue rust-lang#80949.
Reopen rust-lang#47949 after this lands.
(We plan to fine-tune PR rust-lang#78373 to not run into this problem.)
…ution-via-revert-of-pr-78373, r=matthewjasper
Revert 78373 ("dont leak return value after panic in drop")
Short term resolution for issue rust-lang#80949.
Reopen rust-lang#47949 after this lands.
(We plan to fine-tune PR rust-lang#78373 to not run into this problem.)
Fix leaks from panics in destructors Resurrects rust-lang#78373. This avoids the problem with rust-lang#80949 by not unscheduling drops of function arguments until after the call (so they still get a drop terminator on the function unwind path). Closes rust-lang#47949 r? `@lcnr`
Fix leaks from panics in destructors Resurrects rust-lang#78373. This avoids the problem with rust-lang#80949 by not unscheduling drops of function arguments until after the call (so they still get a drop terminator on the function unwind path). Closes rust-lang#47949 r? `@lcnr`
Fix leaks from panics in destructors Resurrects rust-lang#78373. This avoids the problem with rust-lang#80949 by not unscheduling drops of function arguments until after the call (so they still get a drop terminator on the function unwind path). Closes rust-lang#47949 r? `@lcnr`
Fix leaks from panics in destructors Resurrects rust-lang#78373. This avoids the problem with rust-lang#80949 by not unscheduling drops of function arguments until after the call (so they still get a drop terminator on the function unwind path). Closes rust-lang#47949 r? `@lcnr`
Fix leaks from panics in destructors Resurrects rust-lang#78373. This avoids the problem with rust-lang#80949 by not unscheduling drops of function arguments until after the call (so they still get a drop terminator on the function unwind path). Closes rust-lang#47949 r? `@lcnr`
Closes #47949