Reset the proxy when the destructor of the underlying pointer throws - #82
Merged
Merged
Conversation
proxy::destroy() invoked destruction through an lvalue-qualified overload, so the meta_resetting_guard that invoke_impl applies to consuming overloads never ran. When the destructor of the underlying pointer threw, the metadata still pointed at the destroyed object, so has_value() stayed true and the next destruction ran on a dead object. Every path that discards a value reached this: reset(), operator=(nullptr), both branches of the copy assignment operator, the move assignment operator, operator=(P&&) and both emplace overloads. Destruction is a consuming operation like relocation, so give it the same shape. The destroy meta now uses an rvalue-qualified overload, which makes erased_context destroy the pointer through destroying_guard and makes invoke_impl clear the metadata on both the normal and the exceptional path. destroy_dispatch keeps only its tag role and its call operator becomes a no-op, because the destruction it used to perform is what the rvalue machinery already does. The added reset is dead on the non-throwing path and the optimizer removes it. At -O2 the disassembly of ~proxy, reset and the move assignment operator is unchanged for a facade whose destructibility is nothrow. LifetimeTracker gains ThrowingDestructionSession, a Session whose destructor throws, which is the first pointer in the suite with a potentially throwing destructor and the first use of a facade whose destructibility is nontrivial.
mingxwa
marked this pull request as draft
September 8, 2026 02:02
mingxwa
force-pushed
the
user/mingxwa/fix-destruction
branch
from
September 8, 2026 02:05
249c6fd to
2a0012b
Compare
mingxwa
force-pushed
the
user/mingxwa/fix-destruction
branch
from
September 9, 2026 02:07
2a0012b to
7ec78d0
Compare
mingxwa
marked this pull request as ready for review
September 9, 2026 02:26
tian-lt-personal
approved these changes
Sep 9, 2026
Collaborator
Hmm, pretty clever🫤 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
has_value()trueover a destroyed object, so the next destruction ran on a dead object. Every path that discards a value was affected, namelyreset(),operator=(nullptr), both branches of copy assignment, move assignment,operator=(P&&)and bothemplaceoverloads.destroy_dispatchto a no-op dispatch.operator=(P&&)and copy assignment.std::destroy_atbecause Microsoft STL cleverly strengthened it even though the standard didn't say that.Codegen is unchanged where destructibility is
nothrow.