On Sun, 14 Aug 2022 at 20:35, Edward Catmur <ecatmur_at_[hidden]> wrote:

> On Tue, 2 Aug 2022 at 09:37, Sébastien Bini <sebastien.bini_at_[hidden]>
> wrote:
>
>> On Fri, Jun 10, 2022 at 2:04 AM Edward Catmur <ecatmur_at_[hidden]>
>> wrote:
>>
>>> On Wed, 8 Jun 2022 at 09:34, Sébastien Bini <sebastien.bini_at_[hidden]>
>>> wrote:
>>>
>>>> Hi all,
>>>>
>>>> Well, yes; certainly arguing about the exact spelling is bikeshedding.
>>>>> One aspect I think needs further exploration is a similar facility to allow
>>>>> running code before the relocation of subobjects begins, and I think
>>>>> constructor delegation could be the facility to solve that; the other
>>>>> aspect is symmetry with operator=(T).
>>>>>
>>>>
>>>> The delegating constructor will not have the ability to do memberwise
>>>> relocation and to avoid the destructor call. As such using the delegating
>>>> constructor to inject code right before relocation will lose the benefit of
>>>> having a relocation ctor in the first place.
>>>>
>>>
>>> Yes, this is a bit unfortunate. Possibly the delegated constructor could
>>> use std::relocate on subobjects and some trick found to prevent the
>>> destructor call. Or we could return to function-try-with-init.
>>>
>>> Why would you need to have that option? We don't support that for any
>>>> other ctor (custom code before member initialisation).
>>>>
>>>
>>> We do, via constructor delegation. The need here to invoke custom code
>>> is to safely relocate objects that both directly own a resource (i.e. their
>>> destructor releases the resource) and have data members with throwing
>>> relocate. The idea is that before starting to relocate members you can
>>> create a guard object that will release the resource if a data member
>>> relocator throws, to avoid a resource leak.
>>>
>>
>> What about a separate proposal that would allow you to inject custom code
>> on any constructor, before the initializer list? I would have personally
>> found it useful on some occasions, and could prove its full usefulness with
>> relocation:
>>
>> class T
>> {
>> public:
>> T(T&& rhs) : std::unique_ptr<U> guard{MakeGuard(rhs)},
>> T{std::move(rhs), 0} { guard.release(); }
>> // or with relocation ctor:
>> T(T rhs) : auto guard{MakeGuard(rhs)}, auto
>> guard2{MakeSecondGuard(rhs)} /* default memberwise relocation of subojects
>> is done here automatically */ { guard2.release(); guard.release(); }
>> private:
>> T(T&& rhs, int); // guarded ctor
>> };
>>
>> The idea is to declare and initialize new names before the nominal list
>> of initializers (delegating constructors or subobjects). The introduced
>> objects would be recognized as their type and names appear clearly. Their
>> destructor would be called at scope exit (even if due to an exception), in
>> reverse declaration order as usual.
>>
>
> Yes, this is what I'm suggesting as try-with-init. The issue is finding a
> way to add the initialization into the; the position between the `try` and
> `{` of a function-try-block is currently free real estate. Adding it
> directly before the base-and-member-initialization list would be ambiguous
> both to the compiler and to the reader.
>

Sorry, this should be the `try` and the `:` (introducing the
*ctor-initializer*), of course.

