[Clang] Default the warning for chained comparison to an error. (#128145)

Boolean constructs of the form `a < b < c`
never express the likely intent of the user and
we have been warning on them since clang 19.

WG21 is likely to deprecate or make that construct in the future. To
gain more experience and to improve safety, this patches preempt future
language evolution by turning
warn_consecutive_comparison in an error, by default (which can be
disabled with `-Wno-error=parentheses`)

Author: cor3ntin

clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.c

@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-chained-comparison %t
+// RUN: %check_clang_tidy --extra-arg=-Wno-error=parentheses %s bugprone-chained-comparison %t
 
 void badly_chained_1(int x, int y, int z)
 {

clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.cpp

@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-chained-comparison %t
+// RUN: %check_clang_tidy -std=c++98-or-later --extra-arg=-Wno-error=parentheses %s bugprone-chained-comparison %t
 
 void badly_chained_1(int x, int y, int z)
 {

clang/docs/ReleaseNotes.rst

@@ -147,6 +147,8 @@ Improvements to Clang's diagnostics
 - The ``-Wunsafe-buffer-usage`` warning has been updated to warn
   about unsafe libc function calls.  Those new warnings are emitted
   under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``.
+- Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
+  ``-Wno-error=parentheses``.
 
 Improvements to Clang's time-trace
 ----------------------------------

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7126,7 +7126,7 @@ def note_precedence_conditional_first : Note<
 
 def warn_consecutive_comparison : Warning<
   "comparisons like 'X<=Y<=Z' don't have their mathematical meaning">,
-  InGroup<Parentheses>;
+  InGroup<Parentheses>, DefaultError;
 
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,

clang/test/Sema/bool-compare.c

@@ -85,7 +85,7 @@ void f(int x, int y, int z) {
   if ((a<y) != -1) {}// expected-warning {{comparison of constant -1 with boolean expression is always true}}
 
   if ((a<y) == z) {} // no warning
-  if (a>y<z)      {} // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  if (a>y<z)      {} // expected-error {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
   if ((a<y) > z)  {} // no warning
   if((a<y)>(z<y)) {} // no warning
   if((a<y)==(z<y)){} // no warning
@@ -145,7 +145,7 @@ void f(int x, int y, int z) {
   if (-1 !=(a<y)) {} // expected-warning {{comparison of constant -1 with boolean expression is always true}}
 
   if (z ==(a<y))  {}    // no warning
-  if (z<a>y)      {} // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  if (z<a>y)      {} // expected-error {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
   if (z > (a<y))  {}    // no warning
   if((z<y)>(a<y)) {}   // no warning
   if((z<y)==(a<y)){}  // no warning

clang/test/Sema/parentheses.cpp

@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify %s
-// RUN: %clang_cc1 -Wparentheses -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -Wno-error=parentheses -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wno-error=parentheses -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 bool someConditionFunc();
 

clang/test/SemaCXX/bool-compare.cpp

@@ -98,7 +98,7 @@ void f(int x, int y, int z) {
   if ((a<y) != -1) {}// expected-warning {{comparison of constant -1 with expression of type 'bool' is always true}}
 
   if ((a<y) == z) {} // no warning
-  if (a>y<z)      {} // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  if (a>y<z)      {} // expected-error {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
   if ((a<y) > z)  {} // no warning
   if((a<y)>(z<y)) {} // no warning
   if((a<y)==(z<y)){} // no warning
@@ -159,7 +159,7 @@ void f(int x, int y, int z) {
   if (-1 !=(a<y)) {} // expected-warning {{comparison of constant -1 with expression of type 'bool' is always true}}
 
   if (z ==(a<y))  {} // no warning
-  if (z<a>y)      {} // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  if (z<a>y)      {} // expected-error {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
   if (z > (a<y))  {} // no warning
   if((z<y)>(a<y)) {} // no warning
   if((z<y)==(a<y)){} // no warning
@@ -186,8 +186,7 @@ template<typename T, typename U, typename V> struct X6 {
         return v; // expected-error{{cannot initialize return object of type}}
     }
     bool r;
-    // FIXME: We should warn here, DiagRuntimeBehavior does currently not detect this.
-    if(r<0){}
+    if(r<0){} // expected-warning {{result of comparison of constant 0 with expression of type 'bool' is always false}}
 
     if (T x = t) {
       t = x;

clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp

@@ -27,7 +27,7 @@ int e = h<0>(q); // ok, found by unqualified lookup
 void fn() {
   f<0>(q);
   int f;
-  f<0>(q); // expected-error {{invalid operands to binary expression}} // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  f<0>(q); // expected-error {{invalid operands to binary expression}} // expected-error {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
 }
 
 void disambig() {

clang/test/SemaTemplate/typo-dependent-name.cpp

@@ -20,7 +20,7 @@ struct X : Base<T> {
   bool f(T other) {
     // A pair of comparisons; 'inner' is a dependent name so can't be assumed
     // to be a template.
-    return this->inner < other > ::z; // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+    return this->inner < other > ::z; // expected-error {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
   }
 };
 

clang/test/SemaTemplate/typo-template-name.cpp

@@ -36,7 +36,7 @@ namespace InExpr {
 
     // These are valid expressions.
     foo<foo; // expected-warning {{self-comparison}}
-    foo<int()>(0); // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+    foo<int()>(0); // expected-error {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
     foo<int(), true>(false);
     foo<Base{}.n;
   }