[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
@@ -36,7 +36,7 @@ namespace InExpr { // These are valid expressions. foo(0); +foo(0); // expected-warning {{comparisons like 'X<=Y<=Z' don't have their mathematical meaning}} foo(false); shafik wrote: It is a shame we don't catch this one but neither does gcc https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
@@ -85,7 +85,7 @@ void f(int x, int y, int z) { if ((ayyhttps://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
@@ -85,7 +85,7 @@ void f(int x, int y, int z) { if ((ayy` or something). So something like: `comparison 'a>yhttps://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
https://github.com/JOE1994 closed https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
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 Date: Tue, 14 May 2024 17:42:59 -0500 Subject: [PATCH 1/5] [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, DefaultIgnore; + def warn_enum_constant_in_bool_context : Warning< "converting the enum constant to a boolean">, InGroup, 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(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 Date: Wed, 15 May 2024 13:20:06 -0400 Subject: [PATCH 2/5] Update clang/lib/Sema/SemaExpr.cpp Co-authored-by: Aaron Ballman --- 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(LHSExpr)) - if (BI->isComparisonOp()) -Diag(OpLoc, diag::warn_consecutive_comparison); +if (const auto *BI = dyn_cast(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 Date: Wed, 15 May 2024 12:53:38 -0500 Subject: [PATCH 3/5] 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
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
@@ -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. AaronBallman wrote: ```suggestion - Clang emits a ``-Wparentheses`` warning for expressions with consecutive comparisons like ``x < y < z``. Fixes #GH20456. ``` https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
https://github.com/AaronBallman approved this pull request. LGTM aside from a small nit with the release notes. https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
@@ -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}} AaronBallman wrote: > I don't have the capacity to further make clang diagnose case 2 but not case > 3. > I added a TODO note at the bottom of the test lines for now. SGTM! https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
https://github.com/JOE1994 edited https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
@@ -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}} JOE1994 wrote: (I'll refer to each of the 3 cases mentioned in the above comment as case1, case2, and case 3) I pushed a commit that adds 2 `no-warning` test lines for case1. It's unlikely for one to write `((x < y) >= z)` and believe it to have mathematical meaning of `(x < y) AND (y >= z)`. I limited the scope of this PR to diagnosing consecutive "built-in" comparisons (as per the PR title). Thus, the current implementation does not diagnose case 2. I don't have the capacity to further work on making clang diagnose case 2 but not case 3. I added a TODO note at the bottom of the test lines for now. FYI, gcc doesn't diagnose any of case1, case2, or case 3. https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
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 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, DefaultIgnore; + def warn_enum_constant_in_bool_context : Warning< "converting the enum constant to a boolean">, InGroup, 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(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 Date: Wed, 15 May 2024 13:20:06 -0400 Subject: [PATCH 2/4] Update clang/lib/Sema/SemaExpr.cpp Co-authored-by: Aaron Ballman --- 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(LHSExpr)) - if (BI->isComparisonOp()) -Diag(OpLoc, diag::warn_consecutive_comparison); +if (const auto *BI = dyn_cast(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 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
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
https://github.com/JOE1994 edited https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
https://github.com/JOE1994 edited https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
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 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, DefaultIgnore; + def warn_enum_constant_in_bool_context : Warning< "converting the enum constant to a boolean">, InGroup, 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(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 Date: Wed, 15 May 2024 13:20:06 -0400 Subject: [PATCH 2/3] Update clang/lib/Sema/SemaExpr.cpp Co-authored-by: Aaron Ballman --- 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(LHSExpr)) - if (BI->isComparisonOp()) -Diag(OpLoc, diag::warn_consecutive_comparison); +if (const auto *BI = dyn_cast(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 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
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff c5cd049566a795ba5de88dfbb2eb563cad4a9d8a b3cc457efc40a345d4b67c776edd470e35f73de2 -- clang/lib/Sema/SemaExpr.cpp clang/test/Sema/parentheses.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index b4c54a3da4..65c400637a 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(LHSExpr); BI && BI->isComparisonOp()) +if (const auto *BI = dyn_cast(LHSExpr); +BI && BI->isComparisonOp()) Diag(OpLoc, diag::warn_consecutive_comparison); break; `` https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
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 Date: Tue, 14 May 2024 17:42:59 -0500 Subject: [PATCH 1/2] [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, DefaultIgnore; + def warn_enum_constant_in_bool_context : Warning< "converting the enum constant to a boolean">, InGroup, 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(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 Date: Wed, 15 May 2024 13:20:06 -0400 Subject: [PATCH 2/2] Update clang/lib/Sema/SemaExpr.cpp Co-authored-by: Aaron Ballman --- 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(LHSExpr)) - if (BI->isComparisonOp()) -Diag(OpLoc, diag::warn_consecutive_comparison); +if (const auto *BI = dyn_cast(LHSExpr); BI && BI->isComparisonOp()) + Diag(OpLoc, diag::warn_consecutive_comparison); break; case BO_EQ: ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
@@ -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}} AaronBallman wrote: I'd like an additional test along the lines of: ``` (void)((x < y) >= z); ``` and what about something along these lines? ``` struct S { bool operator<(const S &) const; } s, t; (void)(s < t >= z); ``` perhaps that should diagnose while this should not? ``` struct S { S operator<(const S &) const; } a, b; struct T { friend bool operator>=(const S &, const T &); } c; bool val = (a < b >= c); ``` https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
@@ -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(LHSExpr)) + if (BI->isComparisonOp()) +Diag(OpLoc, diag::warn_consecutive_comparison); + AaronBallman wrote: ```suggestion if (const auto *BI = dyn_cast(LHSExpr); BI && BI->isComparisonOp()) Diag(OpLoc, diag::warn_consecutive_comparison); ``` https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
JOE1994 wrote: > Do we know why GCC elected to go that route? This seems like something that > should be enabled by default because the false positive rate should be quite > low. It seems like `gcc` doesn't enable `-Wparentheses` by default, while `clang` enables it by default. I will add a follow-up commit to remove `DefaultIgnore` and update affected Clang tests :) https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
AaronBallman wrote: > Set the warning to DefaultIgnore, although -Wparentheses is enabled by > default Do we know why GCC elected to go that route? This seems like something that should be enabled by default because the false positive rate should be quite low. https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Youngsuk Kim (JOE1994) Changes 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 --- Full diff: https://github.com/llvm/llvm-project/pull/92200.diff 4 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+3) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4) - (modified) clang/lib/Sema/SemaExpr.cpp (+5) - (modified) clang/test/Sema/parentheses.cpp (+7) ``diff 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, DefaultIgnore; + def warn_enum_constant_in_bool_context : Warning< "converting the enum constant to a boolean">, InGroup, 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(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}} +} `` https://github.com/llvm/llvm-project/pull/92200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)
https://github.com/JOE1994 created https://github.com/llvm/llvm-project/pull/92200 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 >From 2c7f9a083c129df70a79d019286b6a29643a8973 Mon Sep 17 00:00:00 2001 From: Youngsuk Kim Date: Tue, 14 May 2024 17:42:59 -0500 Subject: [PATCH] [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, DefaultIgnore; + def warn_enum_constant_in_bool_context : Warning< "converting the enum constant to a boolean">, InGroup, 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(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}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits