[PATCH] D41512: [Sema] -Wtautological-constant-compare is too good. Cripple it.

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 25 00:45:53 PST 2017


lebedev.ri added inline comments.


================
Comment at: include/clang/Basic/DiagnosticGroups.td:439
 def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
+def TautologicalRangeCompare : DiagGroup<"tautological-constant-range-compare">;
----------------
rsmith wrote:
> I forget, are these in-range-compare warnings new too, or just the non-unsigned-enum form? It would make sense to me for these two to be subgroups of `TautologicalRangeCompare`.
I do believe all three are new: `warn_unsigned_enum_always_true_comparison`, `warn_unsigned_always_true_comparison`, `warn_tautological_constant_compare`.
I'll move them into `TautologicalRangeCompare`, and disable them too...

But now we are almost back to square one.
Disabling `TautologicalRangeCompare` will disable all three type-limit-checking diagnostic, and only the inner two can be enabled separately.
Is that really the intended result? I think `warn_tautological_constant_compare` should have it's own flag.
But i can not come up with a name for it. Please advise.


================
Comment at: include/clang/Basic/DiagnosticGroups.td:440
 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
+def TautologicalRangeCompare : DiagGroup<"tautological-constant-range-compare">;
 def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
----------------
rsmith wrote:
> "tautological-constant-in-range-compare" would make more sense to me, as the complement of "tautological-constant-out-of-range-compare".
I did think about it.
To me "tautological-constant-in-range-compare" may sound as if the constant is //somewhere// in the range,
while i'd say "tautological-constant-range-compare" better highlights the fact that the constant *is* the range limit.
No?


Repository:
  rC Clang

https://reviews.llvm.org/D41512





More information about the cfe-commits mailing list