https://github.com/cor3ntin created 
https://github.com/llvm/llvm-project/pull/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`)

>From c2c1be362ff3202b8238d1954e3806d09721d1c1 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinja...@gmail.com>
Date: Fri, 21 Feb 2025 08:41:07 +0100
Subject: [PATCH] [Clang] Default the warning for chained comparison to an
 error.

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`)
---
 clang/docs/ReleaseNotes.rst                       | 2 ++
 clang/include/clang/Basic/DiagnosticSemaKinds.td  | 2 +-
 clang/test/Sema/bool-compare.c                    | 4 ++--
 clang/test/Sema/parentheses.cpp                   | 4 ++--
 clang/test/SemaCXX/bool-compare.cpp               | 7 +++----
 clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp | 2 +-
 clang/test/SemaTemplate/typo-dependent-name.cpp   | 2 +-
 clang/test/SemaTemplate/typo-template-name.cpp    | 2 +-
 8 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e1c61992512b5..01c58295613d7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
 ----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index feef50812eca9..f4f1bc67724a1 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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">,
diff --git a/clang/test/Sema/bool-compare.c b/clang/test/Sema/bool-compare.c
index 2308dc4bf2bb2..861f47864ddd9 100644
--- a/clang/test/Sema/bool-compare.c
+++ b/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
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index e991cee16b0e2..5424704ea01d5 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/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();
 
diff --git a/clang/test/SemaCXX/bool-compare.cpp 
b/clang/test/SemaCXX/bool-compare.cpp
index 4e33e076a7de4..077d55ff9367d 100644
--- a/clang/test/SemaCXX/bool-compare.cpp
+++ b/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;
diff --git a/clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp 
b/clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
index b8397f41febc3..fb840e1036707 100644
--- a/clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
+++ b/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() {
diff --git a/clang/test/SemaTemplate/typo-dependent-name.cpp 
b/clang/test/SemaTemplate/typo-dependent-name.cpp
index 5bd924241480d..1812525ec5d7b 100644
--- a/clang/test/SemaTemplate/typo-dependent-name.cpp
+++ b/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}}
   }
 };
 
diff --git a/clang/test/SemaTemplate/typo-template-name.cpp 
b/clang/test/SemaTemplate/typo-template-name.cpp
index 741aa708e81fc..c1d8aa7c8e27d 100644
--- a/clang/test/SemaTemplate/typo-template-name.cpp
+++ b/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;
   }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to