[clang] b732e14 - [Clang] Default the warning for chained comparison to an error. (#128145)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 21 06:52:13 PST 2025
Author: cor3ntin
Date: 2025-02-21T15:52:09+01:00
New Revision: b732e14cb5681abff78e3873987a2a8b39549c83
URL: https://github.com/llvm/llvm-project/commit/b732e14cb5681abff78e3873987a2a8b39549c83
DIFF: https://github.com/llvm/llvm-project/commit/b732e14cb5681abff78e3873987a2a8b39549c83.diff
LOG: [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`)
Added:
Modified:
clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.c
clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.cpp
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/test/Sema/bool-compare.c
clang/test/Sema/parentheses.cpp
clang/test/SemaCXX/bool-compare.cpp
clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
clang/test/SemaTemplate/typo-dependent-name.cpp
clang/test/SemaTemplate/typo-template-name.cpp
Removed:
################################################################################
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