dblaikie added inline comments.
================
Comment at: llvm/include/llvm/ADT/SmallVector.h:53
+ // The maximum size depends on size_type used.
+ size_t SizeMax() { return size_type(-1ULL); }
----------------
dblaikie wrote:
> I'd probably use numeric_limits here & make this static constexpr
Making it a constexpr /variable/ gets a bit more complicated - do you happen to
know off-hand whether this requires a separate/out-of-line definition if it's
ODR used (ah, seems it does - in C++14 which LLVM Uses, in 17 ("A constexpr
specifier used in a function or static member variable (since C++17)
declaration implies inline.") it would be OK)
I think it's best not to leave a trap that might catch someone up in the future
if SizeMax ends up getting ODR used (eg: in a call to std::less, rather than an
explicit op<, etc) & then the lack of definition would lead to linking failure,
etc. So best to leave it as a static constexpr function rather than static
constexpr variable.
================
Comment at: llvm/lib/Support/SmallVector.cpp:40-47
+// Check that SmallVector with 1 byte elements takes extra space due to using
+// uintptr_r instead of uint32_t for size and capacity.
+static_assert(sizeof(SmallVector<char, 4>) > sizeof(SmallVector<uint32_t, 1>)
||
+ sizeof(uintptr_t) == sizeof(uint32_t),
+ "1 byte elements should cause larger size and capacity type");
+static_assert(sizeof(SmallVector<uint32_t, 2>) ==
+ sizeof(SmallVector<int64_t, 1>),
----------------
I think it's probably best to assert the size more like the above (but using
zero-sized inline buffer), rather than using relative sizes - seems like it'd
be more readable to me at least. Maybe @dexonsmith has a perspective here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77621/new/
https://reviews.llvm.org/D77621
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
