On Thu, 22 Dec 2022 at 16:15, Sébastien Bini <sebastien.bini_at_[hidden]>
wrote:

> > If there is no guarantee that `reloc param;` would immediately destroy a
> value
> > parameter, would the (ugly?) implementation-defined fallback for
> caller-destroy
> > be that - if a move assignment operator is defined - immediately the
> object is
> > move-assigned to a temporary variable and that temporary variable is
> destroyed
> > in turn. And after the function call, the moved-out-object is destroyed
> by the
> > caller?
>
> See below.
>
> On Thu, Dec 22, 2022 at 11:38 AM Edward Catmur <ecatmur_at_[hidden]>
> wrote:
>
>> On Thu, 22 Dec 2022 at 11:11, Sébastien Bini <sebastien.bini_at_[hidden]>
>> wrote:
>>
>>> That makes me wonder what would happen in the following code (I don't
>>> recall we ever took a definitive decision on that point):
>>>
>>> void do_something(std::lock_guard<std::mutex> guard)
>>> {
>>> if (!some_test())
>>> {
>>> reloc guard; /* attempt to release the lock early
>>> as the log function doesn't need it */
>>> log("thread " << std::this_thread::get_id() << " failed");
>>> return;
>>> }
>>> bar();
>>> }
>>>
>>> If the ABI is caller-destroy then `guard` destructor will forcibly be
>>> called at function exit.
>>> What would happen if `guard` were a non-function-parameter local
>>> variable, or the ABI callee-destroy? In other terms, should `reloc` offer
>>> any guarantee on when the destructor is called?
>>>
>>> If we want to keep `reloc` consistent in all situations, then `reloc x;`
>>> should never call the destructor of x (which will be destroyed normally at
>>> its end of scope). If this approach is taken then everyone is dragged down
>>> because of that ABI issue which only some have and that may be resolved in
>>> the future.
>>> This is a missed opportunity in my opinion. `reloc x;` should, when
>>> possible, call the destructor right away. That would allow developers to
>>> preemptively call the destructor of an object, without wrapping the object
>>> in an optional (or use unique_lock in our case). The language will keep
>>> track of the destruction state for us (especially is used in conditional
>>> branches), and this would no longer be a burden for the developer (which
>>> means less bugs).
>>>
>>> I believe that `reloc src`:
>>>
>>> - if `src` is a local object and not a function parameter, then
>>> `src` must be left in a destructed state (either because it was passed to a
>>> relocation constructor or by a direct call to its destructor) at the end of
>>> the expression evaluation.
>>> - otherwise (`src` is a function parameter passed by value) then
>>> when `src` is destroyed is implementation-defined. Typically, as soon as
>>> the ABI permits.
>>>
>>> In the last case, if a `reloc src;` statement is used and the ABI does
>>> not allow to call the destructor right-away, then compilers can still emit
>>> a warning.
>>>
>>
>> I think this is too dangerous. If `src` is movable, then it's OK to
>> move-construct a temporary and defer destruction of the moved-from object
>> (compilers can warn on this if they feel like it). But if it is
>> relocate-only, and the ABI is caller-destroy on that parameter, `reloc src`
>> must be ill-formed.
>>
>
> Yes, I have in mind to force callee-destroy ABI for functions that take
> relocate-only value parameters.
>
>
>> But we want that to work, so if compilers want to allow relocate-only
>> parameters to be caller-destroy, that must be a compiler extension, with
>> the default for such parameters being callee-destroy. There should probably
>> be an example showing this:
>>
>> struct A { A() = default; A(A&&); ~A(); };
>> struct B { B() = default; B(B); ~B(); };
>> void f(A a, B b) {
>> reloc a; // may move and destroy at end
>> reloc b; // must destroy immediately
>> }
>>
>
> I get what you mean, but B being relocate-only, `f` will get
> callee-destroy ABI so in your case `reloc a` won't defer the destructor
> call.
>

Possibly, but it would also be reasonable to have callee-destroy ABI
fine-grained on a per parameter basis, making behavior more consistent if a
relocate-only parameter is added or removed. I think we should leave that
to the implementation.

Okay, things are getting confused for me so let's write things down.
>
> `reloc src` will turn `src` into a prvalue (a temporary).
>

Unless `src` is a reference. In that case, it will be turned into a
temporary lvalue or xvalue, depending on the type of reference.

If the temporary is materialized, then it will be as follows:
>
> 1. If `src` is not a value parameter, or the ABI is callee-destroy,
> then the temporary is initialized using the relocation, move or copy
> constructor. If the last two are picked, then the destructor of the source
> object is called ;
> 2. Otherwise, the temporary is initialized using the move or copy
> constructor, and the destructor of the source object is deferred until the
> function exits.
>
> That being said, we don't know if the `reloc src;` statement should
> require the temporary materialization. Given today's rules, that would be
> yes. But it's really not necessary should we be in the first case: the
> destructor can simply be called.
> If no materialisation is needed, then we can write `reloc guard;` (with
> guard of type `std::lock_guard`), and it will simply destroy the guard,
> even so lock_guard has no copy, move or reloc constructor.
>
> I am in favor of not requiring the temporary materialization in that case.
> It will still be required in the second case (`src` is a value parameter
> and ABI is not callee-destroy).
>

Yes, definitely. There is the question of how to accomplish that: do we
special-case discarded-value relocation expressions, or do we allow (maybe
require) elision of relocate+destroy into destroy? The latter feels more
elegant, but it might mean that destruction is deferred until the end of
the full-expression, and also might mean that `reloc src` is ill-formed for
completely non-relocatable types (no copy, move or relocating constructor).
Whether that last is a problem or not I'm not sure; it might actually be a
benefit. (For example, it would simplify the rule for a function taking
`lock_guard` by value.)

Having said that, things flow naturally for `reloc src;` :
>
> - if we are in the first case and materialisation is elided, then we
> simply call the destructor of src ;
> - if we are in the first case and materialisation is needed, then we
> materialize the temporary and destruct it. It has the same effect as
> destroying `src` directly, except that it's suboptimized and ill-formed for
> immovable objects ;
> - if we are in the second case, then we materialize the temporary and
> destruct it. The source object destruction is deferred until the function
> ends as it is the only thing we can do. Compilers should emit a warning in
> that case IMO.
>
> Hence, in my code snippet where I relocate a lock_guard value parameter
> (reloc guard;), the program is either ill-formed if the ABI is not
> callee-destroy or materialization is required (because lock_guard has no
> move or copy constructor to create a temporary), or else calls the
> destructor of the guard. And in:
>
> void do_something(std::mutex& m)
> {
> std::lock_guard guard{m};
> if (!some_test())
> {
> reloc guard;
> log("thread " << std::this_thread_get_id() << "failed");
> return;
> }
> bar();
> }
>
> `reloc guard;` would either be ill-formed if materialisation is required,
> or will simply call the destructor of the guard.
>

Yes; I'm leaning towards making it ill-formed, under the premise that
materialization is required but elided.