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

[Sean Silva via Phabricator via llvm-commits]

silvas added inline comments.


================
Comment at: llvm/unittests/ADT/SmallVectorTest.cpp:1013
+    TestDefaultSmallSize<int32_t>, TestDefaultSmallSize<int64_t>,
+    TestDefaultSmallSize<void *>, TestDefaultSmallSize<BigElementType>>;
+
----------------
mehdi_amini wrote:
> FWIW I am still unconvinced that it is a good idea to use the default size on large objects. I'm not super comfortable with the implicit `0` as that seems to encourage mis-uses of the SmallVector.
I don't feel strongly about this, except on the usability side. But maybe that's a minor concern, as very large types are probably not that common.

One real issue is that rejecting SmallSize==0 could result in unexpected static_assert's across platforms: sizeof(header) and sizeof(ValueType) will differ across platforms, and both affect whether the default small size will be 0.

I don't see an obvious solution to that, and it seems like it could cause bot breakages. Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90884