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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D77621#2013400 <https://reviews.llvm.org/D77621#2013400>, @browneee wrote:

> Thanks for the tips, MaskRay.
>
> Yes, I expect this would fix that issue.
>
> smeenai, SizeTypeMax() is intended to return size_t.
>
>  ---
>
> I see a couple of options for fixing the truncation warning on 32-bit platforms:
>
> 1. Add an explicit cast to remove the warning.
>   - Disadvantage: the second instantiation still exists even though it is unused.
>
>     ``` static constexpr size_t SizeTypeMax() { return static_cast<size_t>(std::numeric_limits<Size_T>::max()); } ```
> 2. Use a std::conditional to swap the type of one instantiation to avoid conflicts.
>
>   In this case I'd probably swap back to using uintptr_t and disable the uint32_t on 32bit.
>   - Disadvantage: the second instantiation still exists even though it is unused.
>
>     ``` // Will be unused when instantiated with char. // This is to avoid instantiation for uint32_t conflicting with uintptr_t on 32-bit systems. template class llvm::SmallVectorBase<std::conditional<sizeof(void *) != 4, uint32_t, char>::type>; template class llvm::SmallVectorBase<uintptr_t>; ```
> 3. Use preprocessor to disable one of the instantiations on 32-bit platforms.
>
>   In this case I'd probably swap back to using uintptr_t and disable the uint32_t on 32bit.
>   - Disadvantage: uses preprocessor
>   - Disadvantage: potential for portability issues with different platforms lacking certain macros
>
>     ``` #if __SIZEOF_POINTER__ != 4  && !defined(_WIN32) && !defined(__ILP32) template class llvm::SmallVectorBase<uint32_t>; #endif template class llvm::SmallVectorBase<uintptr_t>; ```
>
>     My order of preference would be 1, 2, 3.
>
>     Is there another solution I've missed? Thoughts on which is best? @dblaikie


I don't think (3) needs to be non-portable - it could use SIZE_MAX to test if it's bigger than 2^32? (or == 2^64)? That seems like it'd be a pretty good direct test? & ensure the types written in the explicit specializations are the same types written in the header/std::conditional (seems a bit questionable to rely on uintptr_t being necessarily the same type as either uint32_t or uint64_t - but maybe that's guaranteed/written down somewhere)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621