[PATCH] D37565: [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaChecking.cpp:8589
+  Expr *LHS = E->getLHS()->IgnoreParenImpCasts();
+  Expr *RHS = E->getRHS()->IgnoreParenImpCasts();
+
----------------
lebedev.ri wrote:
> rjmccall wrote:
> > Do you still need these?  I'm always antsy about code that ignores implicit casts, especially before evaluation.
> Yes, absolutely. I did check without them, and they are needed. Else:
> ```
> $ ninja check-clang-sema check-clang-semacxx
> [27/29] Running lit suite /build/clang/test/Sema
> lit.py: /build/clang/test/lit.cfg:200: note: using clang: '/build/llvm-build-Clang-release/./bin/clang'
> FAIL: Clang :: Sema/compare.c (195 of 548)
> ******************** TEST 'Clang :: Sema/compare.c' FAILED ********************
> Script:
> --
> /build/llvm-build-Clang-release/./bin/clang -cc1 -internal-isystem /build/llvm-build-Clang-release/lib/clang/6.0.0/include -nostdsysteminc -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare /build/clang/test/Sema/compare.c -Wno-unreachable-code
> --
> Exit Code: 1
> 
> Command Output (stderr):
> --
> error: 'warning' diagnostics seen but not expected: 
>   File /build/clang/test/Sema/compare.c Line 80: comparison of unsigned expression < 0 is always false
>   File /build/clang/test/Sema/compare.c Line 88: comparison of unsigned expression < 0 is always false
>   File /build/clang/test/Sema/compare.c Line 89: comparison of unsigned expression < 0 is always false
> 3 errors generated.
> 
> --
> 
> ********************
> FAIL: Clang :: Sema/outof-range-constant-compare.c (357 of 548)
> ******************** TEST 'Clang :: Sema/outof-range-constant-compare.c' FAILED ********************
> Script:
> --
> /build/llvm-build-Clang-release/./bin/clang -cc1 -internal-isystem /build/llvm-build-Clang-release/lib/clang/6.0.0/include -nostdsysteminc -triple x86_64-apple-darwin -fsyntax-only -Wtautological-constant-out-of-range-compare -verify /build/clang/test/Sema/outof-range-constant-compare.c
> --
> Exit Code: 1
> 
> Command Output (stderr):
> --
> error: 'warning' diagnostics expected but not seen: 
>   File /build/clang/test/Sema/outof-range-constant-compare.c Line 163: comparison of unsigned expression < 0 is always false
>   File /build/clang/test/Sema/outof-range-constant-compare.c Line 169: comparison of unsigned expression >= 0 is always true
>   File /build/clang/test/Sema/outof-range-constant-compare.c Line 178: comparison of 0 <= unsigned expression is always true
>   File /build/clang/test/Sema/outof-range-constant-compare.c Line 180: comparison of 0 > unsigned expression is always false
> error: 'warning' diagnostics seen but not expected: 
>   File /build/clang/test/Sema/outof-range-constant-compare.c Line 40: comparison of unsigned expression < 0 is always false
>   File /build/clang/test/Sema/outof-range-constant-compare.c Line 46: comparison of unsigned expression >= 0 is always true
>   File /build/clang/test/Sema/outof-range-constant-compare.c Line 55: comparison of 0 <= unsigned expression is always true
>   File /build/clang/test/Sema/outof-range-constant-compare.c Line 57: comparison of 0 > unsigned expression is always false
> 8 errors generated.
> 
> --
> 
> ********************
> Testing Time: 2.05s
> ********************
> Failing Tests (2):
>     Clang :: Sema/compare.c
>     Clang :: Sema/outof-range-constant-compare.c
> 
>   Expected Passes    : 546
>   Unexpected Failures: 2
> FAILED: tools/clang/test/CMakeFiles/check-clang-sema 
> cd /build/llvm-build-Clang-release/tools/clang/test && /usr/bin/python2.7 /build/llvm/utils/lit/lit.py -sv --param clang_site_config=/build/llvm-build-Clang-release/tools/clang/test/lit.site.cfg /build/clang/test/Sema
> ninja: build stopped: subcommand failed.
> ```
What happens if you just move them into isNonBooleanUnsignedValue?


================
Comment at: test/Sema/outof-range-constant-compare.c:41
+    if (a < 0x0000000000000000UL)
+        return 0;
+    if (a <= 0x0000000000000000UL)
----------------
Hmm.  I think this should probably still warn under -Wtautological-compare, shouldn't it?  The fact that it also warns under -Wsign-compare is something we should try to suppress, but it's definitely still a tautological comparison because of the promotion to an unsigned type.


Repository:
  rL LLVM

https://reviews.llvm.org/D37565