llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.22k stars 11.64k forks source link

Class without default constructor, but with destructor is not trivially constructible #18187

Open llvmbot opened 10 years ago

llvmbot commented 10 years ago
Bugzilla Link 17813
Version trunk
OS All
Reporter LLVM Bugzilla Contributor
CC @DougGregor,@zygoloid

Extended Description

The following code does not compile with clang r193986:

struct Foo {
    ~Foo();
};

static_assert(__is_trivially_constructible(Foo), "");
static_assert(__has_trivial_constructor(Foo), "");

This results in:

% ~/LLVM/build/Release+Asserts/bin/clang -c -std=c++11 clang.cpp                                                           
clang.cpp:5:1: error: static_assert failed ""
static_assert(__is_trivially_constructible(Foo), "");
^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Removing the destructor makes the code compile.

This looks like an inconsistency between __is_trivially_constructible and __has_trivial_constructor, the former of which are used by libc++'s std::is_trivially_constructible type trait (the latter is only used if the former is not available).

Also __is_trivially_constructible is influenced by the existence of a destructor, despite §12.1p5 not mentioning destructors:

A default constructor is trivial if it is not user-provided and if: — its class has no virtual functions (10.3) and no virtual base classes (10.1), and — no non-static data member of its class has a brace-or-equal-initializer, and — all the direct base classes of its class have trivial default constructors, and — for all the non-static data members of its class that are of class type (or array thereof), each such class has a trivial default constructor.

llvmbot commented 10 years ago

__is_trivially_constructible(T, Args...) is designed to exactly implement std::is_trivially_constructible<T, Args...>. That checks whether the following declaration "is known to call no operation that is not trivial":

First of all thank you for you detailed explanation! Also I agree that the wording in the Standard is not very clear. Still, going by the spirit of std::is_trivially_constructible I don't think it makes sense to consider T's destructor, as the question is only whether T is trivially constructible, not destructible as well.

I'd also argue that the quote from §20.9.4.3 table 49:

the variable definition for is_constructible, as defined below, is known to call no operation that is not trivial

only talks about "calling" no non-trivial operation, not about "requiring". The declaration of t calls "create" (which as you said is defined to be trivial) and T's constructor to perform direct-initialization of t (§8.5p15). T's destructor is only called when t goes out of scope, not when t is defined.

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 10 years ago

__is_trivially_constructible(T, Args...) is designed to exactly implement std::is_trivially_constructible<T, Args...>. That checks whether the following declaration "is known to call no operation that is not trivial":

  T t(create<Args>()...);

(where, per LWG issue 2336, we are supposed to imagine that create<Args> is somehow trivial). This declaration results in a call to the destructor of t, but it's not clear whether we intentionally consider this or whether it's an accident of our implementation approach.

__has_trivial_constructor has a weird spec, per the GCC documentation (http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html), and doesn't match std::is_trivially_constructible:

If __is_pod (type) is true then the trait is true, else if type is a cv class or union type (or array thereof) with a trivial default constructor ([class.ctor]) then the trait is true, else it is false. Requires: type shall be a complete type, (possibly cv-qualified) void, or an array of unknown bound."

We implement it according to that spec, except (following gcc's behavior as best as we can) we check whether the class has a trivial default constructor and no non-trivial default constructors, not just that it has a trivial default constructor. (A class can have multiple default constructors through default arguments or variadic constructor templates or through a vararg constructor.)

So... the inconsistency is deliberate. Checking the destructor may or may not be intentional, but the library wording does not seem very clear.