[c++17] copy-initialization that calls a constructor should not potentially invoke the destructor

[llvmbot]

Bugzilla Link	39363
Version	unspecified
OS	Windows NT
Reporter	LLVM Bugzilla Contributor
CC	@dwblaikie,@francisvm,@mclow,@zygoloid,@tiagomacarios,@jwakely

Extended Description

https://godbolt.org/z/a8ml4u

#include <memory>

struct foo;

struct bar
{
    // ok
    std::unique_ptr<foo> a;

    // ok
    std::unique_ptr<foo> b{};

    // not ok
    std::unique_ptr<foo> c{nullptr};
};

According to http://eel.is/c++draft/unique.ptr#4.sentence-3, it should be ok for the type to be incomplete. Since this is construction and not assignment, it is not necessary to call delete and so we don't need to do a reset, hence we shouldn't yet require a complete type.

[3a17e7c4-fca4-4827-b2a7-11a54d73d746]

N.B. the unique_ptr bug actually came directly from the standard, it's a defect in both C++11 and C++14, only fixed (apparently accidentally) by https://cplusplus.github.io/LWG/issue2801

[3a17e7c4-fca4-4827-b2a7-11a54d73d746]

The GCC bug is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85552

I'll fix the libstdc++ bug by removing the constructor delegation.

[ec04fc15-fa35-46f2-80e1-5d271f2ef708]

I believe this is a bug in both Clang and GCC (in C++17 onwards): the destructor of the unique_ptr is not potentially-invoked in this program, so should not have a point of instantiation in this translation unit.

The list-initialization case does not potentially-invoke the destructor in C++11 or C++14 either (unlike non-list copy-initialization, list-initialization directly executes a constructor on the destination object even in C++11, and as a result there is no temporary object created to trigger the potential invocation of a destructor). Clang gets that case right, but GCC has another bug there.

Finally, the constructor delegation used by libstdc++'s implementation does potentially invoke the unique_ptr destructor, which appears to make that implementation technique non-conforming (not just here but in general: you can't use constructor delegation in a standard library component if your destructor has a stronger precondition, because the delegating constructor potentially invokes the destructor).

So: two bugs in GCC (though they might be the same thing), one bug in Clang's C++17 support, one bug in libstdc++, and no bugs in libc++. Retargeting this as a Clang bug. :)

[llvmbot]

CC'ing our resident Language Lawyer and Compiler Wizard for their opinion.

@​Richard, could you weigh in on the last couple examples? Does copy-elision still require checking the call to the destructor?

[llvmbot]

Hmm. Clang accepts MyTemp<Foo> t = {1}; but not MyTemp<Foo> t = 1;. GCC accepts neither MyTemp<Foo> t = {1}; nor MyTemp<Foo> t{1};.

I'm going to guess there is ongoing disagreement about the semantics of this code.

[llvmbot]

Reopening to investigate further.

I suspect this is a problem at the language level, and there is nothing libc++ is doing wrong or can do about it. However, I'll re-open the bug until I can confirm.

Here is a minimal reproducer for both GCC and Clang: https://godbolt.org/z/5I_2fq

[llvmbot]

I would expect that to work only when C++17 guaranteed copy elission occurs. In all other cases I would expect it to fail since it requires instantiating the destructor.l

[91b604ff-5b4e-4821-a182-129f49d1a44c]

Hi Eric should the below work? It is what I initially reported to Zachary.

https://godbolt.org/z/62TLrS

#include <memory>

struct foo;

struct bar
{
    std::unique_ptr<foo> a{nullptr};
    std::unique_ptr<foo> b = nullptr;
};

[llvmbot]

libc++ supports this use case, we have tests for it, and it works.

The godbolt link you provided is using libstdc++, not libc++. Adding -stdlib=libc++ makes the example compile as expected (https://godbolt.org/z/pHpTlH)

[mclow]

At the point of instantiation (as opposed to declaration), I suspect that T has to be a complete type.

So this would be ok:

     class Foo;
     using Up = std::unique_ptr<Foo>;

     class Foo { /* ... */ }
     Up p{nullptr};

Since this is construction and not assignment, it is not necessary to call delete and so we don't need to do a reset, hence we shouldn't yet require a complete type.

At the point of construction, we have to instantiate a deleter.

[llvmbot]

assigned to @mclow