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