Fix lvm::is_trivially_copyable portability issues

[serge-sans-paille]

Summary

llvm::is_trivially_copyable portability is verified at compile time using std::is_trivially_copyable as the reference implementation. Unfortunately, the latter is not available on all platforms, so introduce a proper configure check to detect if it is available on the target platform.

In a similar manner, std::is_copy_assignable is not fully supported for gcc4.9. Provide a portable (?) implementation instead.

[fedor.sergeev]

Sorry, now it fails on is_move_assignable:

] cat copyable_with_llvm.cpp
#include "llvm/Support/type_traits.h"
// code from tools/llvm-xray/xray-converter.cpp
template <typename AssociatedData> struct TrieNode {
  int FuncId;
  AssociatedData ExtraData;
};
struct StackIdData {
  // to get is_trivially_copyable instantiation from within the class
  int x [llvm::is_trivially_copyable<TrieNode<StackIdData> *>::value + 1];
};
] g++ -std=c++11 -I ../../llvm/include -I include copyable_with_llvm.cpp 
copyable_with_llvm.cpp: In instantiation of ‘struct TrieNode<StackIdData>’:
/gcc/include/c++/4.9.2/type_traits:1183:45:   required by substitution of ‘template<class _Tp1, class _Up1, class> static std::true_type std::__is_assignable_helper<_Tp, _Up>::__test(int) [with _Tp1 = llvm::detail::trivial_helper<TrieNode<StackIdData>*>&; _Up1 = llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&; <template-parameter-1-3> = <missing>]’
/gcc/include/c++/4.9.2/type_traits:1192:30:   required from ‘class std::__is_assignable_helper<llvm::detail::trivial_helper<TrieNode<StackIdData>*>&, llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&>’
/gcc/include/c++/4.9.2/type_traits:1197:12:   required from ‘struct std::is_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*>&, llvm::detail::trivial_helper<TrieNode<StackIdData>*>&&>’
/gcc/include/c++/4.9.2/type_traits:1227:12:   required from ‘struct std::__is_move_assignable_impl<llvm::detail::trivial_helper<TrieNode<StackIdData>*>, true>’
/gcc/include/c++/4.9.2/type_traits:1233:12:   required from ‘struct std::is_move_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:157:25:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::has_trivial_move_assign’
../../llvm/include/llvm/Support/type_traits.h:170:32:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::value’
copyable_with_llvm.cpp:9:64:   required from here
copyable_with_llvm.cpp:5:18: error: ‘TrieNode<AssociatedData>::ExtraData’ has incomplete type
   AssociatedData ExtraData;
                  ^
copyable_with_llvm.cpp:7:8: error: forward declaration of ‘struct StackIdData’
 struct StackIdData {
        ^

And yes, the same failure on the actual llvm build on xray-converter.cpp.

[fedor.sergeev]

include/llvm/Support/type_traits.h

125

Somehow this declval<F> thing does not look right. Doesnt it have F&& type?
Besides, the result of assignment might as well be a reference, which will reject forming a pointer on its decltype.

[serge-sans-paille]

A few comments missing but I'd like to know if this now works correctly on gcc 4.9, @fedor.sergeev , @Anastasia and @sylvestre.ledru . I successfully passed test on a dedicated setup so I guess it's ok...

[fedor.sergeev]

it does *not* work on my shorter reproducer:

] g++ -std=c++11 -I ../../llvm/include -I include ../../llvm/copyable_with_llvm.cpp 
../../llvm/copyable_with_llvm.cpp: In instantiation of ‘struct TrieNode<StackIdData>’:
../../llvm/include/llvm/Support/type_traits.h:118:34:   required by substitution of ‘template<class T> using IsCopyAssignableImpl = decltype ((declval<T&>)()=(declval<const T&>)()) [with T = llvm::detail::trivial_helper<TrieNode<StackIdData>*>]’
../../llvm/include/llvm/Support/type_traits.h:114:8:   required from ‘struct llvm::detail::IsDetected<llvm::detail::IsCopyAssignableImpl, llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:149:8:   required from ‘struct llvm::is_copy_assignable<llvm::detail::trivial_helper<TrieNode<StackIdData>*> >’
../../llvm/include/llvm/Support/type_traits.h:174:25:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::has_trivial_copy_assign’
../../llvm/include/llvm/Support/type_traits.h:195:32:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<StackIdData>*>::value’
../../llvm/copyable_with_llvm.cpp:12:64:   required from here
../../llvm/copyable_with_llvm.cpp:7:18: error: ‘TrieNode<AssociatedData>::ExtraData’ has incomplete type
   AssociatedData ExtraData;
                  ^
