[PATCH] D39263: [support] remove tautological comparison in Support/Windows/Path.inc

[Bob Haarman via Phabricator via llvm-commits]

inglorion added a comment.

@zturner do you want me to make the semantic changes in the same commit or can we ship the size check removal first and then change the functionality? The latter would be my preference.



================
Comment at: llvm/lib/Support/Windows/Path.inc:726
-  if (Size > std::numeric_limits<SIZE_T>::max())
-    return make_error_code(errc::invalid_argument);
-
----------------
rnk wrote:
> amccarth wrote:
> > I assume someone was originally worried that `SIZE_T` would be 32 bits wide in a 32-bit build but somehow believed `std::size_t` (which is the type of Size) could still somehow be larger than that.  Ain't gonna happen.
> I wonder if we should turn these examples of dynamic range checks into static_asserts. We saw a lot of these tautological comparison warnings in Chrome, where it was much less obvious that the types are the same size. Would this be an appropriate replacement for the check?
>   static_assert(std::numeric_limits<decltype(Size)>::max() <=
>                 std::numeric_limits<SIZE_T>::max());
My original plan was to come up with a way to elide the check if we can statically verify that it will fit, but then zturner pointed out that this is always true and we can just delete the check altogether.

Note that the code checks if Size fits in a SIZE_T. Size was a uint64_t until recently and is now a size_t, which is at most a 64-bit unsigned. Offset is already an uint64_t. So we need a 64-bit value even if Size were 32-bit on some system. And we do have 64 bits, because we're passing two 32-bit values to CreateFileMappingW and MapViewOfFile. So a 64-bit value will always fit.


https://reviews.llvm.org/D39263