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