[PATCH] D54472: Disable invalid isPodLike<> specialization

[serge via Phabricator via llvm-commits]

serge-sans-paille marked an inline comment as done.
serge-sans-paille 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> {
----------------
dexonsmith wrote:
> 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.
Patch proposed in https://reviews.llvm.org/D55005


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

https://reviews.llvm.org/D54472