[PATCH] D155457: [clang] Skip tautological comparison if the comparison involves the 'size_t' type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D155457#4513812 <https://reviews.llvm.org/D155457#4513812>, @xgupta wrote:

> In D155457#4511652 <https://reviews.llvm.org/D155457#4511652>, @aaron.ballman wrote:
>
>> In the x86 compilation: `sizeof(std::size_t) < sizeof(uint64_t)` is true, so we test the other expression; because `Size` is greater than `__SIZE_MAX__` the function returns false and the second static assertion fails as expected.
>> In the x64 compilation: `sizeof(std::size_t) < sizeof(uint64_t)` is false (first static assertion fails) so we shouldn't even be evaluating the RHS of the `&&` to see if it's tautological because it can't contribute to the expression result, right?
>
> Yes, I agree with both statements but what is odd in current behavior?

It's a false positive -- the tautological bit is diagnosed but it doesn't contribute to the result of the expression. The first part of the predicate is specifically intended to ensure the second part of the predicate is *not* tautological.

> It seems to me uint64_t is unsigned long in <cstdint>, not unsigned long long so the new test added in this patch (with `typedef unsigned long uint64_t`) is now passing.
>
> Behavior when the patch is applied-
>
>   $ ./bin/clang++ -Wall -fsyntax-only test.cpp -m64  -Wtype-limits 
>   test.cpp:8:11: warning: result of comparison 'uint64_t' (aka 'unsigned long long') > 18446744073709551615 is always false [-Wtautological-type-limit-compare]
>       8 |      Size > static_cast<uint64_t>(__SIZE_MAX__)) // no-warning
>         |      ~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   test.cpp:13:15: error: static assertion failed due to requirement 'sizeof(unsigned long) < sizeof(unsigned long long)': 
>      13 | static_assert(sizeof(std::size_t) < sizeof(uint64_t), "");
>         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   test.cpp:13:35: note: expression evaluates to '8 < 8'
>      13 | static_assert(sizeof(std::size_t) < sizeof(uint64_t), "");
>         |               ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
>   1 warning and 1 error generated.
>
>   $ ./bin/clang++ -Wall -fsyntax-only test1.cpp -m64  -Wtype-limits 
>   test1.cpp:12:15: error: static assertion failed due to requirement 'sizeof(unsigned long) < sizeof(unsigned long)': 
>      12 | static_assert(sizeof(std::size_t) < sizeof(uint64_t), "");
>         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   test1.cpp:12:35: note: expression evaluates to '8 < 8'
>      12 | static_assert(sizeof(std::size_t) < sizeof(uint64_t), "");
>         |               ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
>   1 error generated.
>
> where test.cpp using wrapper and test1.cpp is using standard headers.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155457