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/3] [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/3] 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/3] 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; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits