[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