dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land.
Thanks for your patience, I missed the updates on Friday. I have a couple of optional comments inline that I don't feel strongly about. LGTM either way. In D77621#1972764 <https://reviews.llvm.org/D77621#1972764>, @dblaikie wrote: > Fair enough - that complexity seems reasonably acceptable to me if you reckon > the memory size benefits are still worthwhile (did you measure them on any > particular workloads? Do we have lots of fairly empty SmallVectors, etc?) if > they don't apply to smaller types like this? I haven't measured anything recently. Last I looked there were a number of SmallVectors inside other data structures (sometimes, sadly, SmallVector) on the heap (or stack). In some cases the main reason not to use `std::vector` is the exception pessimizations. It's nice to keep them small if it's reasonable to. ================ Comment at: llvm/include/llvm/ADT/SmallVector.h:52 + // The maximum size depends on size_type used. + static constexpr size_t SizeMax() { + return std::numeric_limits<Size_T>::max(); ---------------- STL data structures have a name for this called `max_size()`. Should we be consistent with that? ================ Comment at: llvm/include/llvm/ADT/SmallVector.h:179 + using Base::empty; + using Base::size; + ---------------- Optionally we could expose `max_size()` as well. 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
