Boost C++ Libraries: Ticket #11835: boost::has_trivial_copy is incorrect on Clang

John Maddock — Wed, 09 Dec 2015 18:15:03 GMT

Hold on a second - type T in your example is not IMO trivially copyable (because it's not copyable at all). I realise some interpretations of the std will result in std::has_trivial_copy returning true for this type, but that seems to be bonkers to me. It has always been (deliberate) Boost.TypeTraits behaviour for such types to be non-trivial.

What am I missing?

joseph.thomson@… — Wed, 09 Dec 2015 20:00:50 GMT

As far as I can see, the standard unambiguously states that deleted copy constructors (and other similar functions) are trivial. I quote from the C++14 draft:

[class.copy]

A copy/move constructor for class X is trivial if it is not user-provided, its parameter-type-list is > equivalent to the parameter-type-list of an implicit declaration, and if

class X has no virtual functions and no virtual base classes, and
class X has no non-static data members of volatile-qualified type, and
the constructor selected to copy/move each direct base class subobject is trivial, and
for each non-static data member of X that is of class type (or array thereof), the constructor > selected to copy/move that member is trivial;
otherwise the copy/move constructor is non-trivial.

The key term here is "user-provided". Are deleted functions user-provided?

[dcl.fct.def.default]

A function is user-provided if it is user-declared and not explicitly defaulted or deleted on its first declaration.

There is no way around it: T has a trivial copy constructor.

But how about T being trivially copyable?

[class]

A trivially copyable class is a class that

has no non-trivial copy constructors,
has no non-trivial move constructors,
has no non-trivial copy assignment operators,
has no non-trivial move assignment operators, and
has a trivial destructor.

We have already established that the copy constructor is trivial. The other functions aren't even user-defined, so they must be trivial too. The unavoidable conclusion is that T is trivially copyable. For completeness, here is the definition of std::is_trivially_copyable:

[meta.unary.prop]

Template: template <class T> struct is_trivially_copyable;

Condition: T is a trivially copyable type

In my opinion, it does make sense, seeing as is_trivially_copyable simply states whether the object can be safely memcpy'd. Perhaps it's a badly named trait, but you can't deny what the standard says. An object doesn't need to have a copy constructor or assignment operator to be memcpy'd. Besides, what would the requirement be if they were needed? One or the other, or both? Seems better just to separate these concepts out into std::is_trivially_copy_constructible and std::is_trivially_copy_assignable, which is indeed what they did.

status changed; resolution set

John Maddock — Sat, 12 Dec 2015 18:31:39 GMT

status new → closed
resolution → wontfix

The point here is that has_trivial_copy is a Boost-specific trait so we can do what we want, and much more importantly: defining non-copyable types as trivially copyable drives a coach and horses through type safety, and leads to downright nasty and pernicious bugs. The original bug report you referenced is one such - types that are moveable but not copyable should never, ever be memcpy'd. If they are, then bad things typically happen. I may add a note to the docs to this effect, but the more I think about this the less inclined I am to make any change to current behaviour.

joseph.thomson@… — Sat, 12 Dec 2015 21:05:31 GMT

Okay, I did some more searching, and apparently there is a proposal to require at least one special function to not be deleted in order to classify as trivially copyable:

http://stackoverflow.com/questions/29759441/is-a-class-with-deleted-copy-constructor-trivially-copyable

This is because classes which use special OS-level resources, like atomics, mutexes and condition variables, should not be memcpy-able:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4460.html

Note that this still does not mean that a trivially copyable type requires a copy constructor; for example, a class with only a non-deleted move assignment operator may still be trivially copyable.

However, boost::is_trivially_copyable does not exist; this ticket is about boost::has_trivial_copy. You mention that the referenced bug was caused by the unsafe standard-compliant behaviour of GCC's __has_trivial_copy intrinsic; in fact, as I now realise, the bug is still there! Your preferred version of has_trivial_copy is essentially equivalent to std::is_trivially_copy_constructible, and is_trivially_copy_constructible is not a sufficient condition for memcpy-ability; the sufficient condition is is_trivially_copyable. A trivially copy constructible class might not be safe to memcpy, as it may have other non-trivial special functions (it may be unlikely that they would do anything unsafe, but it's possible).

