The issue with size_t comes when we are trying to addd
-Wtype-limits to -Wextra for GCC compatibility in review
https://reviews.llvm.org/D142826.
Details
- Reviewers
aaron.ballman xbolva00
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
There is some issue writing test
$ cat type-limit-compare.cpp // RUN: %clang_cc1 -fsyntax-only -Wtautological-type-limit-compare -verify %s // expected-no-diagnostics #include <cstddef> #include <cstdint> #include <limits> bool foo(uint64_t Size) { if (sizeof(std::size_t) < sizeof(uint64_t) && Size > static_cast<uint64_t>(std::numeric_limits<std::size_t>::max())) // no-warning return false; return true; }
failed with
$ llvm-project/clang/test/Sema/type-limit-compare.cpp:4:10: fatal error: 'cstddef' file not found 4 | #include <cstddef> | ^~~~~~~~~
We typically do not include any system headers (STL or otherwise) as part of the compiler tests; that would test whatever is found on the test system instead of a consistent test. Instead, I'd recommend doing:
namespace std { using size_t = decltype(sizeof(0)); }
Similarly, you can replace uint64_t with unsigned long long and the numeric_limits call with __SIZE_MAX__
I see, Thanks, but there is another thing, writing this way compiler emits a warning as the check to exclude the warning is based on size_t so the test case is not passed.
// RUN: %clang_cc1 %s -fsyntax-only -Wtautological-type-limit-compare -verify // expected-no-diagnostics typedef unsigned long long uint64_t; namespace std { using size_t = decltype(sizeof(0)); } // namespace std bool func(uint64_t Size) { if (sizeof(std::size_t) < sizeof(uint64_t) && Size > (uint64_t)(__SIZE_MAX__)) return false; return true; }
Whether size_t comes from the system header or whether it's manually deduced from decltype(sizeof(0)) should make no difference as far as the frontend is concerned; they should resolve to the same type. Can you explain the test failure you're seeing in a bit more detail?
: 'RUN: at line 1'; /home/shivam/.llvm/llvm-project/build/bin/clang -cc1 -internal-isystem /home/shivam/.llvm/llvm-project/build/lib/clang/17/include -nostdsysteminc /home/shivam/.llvm/llvm-project/clang/test/Sema/type-limit-compare.cpp -fsyntax-only -Wtautological-type-limit-compare -verify -- Exit Code: 1 Command Output (stderr): -- error: 'warning' diagnostics seen but not expected: File /home/shivam/.llvm/llvm-project/clang/test/Sema/type-limit-compare.cpp Line 12: result of comparison 'uint64_t' (aka 'unsigned long long') > 18446744073709551615 is always false 1 error generated.
I think the issue is with (uint64_t)(____SIZE_MAX____) vs (uint64_t)std::numeric_limits<std::size_t>::max() but not sure.
Something seems odd to me about the current behavior. Consider: https://godbolt.org/z/ofezfhaPd
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 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.
Just normal ping, @aaron.ballman, can you please take a look, pre-merge checks are also passing now.
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.
I'm not sure I understand the motivation for this change. Sure, people do that but they also might do the same thing for ssize_t, intmax_t, or to compare int to int32_t.
I think a better heuristic would be to not emit a warning for any integral (and floating point?) type that have the same canonical types (but we probably still want one if their non-canonical type if the same)
Ok, Is there any modification required in this patch or it fixes that false positive?
I am not sure but are you expecting these changes -
// Don't warn if the comparison involves integral or floating-point types with the same canonical types. QualType LHSCanonical = Constant->getType().getCanonicalType(); QualType RHSCanonical = Other->getType().getCanonicalType(); if ((LHSCanonical->isIntegralOrEnumerationType() || LHSCanonical->isFloatingType()) && S.Context.hasSameType(LHSCanonical, RHSCanonical)) { return false; }
This will silence a lot of warnings and a total 5 test case fails.
Can you share some examples of what test cases start failing with that approach? What you have above matches what I think @cor3ntin was asking for and does seem like a pretty reasonable way to silence false positives.
Sure, updated three test cases in the patch and list the other two here -
FAIL: Clang :: Sema/tautological-constant-compare.c (865 of 18988) ******************** TEST 'Clang :: Sema/tautological-constant-compare.c' FAILED ******************** error: 'warning' diagnostics expected but not seen: File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 560 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:568): comparison of 3-bit signed value < 4 is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 574 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:585): comparison of 8-bit unsigned value < 0 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 593 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:595): comparison of 2-bit unsigned value > 3 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 603: result of comparison 'int' > 2147483647 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 608 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:610): comparison of 15-bit unsigned value > 32767 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 614 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:616): comparison of 6-bit signed value > 31 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 621 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:637): comparison of 4-bit signed value < -8 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 623 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:638): comparison of 4-bit signed value > 7 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 628 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:639): comparison of 5-bit signed value < -16 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 629 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:640): comparison of 5-bit signed value > 15 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 632 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:641): comparison of 4-bit signed value > 7 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 633 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:642): comparison of 4-bit signed value < -8 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 635 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:643): comparison of 5-bit signed value < -16 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 648 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:659): comparison of 3-bit signed value > 3 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 652 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:660): comparison of 3-bit signed value < -4 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 656 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:661): comparison of 2-bit unsigned value > 3 is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c Line 657 (directive at /home/shivam/.llvm/llvm-project/clang/test/Sema/tautological-constant-compare.c:662): comparison of 2-bit unsigned value < 0 is always false 17 errors generated.
FAIL: Clang :: Sema/bool-compare.c (10880 of 18988) ******************** TEST 'Clang :: Sema/bool-compare.c' FAILED ******************** Script: -- : 'RUN: at line 1'; /home/shivam/.llvm/llvm-project/build/bin/clang -cc1 -internal-isystem /home/shivam/.llvm/llvm-project/build/lib/clang/17/include -nostdsysteminc -fsyntax-only -verify /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c -Wno-logical-not-parentheses -- Exit Code: 1 Command Output (stderr): -- error: 'warning' diagnostics expected but not seen: File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 8: comparison of constant 1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 33: comparison of constant 1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 34: comparison of constant 2 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 37: comparison of constant -1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 39: comparison of constant 0 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 41: comparison of constant 2 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 44: comparison of constant -1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 46: comparison of constant 0 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 48: comparison of constant 2 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 51: comparison of constant -1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 54: comparison of constant 1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 55: comparison of constant 2 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 58: comparison of constant -1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 61: comparison of constant 1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 62: comparison of constant 4 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 63: comparison of constant -1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 66: comparison of constant 1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 67: comparison of constant 4 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 70: comparison of constant 1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 71: comparison of constant 4 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 73: comparison of constant -1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 77: comparison of constant 2 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 79: comparison of constant -1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 83: comparison of constant 2 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 85: comparison of constant -1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 97: comparison of constant 0 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 99: comparison of constant 2 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 101: comparison of constant -1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 104: comparison of constant 1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 105: comparison of constant 2 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 107: comparison of constant -1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 110: comparison of constant 1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 111: comparison of constant 2 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 113: comparison of constant -1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 115: comparison of constant 0 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 117: comparison of constant 2 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 119: comparison of constant -1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 121: comparison of constant 0 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 123: comparison of constant 4 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 125: comparison of constant 0 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 127: comparison of constant 4 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 129: comparison of constant 0 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 131: comparison of constant 4 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 133: comparison of constant -1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 137: comparison of constant 2 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 139: comparison of constant -1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 143: comparison of constant 2 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 145: comparison of constant -1 with boolean expression is always true File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 158: comparison of constant -1 with boolean expression is always false File /home/shivam/.llvm/llvm-project/clang/test/Sema/bool-compare.c Line 159: comparison of constant 2 with boolean expression is always true
clang/lib/Sema/SemaChecking.cpp | ||
---|---|---|
13807–13815 | This is what I had in mind. I haven't tested it though |
This is what I had in mind. I haven't tested it though