[clang] [clang-tools-extra] [Clang] Default the warning for chained comparison to an error. (PR #128145)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 21 00:52:44 PST 2025
https://github.com/cor3ntin updated https://github.com/llvm/llvm-project/pull/128145
>From 55dcf33ba907e8c273ee3dfe196b71bc4b70c325 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at 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`)
---
.../test/clang-tidy/checkers/bugprone/chained-comparison.c | 2 +-
.../clang-tidy/checkers/bugprone/chained-comparison.cpp | 2 +-
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 +-
10 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.c
index 53c6139dcfbc2..7a4322c6a00c0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.c
+++ b/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)
{
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.cpp
index 8a664aa205acf..88813f72e37ca 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.cpp
+++ b/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)
{
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;
}
More information about the cfe-commits
mailing list