[PATCH] D90884: [SmallVector] Add a default small size.

Sean Silva via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 5 15:10:01 PST 2020


silvas added a comment.

In D90884#2377565 <https://reviews.llvm.org/D90884#2377565>, @mehdi_amini wrote:

> In D90884#2377537 <https://reviews.llvm.org/D90884#2377537>, @rnk wrote:
>
>>> I'm not sure it makes sense to every get a default of a very small N in general. I guess a default like here makes sense for smaller object, what about `static_assert` if `sizeof(T)>sizeof(void*)`?
>>
>> I'm not sure I understand this suggestion. There are lots of `SmallVector<LargeTy>` instantiations.
>
> Sorry I wasn't clear: I didn't mean to prevent `SmallVector<LargeTy, N>` but I was trying to indicate that it may not be a good default for N to be 0 ever. So I would do something like:
>
>   template <typename ElementTy> constexpr size_t calculateDefaultSmallSize() {
>     static_assert(sizeof(ElementTy) < 8, "Please provide a default value for SmallVector when the element type is \"large\"");
>     ...

That seems artificially restrictive. What "bug" are you trying to prevent with that static_assert? It just seems to be a pain for users. Also <8 is way too strict. Maybe <=128 or something. Large types are not uncommon


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90884



More information about the llvm-commits mailing list