https://github.com/JOE1994 updated
https://github.com/llvm/llvm-project/pull/92200
>From 2c7f9a083c129df70a79d019286b6a29643a8973 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim <youngsuk....@hpe.com>
Date: Tue, 14 May 2024 17:42:59 -0500
Subject: [PATCH 1/4] [clang][Sema] Warn consecutive builtin comparisons in an
expression
Made the following decisions for consistency with `gcc 14.1`:
* Add warning under -Wparentheses
* Set the warning to DefaultIgnore, although -Wparentheses is enabled by default
* This warning is only issued when -Wparentheses is explicitly enabled
(via -Wparentheses or -Wall)
Closes #20456
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++++
clang/lib/Sema/SemaExpr.cpp | 5 +++++
clang/test/Sema/parentheses.cpp | 7 +++++++
4 files changed, 19 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ae699ebfc6038..13d58e69aeeb7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -487,6 +487,9 @@ Improvements to Clang's diagnostics
}
};
+- Clang emits a ``-Wparentheses`` warning for expressions with consecutive
comparisons like ``x < y < z``.
+ It was made a ``-Wparentheses`` warning to be consistent with gcc.
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6100fba510059..612043f410890 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6911,6 +6911,10 @@ def warn_precedence_bitwise_conditional : Warning<
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">,
+ InGroup<Parentheses>, DefaultIgnore;
+
def warn_enum_constant_in_bool_context : Warning<
"converting the enum constant to a boolean">,
InGroup<IntInBoolContext>, DefaultIgnore;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..fab34f4fa3e14 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14878,6 +14878,11 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation
OpLoc,
case BO_GT:
ConvertHalfVec = true;
ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
+
+ if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr))
+ if (BI->isComparisonOp())
+ Diag(OpLoc, diag::warn_consecutive_comparison);
+
break;
case BO_EQ:
case BO_NE:
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index 324d9b5f1e414..8e546461fb643 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -215,3 +215,10 @@ namespace PR20735 {
// fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
}
}
+
+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}}
+}
>From b3cc457efc40a345d4b67c776edd470e35f73de2 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim <joseph942...@gmail.com>
Date: Wed, 15 May 2024 13:20:06 -0400
Subject: [PATCH 2/4] Update clang/lib/Sema/SemaExpr.cpp
Co-authored-by: Aaron Ballman <aa...@aaronballman.com>
---
clang/lib/Sema/SemaExpr.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fab34f4fa3e14..b4c54a3da42c0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14879,9 +14879,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation
OpLoc,
ConvertHalfVec = true;
ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
- if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr))
- if (BI->isComparisonOp())
- Diag(OpLoc, diag::warn_consecutive_comparison);
+ if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr); BI &&
BI->isComparisonOp())
+ Diag(OpLoc, diag::warn_consecutive_comparison);
break;
case BO_EQ:
>From 8df35dcf7b802ce8e280930224575077a08e0541 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim <youngsuk....@hpe.com>
Date: Wed, 15 May 2024 12:53:38 -0500
Subject: [PATCH 3/4] Remove DefaultIgnore + clang-format
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +-
clang/lib/Sema/SemaExpr.cpp | 3 ++-
clang/test/Sema/bool-compare.c | 4 ++--
clang/test/SemaCXX/bool-compare.cpp | 4 ++--
clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp | 2 +-
clang/test/SemaTemplate/typo-dependent-name.cpp | 4 ++--
clang/test/SemaTemplate/typo-template-name.cpp | 2 +-
7 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 612043f410890..5ebf116a07bb7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6913,7 +6913,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>, DefaultIgnore;
+ InGroup<Parentheses>;
def warn_enum_constant_in_bool_context : Warning<
"converting the enum constant to a boolean">,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index b4c54a3da42c0..65c400637a73d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -14879,7 +14879,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation
OpLoc,
ConvertHalfVec = true;
ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc);
- if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr); BI &&
BI->isComparisonOp())
+ if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr);
+ BI && BI->isComparisonOp())
Diag(OpLoc, diag::warn_consecutive_comparison);
break;
diff --git a/clang/test/Sema/bool-compare.c b/clang/test/Sema/bool-compare.c
index 923ff4208e8be..2308dc4bf2bb2 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) {} // no warning
+ if (a>y<z) {} // expected-warning {{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) {} // no warning
+ if (z<a>y) {} // expected-warning {{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/bool-compare.cpp
b/clang/test/SemaCXX/bool-compare.cpp
index fe47633bb23b6..4e33e076a7de4 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) {} // no warning
+ if (a>y<z) {} // expected-warning {{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) {} // no warning
+ if (z<a>y) {} // expected-warning {{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 28ecbade1b1a9..b8397f41febc3 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}}
+ f<0>(q); // expected-error {{invalid operands to binary expression}} //
expected-warning {{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 88b2fc373b1f5..fb61b03e50109 100644
--- a/clang/test/SemaTemplate/typo-dependent-name.cpp
+++ b/clang/test/SemaTemplate/typo-dependent-name.cpp
@@ -20,11 +20,11 @@ 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;
+ return this->inner < other > ::z; // expected-warning {{comparisons like
'X<=Y<=Z' don't have their mathematical meaning}}
}
};
-void use_x(X<int> x) { x.f(0); }
+void use_x(X<int> x) { x.f(0); } // expected-note {{requested here}}
template<typename T>
struct Y {
diff --git a/clang/test/SemaTemplate/typo-template-name.cpp
b/clang/test/SemaTemplate/typo-template-name.cpp
index fe5201a8e26c3..741aa708e81fc 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);
+ foo<int()>(0); // expected-warning {{comparisons like 'X<=Y<=Z' don't have
their mathematical meaning}}
foo<int(), true>(false);
foo<Base{}.n;
}
>From cb3a778731ab70ab52a3b4a45c608d06731420ae Mon Sep 17 00:00:00 2001
From: Youngsuk Kim <youngsuk....@hpe.com>
Date: Thu, 16 May 2024 16:41:59 -0500
Subject: [PATCH 4/4] Add no-warning test lines + TODO note
Don't issue warning for expressions like `(X < Y) < Z`. It is unlikely for one
to write `(X < Y) < Z` and confuse it to have mathematical meaning of `(X<Y) &&
(Y<Z)`.
---
clang/test/Sema/parentheses.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang/test/Sema/parentheses.cpp b/clang/test/Sema/parentheses.cpp
index 8e546461fb643..e991cee16b0e2 100644
--- a/clang/test/Sema/parentheses.cpp
+++ b/clang/test/Sema/parentheses.cpp
@@ -221,4 +221,8 @@ 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); // no-warning
+ (void)((x < y) >= z); // no-warning
+
+ // TODO: Also issue warning for consecutive comparisons that use overloaded
comparison operators?
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
