Re: [Boost-bugs] [Boost C++ Libraries] #11835: boost::has_trivial_copy is incorrect on Clang

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #11835: boost::has_trivial_copy is incorrect on Clang
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2015-12-12 21:05:31

#11835: boost::has_trivial_copy is incorrect on Clang
-------------------------+-------------------------------------------------
  Reporter:              |      Owner:  johnmaddock
  joseph.thomson@…       |     Status:  closed
      Type:  Bugs        |  Component:  type_traits
 Milestone:  To Be       |   Severity:  Problem
  Determined             |   Keywords:  has_trivial_copy clang memcpy
   Version:  Boost       |  is_trivially_copyable
  1.59.0                 |
Resolution:  wontfix     |
-------------------------+-------------------------------------------------

Comment (by joseph.thomson@…):

 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.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/11835#comment:4>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.