From 38b01b6bc007cb331f16f3637e2f28d2eb276972 Mon Sep 17 00:00:00 2001 From: Mingxin Wang Date: Mon, 7 Sep 2026 15:43:52 -0400 Subject: [PATCH 1/2] Fix assignment for a facade that forbids relocation proxy::operator=(const proxy&) commits a throwing copy through a temporary, which needs a move assignment. A facade that forbids relocation has none, so the expression re-selected the copy assignment itself and recursed until the stack overflowed. proxy::operator=(P&&) reaches the same expression and inherited the crash, and failed to compile outright for a facade that is not copyable either. Stage the temporary only where an assignment exists to commit it with, and destroy the contained value before constructing the new one otherwise. Assignment then works for every facade, at the cost of leaving *this without a value when the construction throws, which is the best a facade that can neither relocate nor copy without throwing can offer. Take the staged form whenever the commit cannot throw, which is copyability >= nothrow as much as relocatability >= nontrivial. A facade with nothrow copyability keeps its old value when the construction throws, as one with a trivial copy assignment already did. --- include/proxy/v4/detail/core.h | 11 +++++- tests/proxy_lifetime_tests.cpp | 72 ++++++++++++++++++++++++++++++++++ tests/utils.h | 32 +++++++++++++++ 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/include/proxy/v4/detail/core.h b/include/proxy/v4/detail/core.h index 4262904b..daf79a43 100644 --- a/include/proxy/v4/detail/core.h +++ b/include/proxy/v4/detail/core.h @@ -1129,8 +1129,11 @@ class proxy : public detail::facade_traits::direct_accessor, if constexpr (F::copyability == constraint_level::nothrow) { destroy(); initialize(rhs); - } else { + } else if constexpr (F::relocatability >= constraint_level::nontrivial) { *this = proxy{rhs}; + } else { + reset(); + initialize(rhs); } } return *this; @@ -1161,8 +1164,12 @@ class proxy : public detail::facade_traits::direct_accessor, if constexpr (std::is_nothrow_constructible_v, P>) { destroy(); initialize>(std::forward

(ptr)); - } else { + } else if constexpr (F::relocatability >= constraint_level::nontrivial || + F::copyability >= constraint_level::nothrow) { *this = proxy{std::forward

(ptr)}; + } else { + reset(); + initialize>(std::forward

(ptr)); } return *this; } diff --git a/tests/proxy_lifetime_tests.cpp b/tests/proxy_lifetime_tests.cpp index 701e2ce4..75ca89e2 100644 --- a/tests/proxy_lifetime_tests.cpp +++ b/tests/proxy_lifetime_tests.cpp @@ -1121,6 +1121,78 @@ TEST(ProxyLifetimeTests, TestSwap_Trivial) { ASSERT_EQ(ToString(*p2), "123"); } +TEST(ProxyLifetimeTests, Test_CopyAssignment_NoRelocation) { + struct Pinned : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::build {}; + int v1 = 111, v2 = 222; + pro::proxy p1{utils::ThrowingCopyPtr{&v1}}; + pro::proxy p2{utils::ThrowingCopyPtr{&v2}}; + p1 = p2; + ASSERT_EQ(ToString(*p1), "222"); + ASSERT_EQ(ToString(*p2), "222"); + p1 = utils::ThrowingCopyPtr{&v1}; + ASSERT_EQ(ToString(*p1), "111"); +} + +TEST(ProxyLifetimeTests, Test_PointerAssignment_NoRelocationNoCopy) { + struct Pinned : pro::facade_builder // + ::add_convention // + ::support_relocation // + ::build {}; + int v1 = 111, v2 = 222; + pro::proxy p{utils::ThrowingCopyPtr{&v1}}; + p = utils::ThrowingCopyPtr{&v2}; + ASSERT_EQ(ToString(*p), "222"); +} + +TEST(ProxyLifetimeTests, Test_PointerAssignment_ThrowingInitialization) { + struct Movable : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::build {}; + struct TriviallyCopyable + : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::build {}; + struct NothrowCopyable : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::build {}; + int v1 = 111, v2 = 222; + + pro::proxy p1{std::in_place_type>, &v1}; + ASSERT_THROW(p1 = utils::ThrowOnMovePtr{&v2}, + utils::ConstructionFailure); + ASSERT_TRUE(p1.has_value()); + ASSERT_EQ(ToString(*p1), "111"); + + pro::proxy p2{ + std::in_place_type>, &v1}; + ASSERT_THROW(p2 = utils::ThrowOnMovePtr{&v2}, + utils::ConstructionFailure); + ASSERT_TRUE(p2.has_value()); + ASSERT_EQ(ToString(*p2), "111"); + + pro::proxy p3{std::in_place_type>, + &v1}; + ASSERT_THROW(p3 = utils::ThrowOnMovePtr{&v2}, + utils::ConstructionFailure); + ASSERT_TRUE(p3.has_value()); + ASSERT_EQ(ToString(*p3), "111"); +} + TEST(ProxyLifetimeTests, Test_DirectConvension_Lvalue) { utils::LifetimeTracker tracker; std::vector expected_ops; diff --git a/tests/utils.h b/tests/utils.h index ab53e639..33bf3025 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -92,6 +92,38 @@ class LifetimeTracker { std::vector ops_; }; +template +class ThrowingCopyPtr { +public: + using element_type = T; + + explicit ThrowingCopyPtr(T* ptr) noexcept : ptr_(ptr) {} + ThrowingCopyPtr(const ThrowingCopyPtr& rhs) : ptr_(rhs.ptr_) {} + ThrowingCopyPtr& operator=(const ThrowingCopyPtr&) = default; + T& operator*() const noexcept { return *ptr_; } + +private: + T* ptr_; +}; + +template +class ThrowOnMovePtr { +public: + using element_type = T; + + explicit ThrowOnMovePtr(T* ptr) noexcept : ptr_(ptr) {} + ThrowOnMovePtr(const ThrowOnMovePtr&) = default; + ThrowOnMovePtr(ThrowOnMovePtr&& rhs) : ptr_(rhs.ptr_) { + if (ptr_ != nullptr) { + throw ConstructionFailure{LifetimeOperationType::kValueConstruction}; + } + } + T& operator*() const noexcept { return *ptr_; } + +private: + T* ptr_; +}; + namespace spec { using std::to_string; From 45486e4c4f257cb0f019c19a98e92835a9e33fda Mon Sep 17 00:00:00 2001 From: Mingxin Wang Date: Mon, 7 Sep 2026 15:44:16 -0400 Subject: [PATCH 2/2] Exchange values by copying when a facade forbids relocation proxy::swap relocates, so a facade that forbids relocation had no swap at all unless it was trivially copyable, even though std::swap already exchanged such proxies through the copy constructor and the copy assignment, both of which bind an rvalue. The member was missing for types the standard library already reports as swappable. Add an overload for exactly those facades, constrained on relocatability == none, that exchanges the values by copying. Splitting it from the relocating overload rather than branching inside one keeps both noexcept specifications honest: the relocating overload is untouched, and the new one is noexcept when the copies and the destructions are. An empty operand is exchanged with a single copy rather than through a temporary. A facade that can relocate keeps relocating, even where the relocation can throw and a copy could not. Exchanging by copy is what a facade with no other way gets, not a path taken from one that has one. Documented the constraint the hidden friend has always carried. --- docs/spec/proxy/friend_swap.md | 3 +- docs/spec/proxy/swap.md | 13 ++++ include/proxy/v4/detail/core.h | 22 ++++++ tests/proxy_lifetime_tests.cpp | 129 +++++++++++++++++++++++++++++++++ 4 files changed, 166 insertions(+), 1 deletion(-) diff --git a/docs/spec/proxy/friend_swap.md b/docs/spec/proxy/friend_swap.md index a1bf12a8..fdc8f5b2 100644 --- a/docs/spec/proxy/friend_swap.md +++ b/docs/spec/proxy/friend_swap.md @@ -1,7 +1,8 @@ # Function `swap` (`proxy`) ```cpp -friend void swap(proxy& lhs, proxy& rhs) noexcept(noexcept(lhs.swap(rhs))); +friend void swap(proxy& lhs, proxy& rhs) noexcept(noexcept(lhs.swap(rhs))) + requires(requires { lhs.swap(rhs); }); ``` Overloads the [std::swap](https://en.cppreference.com/w/cpp/algorithm/swap) algorithm for `proxy`. Exchanges the state of `lhs` with that of `rhs`. Effectively calls `lhs.swap(rhs)`. diff --git a/docs/spec/proxy/swap.md b/docs/spec/proxy/swap.md index 5765216e..a221b9c8 100644 --- a/docs/spec/proxy/swap.md +++ b/docs/spec/proxy/swap.md @@ -1,11 +1,24 @@ # `proxy::swap` ```cpp +// (1) void swap(proxy& rhs) noexcept(F::relocatability >= constraint_level::nothrow || F::copyability == constraint_level::trivial) requires(F::relocatability >= constraint_level::nontrivial || F::copyability == constraint_level::trivial); + +// (2) (since 5.0.0) +void swap(proxy& rhs) + noexcept(F::copyability >= constraint_level::nothrow && + F::destructibility >= constraint_level::nothrow) + requires(F::relocatability == constraint_level::none && + (F::copyability == constraint_level::nontrivial || + F::copyability == constraint_level::nothrow) && + F::destructibility >= constraint_level::nontrivial); ``` Exchanges the contained values of `*this` and `rhs`. + +- `(1)` Exchanges the values by relocation, or by exchanging the underlying storage when `F::relocatability == constraint_level::trivial` or `F::copyability == constraint_level::trivial` is `true`. If the relocation throws when `F::relocatability == constraint_level::nontrivial`, both operands can be left without a value. +- `(2)` Exchanges the values by copying, for a facade that forbids relocation. If a copy throws when `F::copyability == constraint_level::nontrivial`, one of the two operands can be left without a value. diff --git a/include/proxy/v4/detail/core.h b/include/proxy/v4/detail/core.h index daf79a43..ac12c085 100644 --- a/include/proxy/v4/detail/core.h +++ b/include/proxy/v4/detail/core.h @@ -1224,6 +1224,28 @@ class proxy : public detail::facade_traits::direct_accessor, } } } + void swap(proxy& rhs) noexcept(F::copyability >= constraint_level::nothrow && + F::destructibility >= + constraint_level::nothrow) + requires(F::relocatability == constraint_level::none && + (F::copyability == constraint_level::nontrivial || + F::copyability == constraint_level::nothrow) && + F::destructibility >= constraint_level::nontrivial) + { + if (meta_.has_value()) { + if (rhs.meta_.has_value()) { + proxy temp = *this; + *this = rhs; + rhs = temp; + } else { + rhs = *this; + reset(); + } + } else if (rhs.meta_.has_value()) { + *this = rhs; + rhs.reset(); + } + } template constexpr P& emplace(Args&&... args) noexcept( std::is_nothrow_constructible_v && diff --git a/tests/proxy_lifetime_tests.cpp b/tests/proxy_lifetime_tests.cpp index 75ca89e2..8b21a74a 100644 --- a/tests/proxy_lifetime_tests.cpp +++ b/tests/proxy_lifetime_tests.cpp @@ -1193,6 +1193,135 @@ TEST(ProxyLifetimeTests, Test_PointerAssignment_ThrowingInitialization) { ASSERT_EQ(ToString(*p3), "111"); } +TEST(ProxyLifetimeTests, TestSwap_NoRelocation) { + struct Pinned : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::build {}; + struct PinnedThrowingDestruction + : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::support_destruction // + ::build {}; + + int v1 = 111, v2 = 222; + pro::proxy p1 = &v1; + pro::proxy p2 = &v2; + static_assert(noexcept(p1.swap(p2))); + p1.swap(p2); + ASSERT_EQ(ToString(*p1), "222"); + ASSERT_EQ(ToString(*p2), "111"); + swap(p1, p2); + ASSERT_EQ(ToString(*p1), "111"); + ASSERT_EQ(ToString(*p2), "222"); + swap(p1, p1); + ASSERT_EQ(ToString(*p1), "111"); + + pro::proxy p3; + swap(p1, p3); + ASSERT_FALSE(p1.has_value()); + ASSERT_EQ(ToString(*p3), "111"); + swap(p1, p3); + ASSERT_EQ(ToString(*p1), "111"); + ASSERT_FALSE(p3.has_value()); + swap(p3, p3); + ASSERT_FALSE(p3.has_value()); + pro::proxy p4; + swap(p3, p4); + ASSERT_FALSE(p3.has_value()); + ASSERT_FALSE(p4.has_value()); + + pro::proxy r1 = &v1; + pro::proxy r2 = &v2; + static_assert(!noexcept(r1.swap(r2))); + swap(r1, r2); + ASSERT_EQ(ToString(*r1), "222"); + ASSERT_EQ(ToString(*r2), "111"); +} + +TEST(ProxyLifetimeTests, TestSwap_NoRelocation_ThrowingCopy) { + struct PinnedThrowingCopy + : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::build {}; + utils::LifetimeTracker tracker; + pro::proxy p1{ + std::in_place_type, &tracker}; + pro::proxy p2{ + std::in_place_type, &tracker}; + static_assert(!noexcept(p1.swap(p2))); + swap(p1, p2); + ASSERT_EQ(ToString(*p1), "Session 4"); + ASSERT_EQ(ToString(*p2), "Session 5"); + tracker.ThrowOnNextConstruction(); + ASSERT_THROW(swap(p1, p2), utils::ConstructionFailure); + ASSERT_EQ(ToString(*p1), "Session 4"); + ASSERT_EQ(ToString(*p2), "Session 5"); +} + +TEST(ProxyLifetimeTests, TestSwap_NoRelocation_Null) { + struct PinnedThrowingCopy + : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::build {}; + utils::LifetimeTracker tracker; + std::vector expected_ops; + { + pro::proxy p1{ + std::in_place_type, &tracker}; + expected_ops.emplace_back(1, + utils::LifetimeOperationType::kValueConstruction); + pro::proxy p2; + swap(p1, p2); + ASSERT_FALSE(p1.has_value()); + ASSERT_TRUE(p2.has_value()); + ASSERT_EQ(ToString(*p2), "Session 2"); + expected_ops.emplace_back(2, + utils::LifetimeOperationType::kCopyConstruction); + expected_ops.emplace_back(1, utils::LifetimeOperationType::kDestruction); + ASSERT_TRUE(tracker.GetOperations() == expected_ops); + + swap(p1, p2); + ASSERT_TRUE(p1.has_value()); + ASSERT_EQ(ToString(*p1), "Session 3"); + ASSERT_FALSE(p2.has_value()); + expected_ops.emplace_back(3, + utils::LifetimeOperationType::kCopyConstruction); + expected_ops.emplace_back(2, utils::LifetimeOperationType::kDestruction); + ASSERT_TRUE(tracker.GetOperations() == expected_ops); + } + expected_ops.emplace_back(3, utils::LifetimeOperationType::kDestruction); + ASSERT_TRUE(tracker.GetOperations() == expected_ops); +} + +TEST(ProxyLifetimeTests, TestSwap_PrefersRelocation) { + struct Relocatable + : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::build {}; + int v1 = 111, v2 = 222; + pro::proxy p1 = &v1; + pro::proxy p2 = &v2; + static_assert(!noexcept(p1.swap(p2))); + swap(p1, p2); + ASSERT_EQ(ToString(*p1), "222"); + ASSERT_EQ(ToString(*p2), "111"); +} + TEST(ProxyLifetimeTests, Test_DirectConvension_Lvalue) { utils::LifetimeTracker tracker; std::vector expected_ops;