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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
