[Diagnostics] Support -Wtype-limits for GCC compatibility

[xbolva00]

Summary

GCC's -Wtype-limits (part of -Wextra):
Warn if a comparison is always true or always false due to the limited range of the data type

[lebedev.ri]

See D41512, rC322901, D51545.

clang/include/clang/Basic/DiagnosticGroups.td

784 ↗

(On Diff #188948)

-Wtautological-constant-in-range-compare is quite intentionally not in -Wextra.

[xbolva00]

Remove from -Wextra

[lebedev.ri]

clang/include/clang/Basic/DiagnosticGroups.td

485 ↗

(On Diff #188957)

Is gcc's -Wtype-limits *just* -Wtautological-constant-in-range-compare, or is something else should be there?

[xbolva00]

This comment has been deleted.

clang/include/clang/Basic/DiagnosticGroups.td

485 ↗

(On Diff #188957)

-Wtautological-constant-in-range-compare is enough to diagnose 99% cases from:

https://github.com/gcc-mirror/gcc/blob/41d6b10e96a1de98e90a7c0378437c3255814b16/gcc/testsuite/g%2B%2B.dg/warn/Wtype-limits.C

One missed case is:
int test (int x)
{

if ((long long)x <= 0x123456789ABCLL)
  return 1;
else 
  return 0;

}

Tried -Weverything, no help. We miss this case. I will open PR for that.

But anyway, this missed case should not block this patch.

[lebedev.ri]

clang/include/clang/Basic/DiagnosticGroups.td

485 ↗

(On Diff #188957)

-Wtautological-constant-in-range-compare is enough to diagnose 99% cases from:

That doesn't really answer the question.
I'm sure that *adding* -Wtautological-constant-in-range-compare helps.
But what about the other way around?
Does passing -Wno-type-limits silence *all* the expected diags in that file?

[xbolva00]

clang/include/clang/Basic/DiagnosticGroups.td

485 ↗

(On Diff #188957)

Yes, silences it correctly.

[xbolva00]

Ping

[xbolva00]

Ping @aaron.ballman @rsmith

[aaron.ballman]

I worry a little bit about our -Wtype-limits getting out of sync from GCC's due to making it a synonym for -Wtautological-constant-in-range-compare, but I'm also at a loss for why these two should ever have different functionality.

I think I'm okay with this, but I'd like @rsmith to weigh in as well.

clang/include/clang/Basic/DiagnosticGroups.td

485 ↗

(On Diff #188957)

Thoughts about why we behave differently here: https://github.com/gcc-mirror/gcc/blob/41d6b10e96a1de98e90a7c0378437c3255814b16/gcc/testsuite/g%2B%2B.dg/warn/Wtype-limits2.C

[xbolva00]

Ping @rsmith

[rsmith]

xbolva00 abandoned this revision.

Do you not want to pursue this any more?

This seems reasonable to me, and is in line with other cases where we have diagnostic flags as aliases to GCC's similar-but-not-quite-the-same flags (eg, GCC's -Wnoexcept-type doesn't fire in quite the same set of cases as clang's -Wc++17-compat-mangling, but we alias them; likewise for GCC -Wsequence-point versus Clang -Wunsequenced).

[xbolva00]

Ok, reclaiming patch.

Thanks for the review! If the patch is fine, please approve it.

[rsmith]

In D58841#1483128, @xbolva00 wrote:

Thanks for the review! If the patch is fine, please approve it.

Sure thing! (Phab doesn't permit approving a patch in "abandoned" state.)

[xbolva00]

Rebased

include/clang/Basic/DiagnosticGroups.td

 def StrncatSize : DiagGroup<"strncat-size">;
 def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">;
 def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
 def TautologicalInRangeCompare : DiagGroup<"tautological-constant-in-range-compare",
                                            [TautologicalTypeLimitCompare,
                                             TautologicalUnsignedZeroCompare,
                                             TautologicalUnsignedEnumZeroCompare]>;
+// For compatibility with GCC; -Wtype-limits = -Wtautological-constant-in-range-compare
+def TypeLimits : DiagGroup<"type-limits", [TautologicalInRangeCompare]>;
 def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
 def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare",
                                             [TautologicalOutOfRangeCompare]>;
 def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;
 def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;
 def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">;
 def TautologicalCompare : DiagGroup<"tautological-compare",
                                     [TautologicalConstantCompare,

test/Sema/compare.c

 // RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtautological-constant-in-range-compare %s -Wno-unreachable-code
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtype-limits %s -Wno-unreachable-code
 
 int test(char *C) { // nothing here should warn.
   return C != ((void*)0);
   return C != (void*)0;
   return C != 0;
   return C != 1;  // expected-warning {{comparison between pointer and integer ('char *' and 'int')}}
 }
 

test/Sema/tautological-constant-compare.c

 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra -Wno-sign-compare -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra -Wno-sign-compare -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -x c++ %s
 
 int value(void);

test/Sema/tautological-constant-enum-compare.c

 // RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -Wtautological-constant-in-range-compare -verify %s
 // RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -Wtautological-constant-in-range-compare -verify %s
 // RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -DSILENCE -Wno-tautological-constant-compare -verify %s
 // RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -DSILENCE -Wno-tautological-constant-compare -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -Wtype-limits -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -DSILENCE -Wno-type-limits -verify %s
 
 int main() {
   enum A { A_a = 2 };
   enum A a;
 
 #ifdef SILENCE
   // expected-no-diagnostics
 #else

test/SemaCXX/compare.cpp

 // Force x86-64 because some of our heuristics are actually based
 // on integer sizes.
 
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtautological-constant-in-range-compare -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -pedantic -verify -Wsign-compare -Wtype-limits -std=c++11 %s
 
 int test0(long a, unsigned long b) {
   enum EnumA {A};
   enum EnumB {B};
   enum EnumC {C = 0x10000};
   return
          // (a,b)
          (a == (unsigned long) b) +  // expected-warning {{comparison of integers of different signs}}