[clang][Sema] Warn consecutive builtin comparisons in an expression (#92200)

Add warning under `-Wparentheses` for consistency with `gcc 14.1`.

Closes #20456

---------

Co-authored-by: Aaron Ballman <[email protected]>

Author: JOE1994

clang/docs/ReleaseNotes.rst

@@ -498,6 +498,9 @@ Improvements to Clang's diagnostics
        }
      };
 
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive comparisons like ``x < y < z``.
+  Fixes #GH20456.
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6919,6 +6919,10 @@ def warn_precedence_bitwise_conditional : Warning<
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_consecutive_comparison : Warning<
+  "comparisons like 'X<=Y<=Z' don't have their mathematical meaning">,
+  InGroup<Parentheses>;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup<IntInBoolContext>, DefaultIgnore;

clang/lib/Sema/SemaExpr.cpp

@@ -14867,6 +14867,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
   case BO_GT:
     ConvertHalfVec = true;
     ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+    if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr);
+        BI && BI->isComparisonOp())
+      Diag(OpLoc, diag::warn_consecutive_comparison);
+
     break;
   case BO_EQ:
   case BO_NE:

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)      {} // no warning
+  if (a>y<z)      {} // expected-warning {{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)      {}        // no warning
+  if (z<a>y)      {} // expected-warning {{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

@@ -215,3 +215,14 @@ namespace PR20735 {
     // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+void consecutive_builtin_compare(int x, int y, int z) {
+  (void)(x < y < z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  (void)(x < y > z);  // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  (void)(x < y <= z); // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  (void)(x <= y > z); // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
+  (void)((x < y) < z);  // no-warning
+  (void)((x < y) >= z); // no-warning
+
+  // TODO: Also issue warning for consecutive comparisons that use overloaded comparison operators?
+}

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)      {} // no warning
+  if (a>y<z)      {} // expected-warning {{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)      {} // no warning
+  if (z<a>y)      {} // expected-warning {{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/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}}
+  f<0>(q); // expected-error {{invalid operands to binary expression}} // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
 }
 
 void disambig() {

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

@@ -20,11 +20,11 @@ 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;
+    return this->inner < other > ::z; // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
   }
 };
 
-void use_x(X<int> x) { x.f(0); }
+void use_x(X<int> x) { x.f(0); } // expected-note {{requested here}}
 
 template<typename T>
 struct Y {

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);
+    foo<int()>(0); // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}}
     foo<int(), true>(false);
     foo<Base{}.n;
   }