[PATCH] D54472: Disable invalid isPodLike<> specialization

[serge via Phabricator via llvm-commits]

serge-sans-paille added inline comments.


================
Comment at: include/llvm/ADT/PointerIntPair.h:132
+  static constexpr bool value = true;
+#if !defined(NDEBUG) && (__has_feature(is_trivially_copyable) || (defined(__GNUC__) && __GNUC__ >= 5))
+  static_assert(std::is_trivially_copyable<PointerIntPair<PointerTy, IntBits, IntType, PtrTraits, Info>>::value, "inconsistent behavior between llvm:: and std:: is_trivially_copyable");
----------------
rsmith wrote:
> Why is this guarded by `NDEBUG`?
To limit compile-time overhead.


================
Comment at: include/llvm/Support/type_traits.h:169-175
+template <typename T>
+struct is_trivially_copyable<T*> {
+  static constexpr bool value = true;
+#if !defined(NDEBUG) && (__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");
+#endif
+};
----------------
rsmith wrote:
> chandlerc wrote:
> > Why is this needed? Due to incomplete types? (answer in a comment please!)
> The comment still doesn't explain why you need this. Why would the above implementation not work for pointers to incomplete types? (As far as I can see, the above implementation should work fine in that case, and works fine in practice: https://godbolt.org/z/FJ09dy)
That definitively was a left over. I removed it in the updated patch.


================
Comment at: include/llvm/Support/type_traits.h:168
+
+#if !defined(NDEBUG) && (__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");
----------------
rsmith wrote:
> Why check `NDEBUG` here?
It's debug code, to ensure the consistency of `llvm::is_trivially_copyable` with `std::` version. I assume ignoring this on release build make them faster.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54472/new/

https://reviews.llvm.org/D54472