../../llvm/copyable_with_llvm.cpp:10:8: error: forward declaration of ‘struct StackIdData’
 struct StackIdData {
        ^

Started full build, will report as soon as it is ready.

[fedor.sergeev]

In D57018#1365764, @fedor.sergeev wrote:

Started full build, will report as soon as it is ready.

On xray-converter.cpp fails the same way as on my shorter reproducer.

[fedor.sergeev]

Both the small reproducer and full LLVM-only builds/tests passed with my gcc 4.9.
LGTM

[xbolva00]

cmake/config-ix.cmake

328

Typo

[serge-sans-paille]

Slightly cleaner implementation, added comments and tested with polly.

[TWeaver]

Hi there,

we're seeing build failures on our internal CI as a result of the issue this patch aims to fix.

hope I don't seem too pushy but, are there any plans to land this in the next few hours?

we'd like to see our CI back up and running as soon as possible to avoid any further headaches.

Thanks in advance
Tom W

[serge-sans-paille]

Yeah, I do a final test round and I'll push that one. It's a very touchy piece of code, so I don't want to make all builders fail again...

[uabelho]

We've been having issues with std::is_trivially_copyable too, and it seems not even this patch solves the problems for us.

I'm running an Ubuntu 14.04 machine with clang 3.6.0 and libstdc++-4.8-dev and now I get the following when I compile on trunk:

In file included from ../utils/TableGen/CodeGenSchedule.cpp:14:
In file included from ../utils/TableGen/CodeGenSchedule.h:18:
In file included from ../include/llvm/ADT/DenseMap.h:16:
In file included from ../include/llvm/ADT/DenseMapInfo.h:16:
In file included from ../include/llvm/ADT/ArrayRef.h:12:
In file included from ../include/llvm/ADT/Hashing.h:48:
In file included from ../include/llvm/Support/Host.h:16:
In file included from ../include/llvm/ADT/StringMap.h:16:
In file included from ../include/llvm/ADT/StringRef.h:12:
In file included from ../include/llvm/ADT/STLExtras.h:19:
In file included from ../include/llvm/ADT/Optional.h:21:
../include/llvm/Support/type_traits.h:127:57: error: object of type 'llvm::detail::trivial_helper<std::pair<void *, unsigned long> >' cannot

  be assigned because its copy assignment operator is implicitly deleted
static auto get(F*) -> decltype(std::declval<T &>() = std::declval<const T &>(), std::true_type{});
                                                    ^

../include/llvm/Support/type_traits.h:162:7: note: in instantiation of template class

'llvm::is_copy_assignable<llvm::detail::trivial_helper<std::pair<void *, unsigned long> > >' requested here
is_copy_assignable<detail::trivial_helper<T>>::value;
^

../include/llvm/ADT/SmallVector.h:184:30: note: in instantiation of template class 'llvm::is_trivially_copyable<std::pair<void *, unsigned

long> >' requested here

template <typename T, bool = is_trivially_copyable<T>::value>

^

../include/llvm/ADT/SmallVector.h:321:32: note: in instantiation of default argument for 'SmallVectorTemplateBase<std::pair<void *, unsigned

long> >' required here

class SmallVectorImpl : public SmallVectorTemplateBase<T> {

^~~~~~~~~~~~~~~~~~~~~~~~~~

../include/llvm/ADT/SmallVector.h:845:28: note: in instantiation of template class 'llvm::SmallVectorImpl<std::pair<void *, unsigned long> >'

requested here

class SmallVector : public SmallVectorImpl<T>, SmallVectorStorage<T, N> {

^

../include/llvm/Support/Allocator.h:373:45: note: in instantiation of template class 'llvm::SmallVector<std::pair<void *, unsigned long>, 0>'

    requested here
SmallVector<std::pair<void *, size_t>, 0> CustomSizedSlabs;
                                          ^

../include/llvm/Support/type_traits.h:96:7: note: copy assignment operator of 'trivial_helper<std::pair<void *, unsigned long> >' is

  implicitly deleted because variant field 't' has a non-trivial copy assignment operator
T t;
  ^

[uabelho]

Just saw build-bots failed the same way

[TWeaver]

seems like the best thing to do here is revert the original commit that broke the bots in the first place?

[TWeaver]

and thanks for the patch either way, nice to know we're not in this alone.

[TWeaver]

Slight fix for r351820 seems to have done the trick, many thanks!

[serge-sans-paille]

@uabelho top-of-tree should be fine now.

[uabelho]

In D57018#1366287, @serge-sans-paille wrote:

@uabelho top-of-tree should be fine now.

Yep, thanks!

[v.g.vassilev]

llvm/trunk/include/llvm/Support/type_traits.h

186 ↗

(On Diff #182898)

Hi @serge-sans-paille, we hit this assert when compiling llvm9 on windows (latest version (v16.8.2) of VS 2019):

llvm/Support/type_traits.h(186,23): error C2338: inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable (compiling source file RTDyldMemoryManager.cpp)
llvm/ADT/SmallVector.h(315,32): error C2976: 'llvm::SmallVectorTemplateBase': too few template arguments (compiling source file RTDyldMemoryManager.cpp)

Do you know if this is a bug in msvc or we need to improve something in that area?

[rogerjarrett]

llvm/trunk/include/llvm/Support/type_traits.h

186 ↗

(On Diff #182898)

I am experiencing the same error compiling LLVM 12.0.0 with VC19 version 14.27.29110 any updates ?

[rogerjarrett]

llvm/trunk/include/llvm/Support/type_traits.h

186 ↗

(On Diff #182898)

Mistake on my part:I am experiencing the same error compiling LLVM 10.0.1 with VC19 version 14.27.29110 any updates ? Appears to be related to MSVC compiler version

cmake/config-ix.cmake

if( LLVM_ENABLE_FFI )
   list(REMOVE_ITEM CMAKE_REQUIRED_INCLUDES ${FFI_INCLUDE_PATH})
   list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES ${FFI_LIBRARY_PATH})
 else()
   unset(HAVE_FFI_FFI_H CACHE)
   unset(HAVE_FFI_H CACHE)
   unset(HAVE_FFI_CALL CACHE)
 endif( LLVM_ENABLE_FFI )
 
+check_cxx_source_compiles("
+#include <type_traits>
+struct some_type {};
+static_assert(std::is_trivially_copyable<some_type>::value, \"ok\");
+int main() { return 0;}"
+HAVE_STD_IS_TRIVIALLY_COPYABLE)
+
+
 # Define LLVM_HAS_ATOMICS if gcc or MSVC atomic builtins are supported.
 include(CheckAtomic)
 
 if( LLVM_ENABLE_PIC )
   set(ENABLE_PIC 1)
 else()
   set(ENABLE_PIC 0)
   check_cxx_compiler_flag("-fno-pie" SUPPORTS_NO_PIE_FLAG)

include/llvm/Config/config.h.cmake

 #cmakedefine PACKAGE_VERSION "${PACKAGE_VERSION}"
 
 /* Define to the vendor of this package. */
 #cmakedefine PACKAGE_VENDOR "${PACKAGE_VENDOR}"
 
 /* Define as the return type of signal handlers (`int' or `void'). */
 #cmakedefine RETSIGTYPE ${RETSIGTYPE}
 
+/* Define if std::is_trivially_copyable is supported */
+#cmakedefine HAVE_STD_IS_TRIVIALLY_COPYABLE ${HAVE_STD_IS_TRIVIALLY_COPYABLE}
+
 /* Define to a function implementing stricmp */
 #cmakedefine stricmp ${stricmp}
 
 /* Define to a function implementing strdup */
 #cmakedefine strdup ${strdup}
 
 /* Whether GlobalISel rule coverage is being collected */
 #cmakedefine01 LLVM_GISEL_COV_ENABLED
 
 /* Define to the default GlobalISel coverage file prefix */
 #cmakedefine LLVM_GISEL_COV_PREFIX "${LLVM_GISEL_COV_PREFIX}"
 
 #endif

include/llvm/Support/type_traits.h

 struct is_trivially_move_constructible
     : std::is_move_constructible<
           ::llvm::detail::move_construction_triviality_helper<T>> {};
 template <typename T>
 struct is_trivially_move_constructible<T &> : std::true_type {};
 template <typename T>
 struct is_trivially_move_constructible<T &&> : std::true_type {};
 
+template <typename T>
+class is_copy_assignable {
+  template<class F>
+  static std::true_type apply(decltype(std::declval<F>()=std::declval<F>())*);
+  static std::false_type apply(...);
+  public:
+  static constexpr bool value = decltype(apply((T*)nullptr))::value;
+};
+
 // An implementation of `std::is_trivially_copyable` since STL version
 // is not equally supported by all compilers, especially GCC 4.9.
 // Uniform implementation of this trait is important for ABI compatibility
 // as it has an impact on SmallVector's ABI (among others).
 template <typename T>
 class is_trivially_copyable {
 
   // copy constructors
   static constexpr bool has_trivial_copy_constructor =
       std::is_copy_constructible<detail::trivial_helper<T>>::value;
   static constexpr bool has_deleted_copy_constructor =
       !std::is_copy_constructible<T>::value;
 
   // move constructors
   static constexpr bool has_trivial_move_constructor =
       std::is_move_constructible<detail::trivial_helper<T>>::value;
   static constexpr bool has_deleted_move_constructor =
       !std::is_move_constructible<T>::value;
 
   // copy assign
   static constexpr bool has_trivial_copy_assign =
-      std::is_copy_assignable<detail::trivial_helper<T>>::value;
+      is_copy_assignable<detail::trivial_helper<T>>::value;
   static constexpr bool has_deleted_copy_assign =
-      !std::is_copy_assignable<T>::value;
+      !is_copy_assignable<T>::value;
 
   // move assign
   static constexpr bool has_trivial_move_assign =
       std::is_move_assignable<detail::trivial_helper<T>>::value;
   static constexpr bool has_deleted_move_assign =
       !std::is_move_assignable<T>::value;
 
   // destructor
   static constexpr bool has_trivial_destructor =
       std::is_destructible<detail::trivial_helper<T>>::value;
 
   public:
 
   static constexpr bool value =
       has_trivial_destructor &&
       (has_deleted_move_assign || has_trivial_move_assign) &&
       (has_deleted_move_constructor || has_trivial_move_constructor) &&
       (has_deleted_copy_assign || has_trivial_copy_assign) &&
       (has_deleted_copy_constructor || has_trivial_copy_constructor);
 
-#if (__has_feature(is_trivially_copyable) || (defined(__GNUC__) && __GNUC__ >= 5))
-  static_assert(value == std::is_trivially_copyable<T>::value, "inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable");
+#ifdef HAVE_STD_IS_TRIVIALLY_COPYABLE
+  static_assert(value == std::is_trivially_copyable<T>::value,
+                "inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable");
 #endif
 };
 
 
 } // end namespace llvm
 
 // If the compiler supports detecting whether a class is final, define
 // an LLVM_IS_FINAL macro. If it cannot be defined properly, this