[PATCH] D92522: [SmallVector] Allow SmallVector<T>

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 2 16:22:24 PST 2020


dblaikie added a comment.

I'm generally good with this in terms of incremental forward progress. Still don't feel super great about the "at least 1" thing and the disparate storage limits (preferred size versus break-because-too-big size), but given the varied opinions, seems like the incremental progress is probably the best path forward for now. (perfect being the enemy of good and all that)



================
Comment at: llvm/include/llvm/ADT/SmallVector.h:930
+  size_t PreferredInlineBytes =
+      kPreferredSmallVectorSizeof - sizeof(SmallVectorImpl<T>);
+  size_t NumElementsThatFit = PreferredInlineBytes / sizeof(T);
----------------
Could/should this be `sizeof(SmallVector<T, 0>)`? So that if we move members around for any reason (it's SmallVector, so it's not likely to get willy-nilly refactoring of this kind, but just on principle) this would still correctly capture the size of the SmallVector base footprint before inline buffer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92522



More information about the llvm-commits mailing list