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

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

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

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

Yes, it would result in a link error if:
* the calling code is compiled under an old compiler or old standard
version, and
* the function performs a `reloc` on a relocate-only parameter.