[PATCH] D54472: Disable invalid isPodLike<> specialization

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith 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> {
----------------
chandlerc wrote:
> Quuxplusone wrote:
> > 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.
> While this might be an independently nice improvement, I agree w/ the current mechanics of the patch to make the diff more isolated.
It doesn't seem completely independent to me, because this patch introduces repeated instances of (reader's) ambiguity between is_trivially_copyable and std::is_trivially_copyable.

Can the mechanical change suggested by @Quuxplusone be implemented as an initial NFC commit on isPodLike (and then this patch rebased on top)?  If so, that would avoid cluttering this diff.


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

https://reviews.llvm.org/D54472