On Wed, Dec 21, 2022 at 9:19 PM Edward Catmur <ecatmur_at_[hidden]>
wrote:

> We could simply resolve this issue by stating that no relocation
>> constructor will be added to lock_guard,
>>
>
> Yes, I think that in this specific case it can be argued that
> std::lock_guard should not be made relocatable, since at present one knows
> that if one passes a prvalue std::lock_guard to a function, the mutex is
> necessarily unlocked at the time the function returns, which would no
> longer be the case if it were relocatable. (Even if one doesn't know
> whether the mutex is unlocked at the point of returning or at the end of
> the containing expression, the caller can ensure those two points are the
> same.)
>

That's a compelling argument not to make lock_guard relocate-only. I guess
we could be satisfied by simply adding a relocation ctor to unique_lock. I
feel we have so many guards already, that adding a new one just to be
relocate-only, while having the same purposes of unique_lock would be a bit
overkill.

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.

but that does not fix more pernicious cases, like functions with signature:
>> void foo(non_null<unique_ptr<int>>); non_null and unique_ptr will get a
>> relocation ctor, and then non_null<unique_ptr> will be automatically
>> relocate-only, potentially changing the ABI of foo.
>>
>> I don't know to what extent we can simply ditch those cases by saying
>> they are bad code (really, who would pass a non_null<unique_ptr> by value
>> today?), and the ABI break is the price to pay for writing such bad code in
>> the first place. Maybe an ABI break on such functions is acceptable. I
>> doubt any major libraries provide such APIs?
>>
>
> Right, there's no reason to have a parameter of type
> non_null<unique_ptr<T>>, since it could just be T&.
>
> Still, this does mean that such ABI breakage does need to be detectable; I
> think it might be necessary to propose different mangling for
> callee-destroy parameters. One could even have a scheme where up to two
> symbols are emitted, where the old (caller-destroy) symbol is emitted only
> if the function does not in actual fact relocate from its parameters, in
> which case the new (callee-destroy) symbol forwards to the old and then
> destructs its relocatable parameters on exit. (I think there was a similar
> proposal - not mine - in the original thread.)
>

 Sure, that indeed rings a bell. That would only help in detecting ABI
incompatibilities (as link errors instead of silent crashes), or am I
missing something? But good point!