On Fri, 19 Aug 2022 at 18:15, Edward Catmur <ecatmur_at_[hidden]> wrote:

> On Fri, 19 Aug 2022 at 16:27, Sébastien Bini <sebastien.bini_at_[hidden]>
> wrote:
>
>> On Fri, Aug 19, 2022 at 2:29 PM Edward Catmur <ecatmur_at_[hidden]>
>> wrote:
>>
>>> That being said, now lies the problem on how to write a custom
>>>> implementation for this assignment operator. For the relocation ctor, we
>>>> worked around this by saying that omitted subobject initializers would
>>>> automatically call the appropriate reloc ctor of said subobject. We cannot
>>>> do that magic for the reloc-assign, as there is no initializer list.
>>>> However, I believe that std::relocate can come to the rescue here:
>>>>
>>>> class T : B
>>>> {
>>>> D _d;
>>>> reloc T& operator=(T rhs)
>>>> { // exception safety is omitted for the example
>>>> B::operator=(std::relocate(static_cast<B*>(&rhs)));
>>>> _d = std::relocate(&rhs._d);
>>>> return *this;
>>>> }
>>>> };
>>>>
>>>> std::relocate is a nice fit here as, thanks to the aliasing, we know
>>>> that rhs dtor will not be called. In the same manner, std::relocate
>>>> could also be used to initialize subobjects in the reloc ctor.
>>>>
>>>
>>> Sorry, no. Aliasing doesn't prevent the destructor being called, it just
>>> means that the parameter is the same object as (has the same address as)
>>> the argument. It still has to be (will be) destroyed, either by the
>>> destructor or by the relocating constructor. If you want to write code
>>> like that, you need to relocate rhs into a buffer (placement-new-relocate)
>>> or union.
>>>
>>
>> That's not how I see it. I agree with the address part, but I don't see
>> why the destructor of the source object would still need to be called.
>>
>> In `auto x = reloc y;` given that `y` has not opted-out of the ABI break,
>> it is being relocated into `x`, and its destructor will not be called. We
>> consider that the relocation constructor has done the job already.
>>
>> Why would it be different for the assignment operator? In `x = reloc y;`,
>> with the aliased-prvalue-assignment operator, `y` is also being relocated
>> into `x`. The caller site can perfectly alleviate the call to `y`
>> destructor, as it would do if the relocation constructor were called.
>> Likewise, we consider that the aliased-prvalue-assignment operator has done
>> the job already.
>>
>
> Yes, but we can't trust the user to correctly destroy or relocate-from
> each subobject.
>
> With a relocating constructor, as you'll remember, the default behavior is
> to memberwise relocate each subobject; for each subobject mentioned in the
> base-and-member initialization list, to initialize that subobject on the
> target object according to its initializer and then to destruct the
> corresponding subobject on the source object, but for each subobject not so
> mentioned, to perform (destructive) relocation. By that means, we ensure
> that each subobject of the source object is either destroyed or
> (destructively) relocated-from.
>
> With a relocating assignment operator, there is no analogous syntax, so no
> way to ensure that each subobject is in fact destroyed. Suppose you add a
> data member std::unique_ptr<int> T::_p but forget to relocate it in your
> relocating assignment operator; that would be a leak. This is too much of
> a footgun to be acceptable.
>
> Of course, a defaulted relocating assignment operator doesn't have this
> issue. But that's all-or-nothing.
>
> As we consider that the reloc constructor destroys its parameter, the
>> aliased-prvalue-assignment does the same. And that's why we need aliasing.
>>
>> [...]
>>
>> I get what you mean, the prvalue-assignment operator could simply be not
>> aliased and take its input parameter as a copy. But I fail to see:
>>
>> - where the relocation is involved, except in the potentially elided
>> copy? If relocation is not involved any further, why even mention
>> prvalue-assignment in the proposal?
>>
>> It's not a copy; it's a relocated instance, with that relocation
> potentially elided. If you then use destroy-and-rebuild, the sequence is
> then:
>
> T::T(T); // relocate source into assignment-operator parameter
> T::~T(); // destroy target
> T::T(T); // relocate parameter into target
>
> And with elision, it's:
>
> T::~T(); // destroy target
> T::T(T); // relocate source into target
>
> Note that the destructor of the source object is not called, nor of the
> parameter (if elision does not occur); this is because both have been
> relocated-from, thus obviating the destructor call.
>
>>
>> - in its implementation, how do you forward to subobject
>> prvalue-assignment operator, without involving recursively each time a copy
>> of said subobject?
>>
>> Preferably, you don't; you use destroy-and-rebuild, or copy-and-swap
> (where the copy is a potentially-elided relocate). If you really want to,
> you relocate into a buffer or union, taking your own responsibility for
> relocating or destroying each subobject.
>
>>
>> - the consistency with the relocation constructor?
>>
>> By = default; or by destroy-and-rebuild.
>
>>
>> - and the biggest flaw IMO: what would be the actual gain compared to
>> the move assignment operator? What's great with the relocation constructor,
>> or with the aliased-prvalue-assignment, is that they are in charge of the
>> destruction of the source object. They can leave the source object in a
>> dirty or inconsistent state, that would be broken if passed to a
>> destructor, because then it just becomes unused uninitialized memory. If
>> the prvalue-assignment operator is not in charge of the destruction of its
>> source object, it fails to hold one of the promises of relocation and we
>> are no better than with the move assignment operator.
>>
>> Oh! Burying the lede there. It'd be great if we didn't have to call the
> destructor, but we have to be consistent with existing code. And there are
> at least two strategies available that work with non-movable objects (like
> non_null<unique_ptr>): destroy-and-relocate, and relocate-and-swap. I'd
> like to give this some more thought, though.
>

To expand on this a little further:

* the gain relative to the move assignment operator is that for types that
are non-movable by virtue of not having an empty state, the only way to
successfully write a move assignment operator is for it to swap the
contents of source and target. But since the source of such an assignment
operator is an xvalue, not a prvalue, its destruction may be delayed
indefinitely, leading to potential hazards e.g. deadlocks. However, for a
relocating assignment operator, we can ensure that the source object is
destroyed within the body of the relocating assignment operator (by
relocating from it), so we can release the resource previously owned by the
target within the body of the relocating assignment operator, either before
(destroy-and-rebuild) or immediately after (copy-and-swap) we transfer the
resource owned by the source object to the target object.
* consistency with existing code: one can already write a non-special
assignment operator with the same signature as we intend to use for
relocating assignment. We don't want such code to change meaning and
possibly develop a leak.
* there are three ways to avoid the destructor call on the source object:
relocate into the target (having previously destroyed it); relocate into a
temporary (and then swap the contents of that temporary with the target);
relocate into a union member (and then take responsibility for resources).
Adding another method is unnecessary.

An aside; for types that have the new callee-destroy ABI (through being
relocate-only, or through declaring a relocating constructor and not opting
out of the new argument passing ABI), the copy part of copy-and-swap is
unnecessary: swap simpliciter is sufficient, since the source object is
then destroyed at the closing brace of the relocating assignment operator.
But copy-and-swap is manifestly safer for use with caller-destroy types.