[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Thanks for the refactoring :)



================
Comment at: lib/Sema/SemaChecking.cpp:8697
+
+  return true;
 }
----------------
lebedev.ri wrote:
> If the diagnostic we are about to output is disabled, should we still `return true;` and suppress potential `-Wsign-compare` warning?
The general principle is to behave as if we produced all diagnostics, and then filtered out the ones that aren't enabled. So if a more specialized (eg tautological comparison) diagnostic is disabled, the more general (eg sign compare) diagnostic should not appear. In short, this is fine :)


================
Comment at: lib/Sema/SemaChecking.cpp:8719
+  // Type limit values are handled later by CheckTautologicalComparison().
+  if (IsTypeLimit(S, Other, Constant, ConstValue) && (!OtherIsBooleanType))
     return;
----------------
Is this necessary? (Aren't the type limit values always within the range of the type in question?)

Can we avoid evaluating `Constant` a extra time here? (We already have its value in `Value`.)


Repository:
  rL LLVM

https://reviews.llvm.org/D38101