[PATCH] D54472: Disable invalid isPodLike<> specialization
Arthur O'Dwyer via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 30 18:18:08 PST 2018
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
More information about the llvm-commits
mailing list