lebedev.ri added inline comments.
================
Comment at: lib/Sema/SemaChecking.cpp:8879
+ if (IsComparisonConstant)
return AnalyzeImpConvsInComparison(S, E);
----------------
rjmccall wrote:
> lebedev.ri wrote:
> > rjmccall wrote:
> > > Part of the purpose of checking for signed comparisons up here is to
> > > avoid unnecessarily computing ranges for the operands when we aren't
> > > going to use them. That's still something we want to avoid. I think you
> > > just need to call CheckTrivialUnsignedComparison in this fallback case,
> > > at least when the comparison is not constant.
> > >
> > > You should also rename CheckTrivialUnsignedComparison to something like
> > > CheckTautologicalComparison.
> > Ok, i must admit that i understand the idea to have minimal overhead. But i
> > don't really follow the code in here :)
> > This seems to work, and `check-clang-sema`+`check-clang-semacxx`+stage2
> > still pass.
> > Is this obviously-wrong / some testcase is missing?
> There's a lot of code here doing very similar checks, and we just want to
> ensure that we aren't doing too much redundant work. I think the way you've
> structured it now is fine.
OK :)
================
Comment at: lib/Sema/SemaChecking.cpp:8589
+ Expr *LHS = E->getLHS()->IgnoreParenImpCasts();
+ Expr *RHS = E->getRHS()->IgnoreParenImpCasts();
+
----------------
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.
```
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