This is actually a problem with your documentation. You should probably add your own version of is_trivially_copyable and tell people to check that if they want to memcpy things. And Boost's concept of "trivial" doesn't have to match the C++ standard. You can invent your own definition. However, you should probably apply it consistently.

First, the behaviour of boost::has_trivial_copy still varies between compilers. Specifically, GCC reports that deleted copy constructors are trivial.

Secondly, the behaviours of has_trivial_copy_assign, has_trivial_move_constructor and has_trivial_move_assign do not match has_trivial_copy, on both GCC and Clang.

However, once you have fixed these two issues, you have a problem if you want to implement is_trivially_copyable in terms of these other traits. As the proposed amendment mentions, defining deleted special functions as non-trivial means that this type becomes non-trivial:

struct X
{
    const int val;
};

Thus, your implementation of is_trivially_copyable would say that X cannot be safely memcpy'd. This is a problem, no?

In conclusion:

has_trivial_copy and its related traits do not adhere to your definition of "trivial" on some compilers.
The documentation for these traits says that they can be used to check whether memcpy is safe to use; this is incorrect, as is_trivially_copyable is the required condition.
Your preferred definition of "trivial" is too conservative, and will result in some obviously memcpy-able types reporting false for is_trivially_copyable.
The current standard definition of is_trivially_copyable is potentially dangerous when types use certain OS-level resources; a pending amendment to the standard fixes this.

Perhaps this is a whole other issue in itself. Tell me if you want me to raise a separate ticket, and I will.

joseph.thomson@… — Sat, 12 Dec 2015 21:35:19 GMT

p.s. Ignore the bit where I said "(it may be unlikely that they would do anything unsafe, but it's possible)"; I'm just confusing myself.

John Maddock — Sun, 13 Dec 2015 12:03:32 GMT

has_trivial_copy and its related traits do not adhere to your definition of "trivial" on some compilers.

I just checked them all with gcc 4.6.4, 4.7.4, 4.8.4, 4.9.2, 5.1.0, plus clang and Intel and they all do the correct thing - which is to say they all regard deleted operators as non-trivial. This is in 1.60 beta BTW. Also verified our tests check this. Note that all these traits have been completely rewritten for the next release.

The documentation for these traits says that they can be used to check whether memcpy is safe to use; this is incorrect, as is_trivially_copyable is the required condition.

We have never provided is_trivially_copyable, if we did it would likely be composed from these traits. Plus I believe you have this backwards - is_trivially_copy_constructible is the std trait - and there is a precondition on that trait that the type be copy-constructible, from C++14:

"For a referenceable type T, the same result as is_trivially_constructible<T, const T&>::value, otherwise false."

and then is_trivially_constructible says:

"is_constructible<T, Args...>::value is true and the variable definition for is_constructible, as defined below, is known to call no operation that is not trivial ( 3.9, 12)."

Which would rule out types with deleted copy-constructors from being trivially-copyable.

Your preferred definition of "trivial" is too conservative, and will result in some obviously memcpy-able types reporting false for is_trivially_copyable.

Such as? Note that has_trivial_copy (which I accept is badly named) refers only to copy-constructors. In your example:

struct X
{
    const int val;
};

If you care at all about const-correctness (and you should) then such a type really is non-trivial, or more specifically:

default construct    trivial
copy-construct       trivial
move-construct       trivial
destruct             trivial
copy-assign          not trivial or allowed
move-assign          not trivial or allowed

Which I believe is what 1.60 has implemented.

The current standard definition of is_trivially_copyable is potentially dangerous when types use certain OS-level resources; a pending amendment to the standard fixes this.

