[PATCH] D77621: ADT: SmallVector size/capacity use word-size integers when elements are small

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:52
+  // The maximum size depends on size_type used.
+  static constexpr size_t SizeMax() {
+    return std::numeric_limits<Size_T>::max();
----------------
smeenai wrote:
> browneee wrote:
> > dexonsmith wrote:
> > > STL data structures have a name for this called `max_size()`.  Should we be consistent with that?
> > Good question.
> > 
> > This brought my attention to the existing SmallVectorTemplateCommon::max_size() which also needed to be updated.
> > I'm going to name this new function SizeTypeMax to best describe what it provides, and leave it separate from max_size().
> Was it intentional to make this return a `size_t` rather than a `Size_T`? Clang gives a truncation warning on 32-bit platforms when you try to instantiate the template with `uint64_t` as a result.
I think returning size_t is the correct thing here & the fix is not to use a 64 bit size on a 32 bit machine - that was initially intended to be solved by using uintptr_t, but got lost when that turned out to cause issues on 32 bit machines.

@browneee could you fix this differently, so that when sizeof(uintptr_t) == 4 there's only one instantiation/only uint32_t is used?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621