[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:25:23 PST 2020


silvas added a comment.

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

> In D90884#2377574 <https://reviews.llvm.org/D90884#2377574>, @silvas wrote:
>
>> 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
>
> My take is that if you have a large type and you want a SmallVector,, you should be explicit about the storage. I rather read `SmallVector<LargeType, 4>`  than `SmallVector<LargeType>` and have implicitly N=0.

That makes sense. As a matter of practicality, I think I agree with Duncan here that N=0 silently isn't too bad. (though I'm still wary of making N=0 the default).

> I'm not sure I understand the size correctly, but right now any type > 24B would yield a N=0, wouldn't it?

It depends on the header size (will change that to an absolute size, per Duncan's suggestion).


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