update FOLLY_IS_TRIVIALLY_COPYABLE

Summary:
Let's make sure that boost version is intented to mean exactly
the same as `std::is_trivially_copyable` (see 9 [class] / 6) to avoid any
confusion (current boost path is more close to
`std::is_trivially_copy_constructible` than to
`std::is_trivially_copyable`;  UPD: unfortunately, old versions of boost
don't support `boost::has_trivial_move_constructor` and friends, so I
can't completely mimic `std::is_trivially_copyable` here).

As mentioned in the original comment, `__has_trivial_copy` returns 1 in
case of deleted copy ctor in clang (contradicts 12.8 [class.copy] / 13 +
8.4.2 [dcl.fct.def.default] / 4), which makes `FOLLY_IS_TRIVIALLY_COPYABLE`
accept `std::unique_ptr<>`; using `boost::has_trivial_destructor` would
at least save us from the problems in this case.

Alternative "solution" may be to rename `FOLLY_IS_TRIVIALLY_COPYABLE` to
`FOLLY_IS_TRIVIALLY_COPY_CONSTRUCTIBLE`, and make sure
`folly::small_vector` won't call dtor when memcopies the data around.

Test Plan: fbconfig --clang folly/test:small_vector_test && fbmake runtests_opt (fails in trunk, passes with this diff)

Reviewed By: pgriess@fb.com

FB internal diff: D1216158

diff --git a/folly/Portability.h b/folly/Portability.h
index b227ba0da9ecf2f9018b7481bb67f870ab584c9a..ab41ef4ce2a77a37e02e8cc97fd4b2d49a66eee1 100644 (file)
--- a/folly/Portability.h
+++ b/folly/Portability.h
@@ -123,10 +123,13 @@ struct MaxAlign { char c; } __attribute__((aligned));
 // to Boost otherwise.
 #if FOLLY_HAVE_STD__IS_TRIVIALLY_COPYABLE
 #include <type_traits>
 // to Boost otherwise.
 #if FOLLY_HAVE_STD__IS_TRIVIALLY_COPYABLE
 #include <type_traits>

-#define FOLLY_IS_TRIVIALLY_COPYABLE(T)      (std::is_trivially_copyable<T>::value)
+#define FOLLY_IS_TRIVIALLY_COPYABLE(T)                   \
+  (std::is_trivially_copyable<T>::value)

 #else
 #include <boost/type_traits.hpp>
 #else
 #include <boost/type_traits.hpp>

-#define FOLLY_IS_TRIVIALLY_COPYABLE(T)      (boost::has_trivial_copy<T>::value)
+#define FOLLY_IS_TRIVIALLY_COPYABLE(T)                   \
+  (boost::has_trivial_copy<T>::value &&                  \
+   boost::has_trivial_destructor<T>::value)

 #endif
 #endif // __cplusplus
 
 #endif
 #endif // __cplusplus
 

diff --git a/folly/test/small_vector_test.cpp b/folly/test/small_vector_test.cpp
index 7c1d884948e5946228dee5fe011fecad4ce8b52b..200257b77e5c06f03fbdd3f6d62b68bfcbd060bd 100644 (file)
--- a/folly/test/small_vector_test.cpp
+++ b/folly/test/small_vector_test.cpp
@@ -63,6 +63,9 @@ static_assert(sizeof(small_vector<int16_t,4,NoHeap,uint16_t,
 
 #endif
 
 
 #endif
 

+static_assert(!FOLLY_IS_TRIVIALLY_COPYABLE(std::unique_ptr<int>),
+              "std::unique_ptr<> is trivially copyable");
+

 namespace {
 
 struct NontrivialType {
 namespace {
 
 struct NontrivialType {