rjmccall added inline comments.
================
Comment at: lib/Sema/SemaChecking.cpp:8879
+ if (IsComparisonConstant)
return AnalyzeImpConvsInComparison(S, E);
----------------
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.
================
Comment at: lib/Sema/SemaChecking.cpp:8589
+ Expr *LHS = E->getLHS()->IgnoreParenImpCasts();
+ Expr *RHS = E->getRHS()->IgnoreParenImpCasts();
+
----------------
Do you still need these? I'm always antsy about code that ignores implicit
casts, especially before evaluation.
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
