dblaikie accepted this revision. dblaikie added a comment. In D77621#2001099 <https://reviews.llvm.org/D77621#2001099>, @dexonsmith wrote:
> In D77621#2000237 <https://reviews.llvm.org/D77621#2000237>, @nikic wrote: > > > Okay, I'm basically fine with that, if it is our stance that SmallVector > > should always be preferred over std::vector. Not really related to this > > revision, but it would probably help to do some renaming/aliasing to > > facilitate that view. Right now, the number of `SmallVector<T, 0>` uses in > > LLVM is really small compared to the `std::vector<T>` uses (100 vs 6000 > > based on a not very accurate grep). I think part of that is in the name, > > and calling it `using Vector<T> = SmallVector<T, 0>` and `using > > VectorImpl<T> = SmallVectorImpl<T>` would make it a lot more obvious that > > this is our preferred general purpose vector type, even if the stored data > > is not small. > > > Those aliases SGTM. I'd be slightly against, just because having a name that differs from the standard name only in case seems pretty subtle - that and momentum, we've had SmallVector around for a while & I think it's OK. I don't mind some places using std::vector either, though. Don't feel strongly enough that I'd outright stand against such an alias/change, but just expressing this amount of disfavor. In D77621#2001378 <https://reviews.llvm.org/D77621#2001378>, @nikic wrote: > So tl;dr looks like as long as we keep `grow_pod` outside the header file, > this change seems to be approximately free in terms of compile-time and > memory usage both. Awesome - thanks for looking into it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77621/new/ https://reviews.llvm.org/D77621 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
