[PATCH] D54472: Disable invalid isPodLike<> specialization

[Arthur O'Dwyer via Phabricator via llvm-commits]

Quuxplusone added inline comments.


================
Comment at: include/llvm/ADT/SmallVector.h:333
   explicit SmallVectorImpl(unsigned N)
-      : SmallVectorTemplateBase<T, isPodLike<T>::value>(N) {}
+      : SmallVectorTemplateBase<T, is_trivially_copyable<T>::value>(N) {}
 
----------------
The `, is_trivially_copyable<T>::value` can safely be removed here. Sorry I missed it on the other review.


================
Comment at: include/llvm/Analysis/BlockFrequencyInfoImpl.h:188
 
-    BlockNode() = default;
+    BlockNode() noexcept(true) = default;
     BlockNode(IndexType Index) : Index(Index) {}
----------------
Unintended diff? Also, surely `noexcept(true)` a.k.a. `noexcept` would be the default here anyway. If anything wants `noexcept`, it'd be line 189, not line 188. :)


================
Comment at: include/llvm/Support/type_traits.h:163
+      (has_trivial_copy_constructor || has_trivial_copy_assign ||
+       has_trivial_move_constructor || has_trivial_move_assign);
+
----------------
This last clause — `(has_trivial_copy_constructor || has_trivial_copy_assign || has_trivial_move_constructor || has_trivial_move_assign)` — does not correspond to anything in the Standard. So:

https://godbolt.org/z/R5hCff
```
struct S {
    S(S&&) = delete;
};
static_assert(is_trivially_copyable<S>::value);
```
```
error: static_assert failed "consistent behavior"
  static_assert(value == std::is_trivially_copyable<T>::value, "consistent behavior");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
I'm not sure but I would suggest just getting rid of this last clause.


================
Comment at: tools/clang/lib/Sema/SemaChecking.cpp:11553
     public:
-      Seq() = default;
+      Seq() : Index(0) {}
     };
----------------
Unintended diff? But harmless.


================
Comment at: unittests/ADT/ArrayRefTest.cpp:252
 
+#if __has_feature(is_trivially_copyable) || (defined(__GNUC__) && __GNUC__ >= 5)
+static_assert(is_trivially_copyable<ArrayRef<int>>::value,
----------------
Why does this need the guard? You're not using the `std::` version here.
(And likewise on the next 7-or-so diffs.)


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

https://reviews.llvm.org/D54472