[Bug c++/112335] missed optimization on reset and assign unique_ptr

["redi at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Federico Kircheis from comment #7)
> > Demo: https://godbolt.org/z/PWd96j4fz
> 
> Thank you for the example, but honestly, I think I am still missing
> something.
> 
> If you modify the main this way
> 
> ----
> int main()
> {
>     global = nullptr;
>     std::unique_ptr<s> local1(new s);
>     bar1(global, local1);
>     global = nullptr;
>     std::unique_ptr<s> local2(new s);
>     bar1(global, local2);
> }
> ----
> 
> 
> or this way
> 
> 
> ----
> int main()
> {
>     global = nullptr;
>     std::unique_ptr<s> local1(new s);
>     bar2(global, local1);
>     global = nullptr;
>     std::unique_ptr<s> local2(new s);
>     bar2(global, local2);
> }
> ----
> 
> the output is the same.
> First time it prints 0, the second time a non-0 address.

You're right, my example failed to show the difference I was trying to show,
which is that the value of ps1.get() observed by the destructor is different.
To show that, global.get() must be non-null initially, but it was always nul in
my example (the addresses being printed out where when the destructors ran in
main, not in bar1 and bar2).

Try this one: https://godbolt.org/z/KEbnPxEdq

This shows that in bar1 the value of ps1.get() is null when the destructor
runs. In bar2 the value of ps2.get() is not null when the destructor runs.

> 
> > But a much simpler example where the difference is observable is when the two arguments to bar1 and bar2 are the same object:
> > https://godbolt.org/z/z8fsTan8K
> 
> And this is absolutely correct, did not think of it.
> 
> > I'm not sure what you mean here. The postcondition holds unless the deleter *also* destroys the unique_ptr itself.
> 
> As written, I was not sure if this plays a role; this is what the standard
> says (and I#ve misunderstood the example of Andrew).
> A similar effect can be observed in the following example
> 
> ----
> #include <memory>
> 
> using s = int; // for simplicity
> 
> void bar1(std::unique_ptr<s>& ps1, std::unique_ptr<s>& ps2){
>     ps1.reset();
>     ps1 = std::move(ps2);
> }
> void bar2(std::unique_ptr<s>& ps1, std::unique_ptr<s>& ps2){
>     ps1 = std::move(ps2);
> }
> 
> void bar3(std::unique_ptr<s>& ps1, std::unique_ptr<s>& ps2){
>     ps1.reset();
>     ps1.reset();
>     ps1 = std::move(ps2);
> }
> ----
> 
> While you are right that bar1 and bar2 have different behavior if they point
> to the same object, bar1 and bar3 are fundamentally the same function, even
> in case of aliasing.
> Unless the post-condition of reset is not hold; otherwise the second
> "ps1.reset()" could be completely removed, as ps1 is nullptr.

The postcondition has to hold, by definition, that's what it means. It only
doesn't hold if the unique_ptr object itself ceases to exist, and that doesn't
happen with std::default_delete. It's irrelevant whether it happens with some
other deleters, because the code you're compiling doesn't use those other
deleters (if it did, it would be compiled differently).

This example is more interesting, bar1 does:

       test    r12, r12
        je      .L11
        mov     rdi, r12
        call    s::~s() [complete object destructor]
        mov     rdi, r12
        mov     esi, 1
        call    operator delete(void*, unsigned long)


But bar3 has extra instructions:

       test    r12, r12
        je      .L11
        mov     rdi, r12
        call    s::~s() [complete object destructor]
        mov     rdi, r12
        mov     esi, 1
        call    operator delete(void*, unsigned long)
        mov     r12, QWORD PTR [rbx]
        mov     QWORD PTR [rbx], 0
        test    r12, r12
        je      .L11
        mov     rdi, r12
        call    s::~s() [complete object destructor]
        mov     esi, 1
        mov     rdi, r12
        call    operator delete(void*, unsigned long)

I think the reason the redundant ps1.reset() isn't removed is that the compiler
doesn't know what operator delete(void*, size_t) does. In theory, the user
could replace that function with one which modifies a global unique_ptr, and
ps1 could alias that global.

https://godbolt.org/z/WGPTesEb3 shows that bar3 is not the same as bar1,
because it runs an additional destructor if operator delete() plays silly
games.

If we had PR 110137 then I think the redundant ps1.reset() would be optimized
away. Without it, the compiler assumes operator delete() might be silly.

Luckily, that bar3 code seems unlikely to be written. The second ps1.reset()
can obviously be removed by the programmer.