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.

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...