Right, IMO code should use traits which pertain to the actual operation that will be performed - for example boost::has_trivial_copy is closest to std::is_trivially_copy_constructible and both should return false for non-copyable types and avoid that nasty (and very real) black hole.

One final point: has_trivial_copy and friends were developed for C++03 (and later included in TR1 standard). There's a lot of code out there which relies on has_trivial_copy implies safe-to-memcpy instead of copy construct. That code should absolutely not be broken just because C++11 has introduced new features: ie deleted functions.

joseph.thomson@… — Sun, 13 Dec 2015 15:36:17 GMT

I just checked them all with gcc 4.6.4, 4.7.4, 4.8.4, 4.9.2, 5.1.0, plus clang and Intel and they all do the correct thing - which is to say they all regard deleted operators as non-trivial. This is in 1.60 beta BTW. Also verified our tests check this. Note that all these traits have been completely rewritten for the next release.

That's good!

One final point: has_trivial_copy and friends were developed for C++03 (and later included in TR1 standard). There's a lot of code out there which relies on has_trivial_copy implies safe-to-memcpy instead of copy construct. That code should absolutely not be broken just because C++11 has introduced new features: ie deleted functions.

I agree. It's a shame that Boost.TypeTraits' definition of "trivial" special functions doesn't match the language specification, but not breaking existing code is more important technically correct naming. Have you considered providing aliases (e.g. boost::is_copy_constructible) that match the standard library?

We have never provided is_trivially_copyable, if we did it would likely be composed from these traits. Plus I believe you have this backwards - is_trivially_copy_constructible is the std trait - and there is a precondition on that trait that the type be copy-constructible, from C++14:

"For a referenceable type T, the same result as is_trivially_constructible<T, const T&>::value, otherwise false."

and then is_trivially_constructible says:

"is_constructible<T, Args...>::value is true and the variable definition for is_constructible, as defined below, is known to call no operation that is not trivial ( 3.9, 12)." Which would rule out types with deleted copy-constructors from being trivially-copyable.

Classes with deleted copy constructors can be trivially copyable (as I will demonstrate). However, the issue I was trying to raise here was that trivially copy constructible types are not necessarily trivially copyable, so it is potentially undefined behaviour to memcpy them.

I think you may be confusing three different concepts:

triviality of special functions

[class.copy]

A copy/move constructor for class X is trivial if it is not user-provided, its parameter-type-list is equivalent to the parameter-type-list of an implicit declaration, and if

class X has no virtual functions (10.3) and no virtual base classes (10.1), and
class X has no non-static data members of volatile-qualified type, and
the constructor selected to copy/move each direct base class subobject is trivial, and
for each non-static data member of X that is of class type (or array thereof), the constructor selected to copy/move that member is trivial;
otherwise the copy/move constructor is non-trivial.

A copy/move assignment operator for class X is trivial if it is not user-provided, its parameter-type-list is equivalent to the parameter-type-list of an implicit declaration, and if

class X has no virtual functions (10.3) and no virtual base classes (10.1), and
class X has no non-static data members of volatile-qualified type, and
the assignment operator selected to copy/move each direct base class subobject is trivial, and
for each non-static data member of X that is of class type (or array thereof), the assignment operator selected to copy/move that member is trivial;
otherwise the copy/move assignment operator is non-trivial.

A destructor is trivial if it is not user-provided and if:

the destructor is not virtual,
all of the direct base classes of its class have trivial destructors, and
for all of the non-static data members of its class that are of class type (or array thereof), each such class has a trivial destructor.
Otherwise, the destructor is non-trivial.

[dcl.fct.def.default]

A function is user-provided if it is user-declared and not explicitly defaulted or deleted on its first declaration.

This all means that deleted special functions can be trivial. It doesn't matter how boost::has_trivial_copy behaves (you can make it do whatever you like); this is the standard's definition of what constitutes a trivial special function, and it is essential to when interpreting the standard's definition of "trivially copyable".