As for operator=(T), what do you have in mind? I am satisfied with leaving
>>>> it implementation defined, users can freely use destruct + relocate or use
>>>> swap at their own convenience.
>>>>
>>>
>>> Ah, hmm. The difficulty is that if we make it support `= default;` and
>>> that does the Obvious Thing of memberwise reloc-assign then we're setting
>>> up a footgun; it would turn into a memory leak for std::unique_ptr;
>>> memberwise reloc-assign is incorrect for resource-owning classes, even if
>>> memberwise reloc-construction is correct for them. But then again, a struct
>>> containing a non_null<unique_ptr> will need to generate a reloc-assignment
>>> operator that calls the reloc-assignment operator of non_null<unique_ptr>
>>> (since that's its only assignment operator), and it would feel weird to
>>> have a default implementation that the user can't access with `= default`.
>>>
>>> using N = non_null<unique_ptr<int>>;
>>> struct S { N n; } s1 = ..., s2 = ...;
>>> s1 = reloc s2; // calls into N::operator=(N); N::operator=(N const&) and
>>> N::operator=(N&&) are both deleted.
>>>
>>
>> As you pointed out we cannot be satisfied with the default
>> implementation, as doing the memberwise reloc-assign is wrong. I can only
>> think of two ways of making reloc-assignments (x = reloc y;): either by
>> using (a) std::swap, or by relying on the uncanny (b) destruct + placement
>> new on this. Doing reloc-assignment using (a) comes at the cost of three
>> std::relocate_at (for the swap) and one destructor call (y is destructed at
>> the end of the swap):
>>
>> x = reloc y; // if done by std::swap, would cost 3 std::relocate_at,
>> + destructor call on y.
>>
>> Doing reloc-assign using (b) is cheaper as it only costs one
>> std::relocate_at call and one destructor call (on x):
>>
>> x = reloc y; // could be the same as std::destroy_at(&x);
>> std::relocate_at(&y, &x);
>>
>> For (b) to work though, the destructor of y must not be called, as if it
>> were used in reloc initialization:
>>
>> auto x = reloc y; // in this context, should x have a reloc ctor,
>> the destructor of y is not called.
>>
>> I suggest the following way to declare a reloc-assignment operator, that
>> would use (b) implementation:
>>
>> class T
>> {
>> public:
>> // [...]
>> T& operator=(T rhs) = reloc; // same as { std::destroy_at(this); return
>> *std::relocate_at(&rhs, this); }
>> };
>>
>> This declares a prvalue assignment operator, and defines it with a
>> special implementation. We do not use = default; as users may expect
>> memberwise relocation which is not what's happening. This = reloc; also
>> allows us to make it a special function so it can share similarities with
>> the reloc constructor: the destructor of rhs is not called at callee site,
>> and rhs is not actually passed by value to the operator=() despite its
>> signature, but by reference. This allows us to make the same set of
>> optimizations as with the relocation constructor.
>>
>> The destroy + placement new approach is considered safe. Since rhs is a
>> prvalue (or behind the scene, a reference to a prvalue), its address cannot
>> be (by construction) this or any subobject of *this. As such
>> std::destroy_at(this); cannot destruct rhs as a side effect.
>>
>> If an exception leaks through this reloc-assignment operator then *this
>> will likely be in a destructed state (the exception was either thrown by
>> std::destroy_at(this), leaving the object in a somewhat destructed state,
>> or by std::relocate_at which will have failed to reconstruct *this
>> properly). That destructed state may have disastrous consequences if *this
>> is an object with automatic storage whose destructor will inevitably be
>> called at some point (probably even during the stack unwinding incurred by
>> the exception leak), which will result in calling a destructor on a
>> destructed object. For this reason, I suggest that declaring this
>> reloc-assign operator yields a compilation error if the destructor and the
>> relocation operations are not noexcept. Note that this forcefully renders
>> the reloc-assign operator noexcept. Also, std::relocate_at needs to be
>> valid so the type must have a copy, move or relocation ctor.
>>
>> I further suggest adding an extra declaration: T& operator=(T rhs) =
>> reloc(auto); which will silently ignore the reloc-assignment operator
>> declaration if the above requirements are not met (useful for template
>> code).
>>
>> Note that if a class does not provide a reloc-assignment operator,
>> instructions such as `x = reloc y;` are still possible. reloc merely
>> transforms y into a prvalue. Overload resolution will simply pick the
>> move-assignment operator or copy-assignment operator instead if provided.
>>
>
> I'm not too sure. The thing is, this isn't too difficult to write by hand;
> even the exception safety stuff can be done with static_assert and requires
> (for reloc(auto)).
>
> But also, there's a safer way to implement relocating assignment, which is
> (as always) copy-and-swap:
>
> T& operator=(T rhs) { rhs.swap(*this); return *this; }
>
> We should be encouraging the use of copy-and-swap idiom (well, without the
> copy) rather than the unsafe destroy-and-relocate. The only problem is that
> there isn't a way to automatically generate a memberwise swap...
>