New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Class without default constructor, but with destructor is not trivially constructible #18187
Comments
__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()...); (where, per LWG issue 2336, we are supposed to imagine that create 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. |
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 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. |
…pe traits Some compiler provided type traits like __has_trivial_constructor have been documented as deprecated for quite some time. Still, some people apparently still use them, even though mixing them with concepts and with deleted functions leads to weird results. There's also disagreement about some edge cases between GCC (which Clang claims to follow) and MSVC. This patch adds deprecation warnings for the usage of those builtins, except for __has_trivial_destructor which doesn't have a GCC alternative. I made the warning on by default, so I had to silence it for some tests but it's not too many. Some (decade old) history of issues with those builtins: #18187 #18559 #22161 #33063 The abseil usage of them that triggered me to add this warning: abseil/abseil-cpp#1201 Weird interaction of those builtins with C++20's conditionally trivial special member functions: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106085 Reviewed By: #clang-language-wg, aaron.ballman Differential Revision: https://reviews.llvm.org/D129170
…pe traits Some compiler provided type traits like __has_trivial_constructor have been documented as deprecated for quite some time. Still, some people apparently still use them, even though mixing them with concepts and with deleted functions leads to weird results. There's also disagreement about some edge cases between GCC (which Clang claims to follow) and MSVC. This patch adds deprecation warnings for the usage of those builtins, except for __has_trivial_destructor which doesn't have a GCC alternative. I made the warning on by default, so I had to silence it for some tests but it's not too many. Some (decade old) history of issues with those builtins: llvm/llvm-project#18187 llvm/llvm-project#18559 llvm/llvm-project#22161 llvm/llvm-project#33063 The abseil usage of them that triggered me to add this warning: abseil/abseil-cpp#1201 Weird interaction of those builtins with C++20's conditionally trivial special member functions: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106085 Reviewed By: #clang-language-wg, aaron.ballman Differential Revision: https://reviews.llvm.org/D129170
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.
The text was updated successfully, but these errors were encountered: