On Thu, Dec 22, 2022 at 5:36 PM Edward Catmur <ecatmur_at_[hidden]>
wrote:

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

I don't see why one would want it to be on a parameter basis, but I am okay
with leaving it implementation-defined. The requirement then becomes that a
function must be in charge of the lifetime of its relocate-only value
parameters. Implementations are free to take any ABI decisions.

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

Yes.

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

The second approach seems better. But what do you mean with "destruction is
deferred until the end of the full-expression"? in `reloc src;` I expect
the destructor to be called when the code reaches the next instruction. How
is that an issue? Unless you have more complex expressions in mind?

I don't see the benefits of making it ill-formed for immovable types, while
I clearly see some benefits of allowing it. A function taking a lock_guard
by value needs not do anything different than before, since lock_guard has
no relocation constructor. If the ABI allows it, then `reloc guard;` will
call the destructor, otherwise it's ill-formed.


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