aaron.ballman added inline comments.

================
Comment at: lib/Sema/SemaChecking.cpp:8605
 
-  return Match;
+  const char *Cmp; // Simplified, pretty-printed comparison string
+  // Similar to E->getOpcodeStr(), but with extra 0 on either LHS or RHS
----------------
I'd drop this comment entirely.


================
Comment at: lib/Sema/SemaChecking.cpp:8608
+
+  if (Op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
+    Result = false;
----------------
Instead of doing all of this complex logic, why not structure the code like the 
original and use '?:' to determine which diagnostic id to pass in?
```
if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
  S.Diag(E->getOperatorLoc(), HasEnumType(LHS) ? 
diag::warn_lunsigned_enum_always_true_comparison :
diag::warn_lunsigned_always_true_comparison)
    << "< 0" << "false"
    << LHS->getSourceRange() << RHS->getSourceRange();
```


Repository:
  rL LLVM

https://reviews.llvm.org/D37629



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to