[clang] [Clang][diagnostics] Improve the diagnostics for chained comparisons (PR #129285)
Amr Hesham via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 5 10:04:56 PST 2025
https://github.com/AmrDeveloper updated https://github.com/llvm/llvm-project/pull/129285
>From 7a61f41a3104dd0765dea9930e0cf6f94d656598 Mon Sep 17 00:00:00 2001
From: AmrDeveloper <amr96 at programmer.net>
Date: Fri, 28 Feb 2025 19:27:41 +0100
Subject: [PATCH 1/3] [Clang][diagnostics] Improve the diagnostics for chained
comparisons to report actual expressions and operators
---
clang/docs/ReleaseNotes.rst | 2 ++
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +-
clang/lib/Sema/SemaExpr.cpp | 3 ++-
clang/test/Sema/bool-compare.c | 4 ++--
clang/test/Sema/parentheses.cpp | 8 ++++----
clang/test/SemaCXX/bool-compare.cpp | 4 ++--
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 +-
9 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 688d50a394c62..0779e824bf8a9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -220,6 +220,8 @@ Improvements to Clang's diagnostics
:doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
feature will be default-enabled with ``-Wthread-safety`` in a future release.
+- Improve the diagnostics for chained comparisons to report actual expressions and operators.
+
- Improve the diagnostics for shadows template parameter to report correct location (#GH129060).
Improvements to Clang's time-trace
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0efb15405ed5d..bcbd994ab4d83 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7157,7 +7157,7 @@ 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">,
+ "comparisons like 'X %0 Y %1 Z' don't have their mathematical meaning">,
InGroup<Parentheses>, DefaultError;
def warn_enum_constant_in_bool_context : Warning<
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 1738663327453..59bef237a2377 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14924,7 +14924,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr);
BI && BI->isComparisonOp())
- Diag(OpLoc, diag::warn_consecutive_comparison);
+ Diag(OpLoc, diag::warn_consecutive_comparison)
+ << BI->getOpcodeStr() << BinaryOperator::getOpcodeStr(Opc);
break;
case BO_EQ:
diff --git a/clang/test/Sema/bool-compare.c b/clang/test/Sema/bool-compare.c
index 861f47864ddd9..675f808ece0dd 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-error {{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-error {{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 5424704ea01d5..afc1f3b0caa81 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -217,10 +217,10 @@ namespace PR20735 {
}
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); // 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
diff --git a/clang/test/SemaCXX/bool-compare.cpp b/clang/test/SemaCXX/bool-compare.cpp
index 077d55ff9367d..b527fcf9ae79e 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-error {{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-error {{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/SemaCXX/cxx2a-adl-only-template-id.cpp b/clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
index fb840e1036707..cff576c3d31a7 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-error {{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 1812525ec5d7b..7fb3af29aca5e 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-error {{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 c1d8aa7c8e27d..bc0038d8c6bd9 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-error {{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;
}
>From 99197b6d8e2cb6241d77433e3e8a9c17a19459fb Mon Sep 17 00:00:00 2001
From: AmrDeveloper <amr96 at programmer.net>
Date: Mon, 3 Mar 2025 20:27:55 +0100
Subject: [PATCH 2/3] Add issue number to release note
---
clang/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0779e824bf8a9..484f5bd0159b7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -220,7 +220,7 @@ Improvements to Clang's diagnostics
:doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
feature will be default-enabled with ``-Wthread-safety`` in a future release.
-- Improve the diagnostics for chained comparisons to report actual expressions and operators.
+- Improve the diagnostics for chained comparisons to report actual expressions and operators (#GH129069).
- Improve the diagnostics for shadows template parameter to report correct location (#GH129060).
>From b0de9a9e9f9e299e5023a882ed6f09a74036bd11 Mon Sep 17 00:00:00 2001
From: AmrDeveloper <amr96 at programmer.net>
Date: Wed, 5 Mar 2025 19:04:03 +0100
Subject: [PATCH 3/3] Improve the error message
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +-
clang/test/Sema/bool-compare.c | 4 ++--
clang/test/Sema/parentheses.cpp | 8 ++++----
clang/test/SemaCXX/bool-compare.cpp | 4 ++--
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 +-
7 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index bcbd994ab4d83..b8cfc082b98b7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7157,7 +7157,7 @@ def note_precedence_conditional_first : Note<
"place parentheses around the '?:' expression to evaluate it first">;
def warn_consecutive_comparison : Warning<
- "comparisons like 'X %0 Y %1 Z' don't have their mathematical meaning">,
+ "chained comparison 'X %0 Y %1 Z' does not behave the same as a mathematical expression">,
InGroup<Parentheses>, DefaultError;
def warn_enum_constant_in_bool_context : Warning<
diff --git a/clang/test/Sema/bool-compare.c b/clang/test/Sema/bool-compare.c
index 675f808ece0dd..11ed156328b2e 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-error {{comparisons like 'X > Y < Z' don't have their mathematical meaning}}
+ if (a>y<z) {} // expected-error {{chained comparison 'X > Y < Z' does not behave the same as a mathematical expression}}
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-error {{comparisons like 'X < Y > Z' don't have their mathematical meaning}}
+ if (z<a>y) {} // expected-error {{chained comparison 'X < Y > Z' does not behave the same as a mathematical expression}}
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 afc1f3b0caa81..8d649da3abe32 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -217,10 +217,10 @@ namespace PR20735 {
}
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); // expected-warning {{chained comparison 'X < Y < Z' does not behave the same as a mathematical expression}}
+ (void)(x < y > z); // expected-warning {{chained comparison 'X < Y > Z' does not behave the same as a mathematical expression}}
+ (void)(x < y <= z); // expected-warning {{chained comparison 'X < Y <= Z' does not behave the same as a mathematical expression}}
+ (void)(x <= y > z); // expected-warning {{chained comparison 'X <= Y > Z' does not behave the same as a mathematical expression}}
(void)((x < y) < z); // no-warning
(void)((x < y) >= z); // no-warning
diff --git a/clang/test/SemaCXX/bool-compare.cpp b/clang/test/SemaCXX/bool-compare.cpp
index b527fcf9ae79e..1d856f2865ab6 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-error {{comparisons like 'X > Y < Z' don't have their mathematical meaning}}
+ if (a>y<z) {} // expected-error {{chained comparison 'X > Y < Z' does not behave the same as a mathematical expression}}
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-error {{comparisons like 'X < Y > Z' don't have their mathematical meaning}}
+ if (z<a>y) {} // expected-error {{chained comparison 'X < Y > Z' does not behave the same as a mathematical expression}}
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/SemaCXX/cxx2a-adl-only-template-id.cpp b/clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
index cff576c3d31a7..5c0d89d9125f2 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-error {{comparisons like 'X < Y > Z' don't have their mathematical meaning}}
+ f<0>(q); // expected-error {{invalid operands to binary expression}} // expected-error {{chained comparison 'X < Y > Z' does not behave the same as a mathematical expression}}
}
void disambig() {
diff --git a/clang/test/SemaTemplate/typo-dependent-name.cpp b/clang/test/SemaTemplate/typo-dependent-name.cpp
index 7fb3af29aca5e..5f518f2ac6861 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-error {{comparisons like 'X < Y > Z' don't have their mathematical meaning}}
+ return this->inner < other > ::z; // expected-error {{chained comparison 'X < Y > Z' does not behave the same as a mathematical expression}}
}
};
diff --git a/clang/test/SemaTemplate/typo-template-name.cpp b/clang/test/SemaTemplate/typo-template-name.cpp
index bc0038d8c6bd9..66e793505413c 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-error {{comparisons like 'X < Y > Z' don't have their mathematical meaning}}
+ foo<int()>(0); // expected-error {{chained comparison 'X < Y > Z' does not behave the same as a mathematical expression}}
foo<int(), true>(false);
foo<Base{}.n;
}
More information about the cfe-commits
mailing list