On Tue, 16 Aug 2022 at 14:25, Sébastien Bini <sebastien.bini@gmail.com> wrote:
On Mon, Aug 15, 2022 at 1:06 AM Edward Catmur <ecatmur@googlemail.com> wrote:
On Sun, 14 Aug 2022 at 20:35, Edward Catmur <ecatmur@googlemail.com> wrote:
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. 

To contextualize, you are suggesting something of the sort:

class T : B
  D _d;
  T(T rhs) : try (auto guard = MakeGuard(rhs)) /* optionally subobject initialization: */ B{rhs}, _d{rhs._d}
    /* reloc ctor body */
  catch (std::exception const&)
    /* catch body */

Actually, the `try` comes before the `:`:

T(T rhs) try (auto guard = MakeGuard(rhs)) : B{rhs}, ...

Generally speaking, a try-with-init ctor sounds dangerous. What happens if a subobject ctor throws, and the catch block absorbs the exception (e.g. does not call `throw` to propagate it). This means you would end up with a semi-constructed object, with some of its subobjects left uninitialized? It's even more hazardous if the constructor is a relocation constructor as some of the source subobjects will not be destructed.
The exception must be propagated somehow, the caller must know that its object was not properly constructed.

That's fine; a function-try-block on a constructor cannot swallow the exception: return is prohibited (http://eel.is/c++draft/except#handle-12.sentence-1), and reaching the end of the catch block automatically rethrows (http://eel.is/c++draft/except#handle-12.sentence-1).

But I don't get why you would need a try-with-init approach: the newly introduced object (e.g. `guard`) should clean-up its resources in its destructor, which will be called automatically or by stack-unwinding if a subobject ctor throws. What you need is, IMO, to have the ability to initialize an extra object before the subobjects initialization.

try-with-init gives you the opportunity to do something more with the guard object than just destruct it.  For example, you can capture a raw pointer and then delete it in the catch block.
The syntax I proposed is not ambiguous to the compiler as the newly introduced names must declare their type (so they cannot be data members) and name (so it cannot be a delegating/main class ctor call):

T(T rhs) : auto guard{MakeGuard(rhs)}, B{rhs}, _d{rhs._d} {}

Sure, but it would be quite easy to write this (or something that looks like it) by accident, without intending for it to be a guard object declaration.  It's a lot nicer as a compiler author to know immediately that you're parsing a try-with-init guard object definition, as opposed to a misspelled base-or-member initializer.

I admit it may be confusing to the reader. But as long as we agree on the solution, we are free to come up with any, clearer syntax we want:
T(T rhs) : [guard = MakeGuard(rhs)] B{rhs}, _d{rhs._d} {}

Absolutely. But try-with-init has the advantage that it doesn't even have to be a guard object, since the release code can go directly in the catch block. 

class T
  // [...]
  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 caller 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)).

You can write it by hand, but you cannot write by hand that the source object is not copied, nor that its destructor is not called. The = reloc provides that magic bit for us, as it does for the relocation ctor.

Hm, but you can write:

T& operator=(T rhs) { std::destroy_at(this); return *new (this) T(reloc rhs); }

If we permit operator=(T), as a special member function, the same aliasing power as the relocating constructor, then it invokes precisely one destructor call and one relocating constructor call, which is perfect.  If it doesn't have aliasing on its prvalue rhs parameter (admittedly that might be tricky from an ABI standpoint), then there's potentially two calls to the relocating constructor, but they can be elided together.

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

I agree that having =reloc using destroy-and-relocate may encourage users to use that idiom in places where it is not necessarily safe. Copy-and-swap is always safe although not as optimized in this precise use case...

I believe it is of prime importance to have the reloc-assignment operator to do exactly as intended. That means in `y = reloc x;`: we want the resources of `x` to be (mem)moved to `y`, `y` being cleaned beforehand, and to skip the destructor call on `x`. copy-and-swap does not quite fit in here in my opinion.

It's not quite as tidy, but copy-and-swap has that ironclad safety guarantee that's reassuring in library code.  Ideally, library code should be manifestly correct; and user code should be rule-of-zero so can use memberwise relocating assignment (by default).  So I am not sure that destroy-and-rebuild will actually find much use in practice to make it worth adding syntax for it.  Getting some implementation experience and the numbers on it could swing things.

"As intended" is tricky, because you can follow it with "but what if...".
What feels unsafe about destroy-and-relocate is that we destruct an object within a member function. We can tweak things around and turn the reloc-assignment operator into a static function behind the scene:
T& operator=(T src) = reloc; // same as: static T& assign_reloc(T& dst, T& src) { std::destroy_at(&dst); return *std::relocate_at(&src, &dst); }

Here we no longer have the scary std::destroy_at(this), and we avoid self-destruction in a member function (which may be an UB?).

That's still destroying an object from within its member function, just not directly.  It's fine, though; `delete this` has always been legal, it's just never been a sign of good code.  One library I use regularly (rapidjson) uses destroy-and-rebuild for (lvalue) assignment, and it works fine - as long as you sort out exception safety.

I fear that if we fallback to copy-and-swap then determined users will rely on dirty hacks to use some unsafe, home-made destroy-and-relocate instead.

But if they're that determined, they'll ignore the guardrails on the compiler-implemented version and write their own unsafe destroy-and-rebuild anyway (since the builtin one won't compile if it's unsafe).  Better to nudge users towards a copy-and-swap implementation that always works.