[PATCH] D54472: Disable invalid isPodLike<> specialization

Arthur O'Dwyer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 27 22:15:01 PST 2018


Quuxplusone added inline comments.


================
Comment at: include/llvm/ADT/SmallVector.h:185
+/// put method implementations that are designed to work with non-POD-like T's.
+template <typename T, bool is_trivially_copyable>
 class SmallVectorTemplateBase : public SmallVectorTemplateCommon<T> {
----------------
Incidentally, unless there's a portability issue I'm not aware of, I would prefer to write this as

    template <typename T, bool = is_trivially_copyable<T>::value>
    class SmallVectorTemplateBase : public SmallVectorTemplateCommon<T> {

so that on line ~322 you could write simply

    template <typename T>
    class SmallVectorImpl : public SmallVectorTemplateBase<T> {
      using SuperClass = SmallVectorTemplateBase<T>;

without repeating your `is_trivially_copyable<T>::value` all over the place.

The more times you have to manually type out `is_trivially_copyable<T>::value`, the greater the chance you'll slip up and write `std::is_trivially_copyable<T>::value` on autopilot.


================
Comment at: include/llvm/ADT/bit.h:45
+// This case is GCC 4.x. clang with libc++ or libstdc++ never get here. Unlike
+// llvm/Support/type_traits.h's is_trivially_constructible we don't want to
+// provide a good-enough answer here: developers in that configuration will hit
----------------
Should this say "`is_trivially_copyable`"?


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

https://reviews.llvm.org/D54472





More information about the llvm-commits mailing list