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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Sema/SemaChecking.cpp:8592
+  bool BadR = !RType->isIntegerType() || RType->isSignedIntegerType() ||
+              RHS->isKnownToHaveBooleanValue();
+
----------------
rjmccall wrote:
> Please extract a function which computes this for an Expr* and then call it as part of the conditions below, e.g.:
> 
>   if (op == BO_LT && isNonBooleanUnsignedValue(E->getLHS()) && IsZero(S, E->getRHS()))
Yes, that is indeed a better way :)


================
Comment at: lib/Sema/SemaChecking.cpp:8879
+  if (IsComparisonConstant)
     return AnalyzeImpConvsInComparison(S, E);
   
----------------
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? 


Repository:
  rL LLVM

https://reviews.llvm.org/D37565