Skip to content

Commit d9393de

Browse files
committed
Reach the trivial copy and relocation by the source facade
A converting initialization asked the destination facade whether the copy or the relocation is trivial, but the triviality that licenses a byte copy belongs to the value being moved, which lives in the source. A super is never stricter than the facade converting to it, so the byte copy was missed whenever the derived facade was the stricter of the two, and the value went through the erased invoker instead. Corrected three lines of the specification along the way. The copy assignment is not "as if by auto(rhs).swap(*this)", which is not even formed for a facade that forbids relocation. The converting move assignment loses the contained value on a throwing relocation exactly as the move assignment does. The move constructor is noexcept for trivial relocatability, which its documented signature spelled as an equality.
1 parent c371353 commit d9393de

4 files changed

Lines changed: 28 additions & 5 deletions

File tree

docs/spec/proxy/assignment.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ proxy& operator=(P&& ptr)
5353
Assigns a new value to `proxy` or destroys the contained value.
5454
5555
- `(1)` Destroys the current contained value if it exists. After the call, `*this` does not contain a value.
56-
- `(2)` Copy assignment operator copies the contained value of `rhs` to `*this`. If `rhs` does not contain a value, it destroys the contained value of `*this` (if any) as if by `auto(rhs).swap(*this)`. The copy assignment is trivial when `F::copyability == constraint_level::trivial` is `true`.
56+
- `(2)` Copy assignment operator copies the contained value of `rhs` to `*this`. If `rhs` does not contain a value, it destroys the contained value of `*this` (if any). The copy assignment is trivial when `F::copyability == constraint_level::trivial` is `true`.
5757
- `(3)` Move assignment operator moves the contained value of `rhs` to `*this`. If `rhs` does not contain a value, it destroys the contained value of `*this` (if any). If the move construction throws when `F::relocatability == constraint_level::nontrivial`, `*this` does not contain a value. After move assignment, `rhs` is in a valid state with an unspecified value. The move assignment operator does not participate in overload resolution when `F::copyability == constraint_level::trivial`, falling back to the trivial copy assignment operator.
5858
- `(4)` Converting copy assignment operator copies the contained value of `rhs` to `*this`, as if by constructing a `proxy` from `rhs` and assigning it. If `rhs` does not contain a value, it destroys the contained value of `*this` (if any). Participates in overload resolution only if `F2` is not `F` and `F` is a super of `F2`, reachable via `typename F2::super_types` transitively.
59-
- `(5)` Converting move assignment operator moves the contained value of `rhs` to `*this`. If `rhs` does not contain a value, it destroys the contained value of `*this` (if any). After the assignment, `rhs` does not contain a value. Participates in overload resolution only if `F2` is not `F` and `F` is a super of `F2`, reachable via `typename F2::super_types` transitively. The converting move assignment operator does not participate in overload resolution when `F::copyability == constraint_level::trivial`, falling back to `(4)`.
59+
- `(5)` Converting move assignment operator moves the contained value of `rhs` to `*this`. If `rhs` does not contain a value, it destroys the contained value of `*this` (if any). If the move construction throws when `F::relocatability == constraint_level::nontrivial`, `*this` does not contain a value. After the assignment, `rhs` does not contain a value. Participates in overload resolution only if `F2` is not `F` and `F` is a super of `F2`, reachable via `typename F2::super_types` transitively. The converting move assignment operator does not participate in overload resolution when `F::copyability == constraint_level::trivial`, falling back to `(4)`.
6060
- `(6)` Let `VP` be `std::decay_t<P>`. Sets the contained value to an object of type `VP`, direct-non-list-initialized with `std::forward<P>(ptr)`. Participates in overload resolution only if `VP` is not a specialization of `proxy` and is a pointer-like type eligible for `proxy` (see [*ProFacade* requirements](../ProFacade.md)). *Since 3.3.0*: If [`proxiable<VP, F>`](../proxiable.md) is `false`, the program is ill-formed and a diagnostic is generated.
6161
6262
## Return Value

docs/spec/proxy/constructor.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ proxy(const proxy& rhs)
1515

1616
// (3)
1717
proxy(proxy&& rhs)
18-
noexcept(F::relocatability == constraint_level::nothrow)
18+
noexcept(F::relocatability >= constraint_level::nothrow)
1919
requires(F::relocatability >= constraint_level::nontrivial &&
2020
F::copyability != constraint_level::trivial);
2121

include/proxy/v4/detail/core.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,7 +1421,7 @@ class proxy : public detail::facade_traits<F>::direct_accessor,
14211421
void initialize(const proxy<F2>& rhs) {
14221422
PRO4D_DEBUG(std::ignore = &pro_symbol_guard;)
14231423
if (rhs.has_value()) {
1424-
if constexpr (F::copyability == constraint_level::trivial) {
1424+
if constexpr (F2::copyability == constraint_level::trivial) {
14251425
std::uninitialized_copy_n(rhs.ptr_, F2::max_size, ptr_);
14261426
} else {
14271427
invoke<detail::copy_dispatch,
@@ -1438,7 +1438,7 @@ class proxy : public detail::facade_traits<F>::direct_accessor,
14381438
PRO4D_DEBUG(std::ignore = &pro_symbol_guard;)
14391439
if (rhs.has_value()) {
14401440
auto meta = rhs.meta_;
1441-
if constexpr (F::relocatability == constraint_level::trivial) {
1441+
if constexpr (F2::relocatability == constraint_level::trivial) {
14421442
std::uninitialized_copy_n(rhs.ptr_, F2::max_size, ptr_);
14431443
rhs.meta_.reset();
14441444
} else {

tests/proxy_lifetime_tests.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1284,3 +1284,26 @@ TEST(ProxyLifetimeTests, Test_ConvertingCopyAssignment_NoRelocation) {
12841284
ASSERT_TRUE(p2.has_value());
12851285
ASSERT_EQ(ToString(*p2), "Session 2");
12861286
}
1287+
1288+
TEST(ProxyLifetimeTests, Test_Substitution_TrivialDerived) {
1289+
struct Super : pro::facade_builder //
1290+
::add_convention<utils::spec::FreeToString,
1291+
std::string() const> //
1292+
::support_copy<pro::constraint_level::nothrow> //
1293+
::support_relocation<pro::constraint_level::nontrivial> //
1294+
::build {};
1295+
struct Derived : pro::facade_builder //
1296+
::add_facade<Super> //
1297+
::support_copy<pro::constraint_level::trivial> //
1298+
::restrict_layout<sizeof(int*), alignof(int*)> //
1299+
::build {};
1300+
static_assert(Derived::max_size < Super::max_size);
1301+
int v = 123;
1302+
pro::proxy<Derived> p1 = &v;
1303+
pro::proxy<Super> p2 = p1;
1304+
ASSERT_TRUE(p1.has_value());
1305+
ASSERT_EQ(ToString(*p2), "123");
1306+
pro::proxy<Super> p3 = std::move(p1);
1307+
ASSERT_FALSE(p1.has_value());
1308+
ASSERT_EQ(ToString(*p3), "123");
1309+
}

0 commit comments

Comments
 (0)