trivial copyability of classes

[class]

A trivially copyable class is a class that:

has no non-trivial copy constructors (12.8),
has no non-trivial move constructors (12.8),
has no non-trivial copy assignment operators (13.5.3, 12.8),
has no non-trivial move assignment operators (13.5.3, 12.8), and
has a trivial destructor (12.4).

Since deleted special function can be trivial, according to the standard, this means that this function is trivially copyable, however counter-intuitive it may be:

struct X
{
    X(X const&) = delete;
    X& operator=(X const&) = delete;
    X(X&&) = delete;
    X& operator=(X&&) = delete;
};

However, a proposal has been submitted to change the definition of "trivially copyable", because under the current definition, classes like std::atomic and std::mutex are trivially copyable.

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1734

This proposal suggests that "trivially copyable" classes have to have at least one non-deleted copy/move special functions (and that the destructor be non-deleted):

A trivially copyable class is a class:

where each copy constructor, move constructor, copy assignment operator, and move assignment operator (12.8, 13.5.3) is either deleted or trivial,
that has at least one non-deleted copy constructor, move constructor, copy assignment operator, or move assignment operator, and
that has a trivial, non-deleted destructor (12.4).

This would make the aforementioned class no longer trivially copyable. However, a trivially copyable class does not necessarily require a non-deleted copy constructor; any non-deleted special function will fulfil the requirements. Thus, this class would still be trivially copyable:

struct X
{
    const int val;
};

I took this example from a C++ standards committee document:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4460.html

So in their words:

So, why don't we just say that a deleted special member function is non-trivial? Alas, that would mean the following type becomes non-trivial:
struct X
{
    const int val;
};
X has a deleted copy assignment operator. That doesn't mean that X is not trivially copyable. And X has been trivial since the dawn of time, so we must be careful not to break such expectations.

trivial constructibility/assignability of classes

You already quoted the standard, so I don't have to.

For a class to be trivially copy constructible, it must be copy constructible, and the copy constructor must be trivial. Since you cannot copy construct an object using a deleted copy constructor, I agree that a class with a deleted copy constructor is not trivially copy constructible. However, copy constructibility is not a requirement of trivial copyability; the presence of a trivial copy constructor is (even a deleted one). Even under the proposed amendment, as long as there is at least one other non-deleted copy/move function, a class does not have to be copy constructible to be trivially copyable.

In conclusion...

In other words, both trivially copyable and trivially copy constructible classes must have trivial copy constructors (deleted copy constructors may be trivial), but only trivially copy constructible classes must be copy constructible.

But the most important thing is this:

[intro.object]

An object of trivially copyable or standard-layout type (3.9) shall occupy contiguous bytes of storage.

[basic.types]

For any trivially copyable type T, if two pointers to T point to distinct T objects obj1 and obj2, where neither obj1 nor obj2 is a base-class subobject, if the underlying bytes (1.7) making up obj1 are copied into obj2,43 obj2 shall subsequently hold the same value as obj1.
T* t1p;
T* t2p;
  // provided that t2p points to an initialized object ...
std::memcpy(t1p, t2p, sizeof(T));
  // at this point, every subobject of trivially copyable type in *t1p contains
  // the same value as the corresponding subobject in *t2p

Thus, using memcpy to copy a non-trivially copyable class would be undefined behaviour. Since trivially copy constructible classes are not necessarily trivially copyable, relying on boost::has_trivial_copy to tell you whether it's safe to use memcpy could lead to undefined behaviour. This is why I say that your documentation needs to be changed (and potentially, boost::is_trivially_copyable added).

Note that you could implement a boost::is_trivially_copyable that requires all copy/move special functions to be non-deleted. While this would be overly-conservative (e.g. the aforementioned X class would register as non-trivially copyable), it would at least be sufficient to check for memcpy-ability.