Hello all,

Sorry for the late reply.

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.

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.

Thank you for reading & regards,
Sébastien