https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/127336
>From 272385df25b791b50472b92e12157477d021a26f Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Sat, 15 Feb 2025 18:19:44 +0200 Subject: [PATCH 1/6] [Clang] add -Wshift-bool warning to handle shifting of bool --- clang/docs/ReleaseNotes.rst | 1 + clang/include/clang/Basic/DiagnosticGroups.td | 2 ++ .../clang/Basic/DiagnosticSemaKinds.td | 3 +++ clang/lib/Sema/SemaExpr.cpp | 6 ++++++ clang/test/Sema/shift-bool.cpp | 21 +++++++++++++++++++ 5 files changed, 33 insertions(+) create mode 100644 clang/test/Sema/shift-bool.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 6056a6964fbcc..d623d7daf05ea 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -138,6 +138,7 @@ Improvements to Clang's diagnostics - Fixed a bug where Clang's Analysis did not correctly model the destructor behavior of ``union`` members (#GH119415). - A statement attribute applied to a ``case`` label no longer suppresses 'bypassing variable initialization' diagnostics (#84072). +- The ``-Wshift-bool`` warning has been added to warn about shifting a ``boolean``. (#GH28334) Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 05e39899e6f25..193aea3a8dfc3 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -461,6 +461,8 @@ def DanglingField : DiagGroup<"dangling-field">; def DanglingInitializerList : DiagGroup<"dangling-initializer-list">; def DanglingGsl : DiagGroup<"dangling-gsl">; def ReturnStackAddress : DiagGroup<"return-stack-address">; +def ShiftBool: DiagGroup<"shift-bool">; + // Name of this warning in GCC def : DiagGroup<"return-local-addr", [ReturnStackAddress]>; def Dangling : DiagGroup<"dangling", [DanglingAssignment, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c4f0fc55b4a38..754cf1e14799b 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7115,6 +7115,9 @@ def warn_shift_result_sets_sign_bit : Warning< "signed shift result (%0) sets the sign bit of the shift expression's " "type (%1) and becomes negative">, InGroup<DiagGroup<"shift-sign-overflow">>, DefaultIgnore; +def warn_shift_bool : Warning< + "%select{left|right}0 shifting a `bool` implicitly converts it to 'int'">, + InGroup<ShiftBool>, DefaultIgnore; def warn_precedence_bitwise_rel : Warning< "%0 has lower precedence than %1; %1 will be evaluated first">, diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 3cd4010740d19..0816403b2df2a 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -11246,6 +11246,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS, if (S.getLangOpts().OpenCL) return; + if (LHS.get()->IgnoreParenImpCasts()->getType()->isBooleanType()) { + S.Diag(Loc, diag::warn_shift_bool) + << (Opc == BO_Shr) /*left|right*/ << LHS.get()->getSourceRange(); + return; + } + // Check right/shifter operand Expr::EvalResult RHSResult; if (RHS.get()->isValueDependent() || diff --git a/clang/test/Sema/shift-bool.cpp b/clang/test/Sema/shift-bool.cpp new file mode 100644 index 0000000000000..dfbc1a1e73307 --- /dev/null +++ b/clang/test/Sema/shift-bool.cpp @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -fsyntax-only -Wshift-bool -verify %s + +void t() { + int x = 10; + bool y = true; + + bool a = y << x; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool b = y >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool c = false << x; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool d = false >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool e = y << 5; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool f = y >> 5; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool g = y << -1; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool h = y >> -1; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool i = y << 0; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} + bool j = y >> 0; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} +} >From b521922a0773f4498387597adf0eb2346b355959 Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Thu, 20 Feb 2025 18:06:19 +0200 Subject: [PATCH 2/6] update release notes --- 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 d623d7daf05ea..903fe9c790e33 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -138,7 +138,7 @@ Improvements to Clang's diagnostics - Fixed a bug where Clang's Analysis did not correctly model the destructor behavior of ``union`` members (#GH119415). - A statement attribute applied to a ``case`` label no longer suppresses 'bypassing variable initialization' diagnostics (#84072). -- The ``-Wshift-bool`` warning has been added to warn about shifting a ``boolean``. (#GH28334) +- The ``-Wshift-bool`` warning has been added to warn about shifting a boolean. (#GH28334) Improvements to Clang's time-trace ---------------------------------- >From 87707c5d47e3a2010d5523c274053f8e80c4bbdc Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Thu, 20 Feb 2025 18:06:44 +0200 Subject: [PATCH 3/6] update diagnostic groups --- clang/include/clang/Basic/DiagnosticGroups.td | 1 - clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 193aea3a8dfc3..b775f92f33baf 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -461,7 +461,6 @@ def DanglingField : DiagGroup<"dangling-field">; def DanglingInitializerList : DiagGroup<"dangling-initializer-list">; def DanglingGsl : DiagGroup<"dangling-gsl">; def ReturnStackAddress : DiagGroup<"return-stack-address">; -def ShiftBool: DiagGroup<"shift-bool">; // Name of this warning in GCC def : DiagGroup<"return-local-addr", [ReturnStackAddress]>; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 754cf1e14799b..5564ebd480ddd 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7116,8 +7116,8 @@ def warn_shift_result_sets_sign_bit : Warning< "type (%1) and becomes negative">, InGroup<DiagGroup<"shift-sign-overflow">>, DefaultIgnore; def warn_shift_bool : Warning< - "%select{left|right}0 shifting a `bool` implicitly converts it to 'int'">, - InGroup<ShiftBool>, DefaultIgnore; + "right shifting a `bool` implicitly converts it to 'int'">, + InGroup<DiagGroup<"shift-bool">>, DefaultIgnore; def warn_precedence_bitwise_rel : Warning< "%0 has lower precedence than %1; %1 will be evaluated first">, >From a777bf7361c4525847190a2be1752479706ee7b3 Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Thu, 20 Feb 2025 18:07:01 +0200 Subject: [PATCH 4/6] handle only right shift --- clang/lib/Sema/SemaExpr.cpp | 6 +++--- clang/test/Sema/shift-bool.cpp | 19 +++++-------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 0816403b2df2a..1353d24163aa3 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -11246,9 +11246,9 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS, if (S.getLangOpts().OpenCL) return; - if (LHS.get()->IgnoreParenImpCasts()->getType()->isBooleanType()) { - S.Diag(Loc, diag::warn_shift_bool) - << (Opc == BO_Shr) /*left|right*/ << LHS.get()->getSourceRange(); + if (Opc == BO_Shr && + LHS.get()->IgnoreParenImpCasts()->getType()->isBooleanType()) { + S.Diag(Loc, diag::warn_shift_bool) << LHS.get()->getSourceRange(); return; } diff --git a/clang/test/Sema/shift-bool.cpp b/clang/test/Sema/shift-bool.cpp index dfbc1a1e73307..c531c3a301c90 100644 --- a/clang/test/Sema/shift-bool.cpp +++ b/clang/test/Sema/shift-bool.cpp @@ -4,18 +4,9 @@ void t() { int x = 10; bool y = true; - bool a = y << x; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} - bool b = y >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} - - bool c = false << x; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} - bool d = false >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} - - bool e = y << 5; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} - bool f = y >> 5; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} - - bool g = y << -1; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} - bool h = y >> -1; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} - - bool i = y << 0; // expected-warning {{left shifting a `bool` implicitly converts it to 'int'}} - bool j = y >> 0; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + bool a = y >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + bool b = false >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + bool c = y >> 5; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + bool d = y >> -1; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + bool e = y >> 0; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} } >From 02184d030bbf8360d42f61a6cd39ff37ff9758de Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Thu, 20 Feb 2025 18:08:47 +0200 Subject: [PATCH 5/6] cleanup --- clang/include/clang/Basic/DiagnosticGroups.td | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index b775f92f33baf..05e39899e6f25 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -461,7 +461,6 @@ def DanglingField : DiagGroup<"dangling-field">; def DanglingInitializerList : DiagGroup<"dangling-initializer-list">; def DanglingGsl : DiagGroup<"dangling-gsl">; def ReturnStackAddress : DiagGroup<"return-stack-address">; - // Name of this warning in GCC def : DiagGroup<"return-local-addr", [ReturnStackAddress]>; def Dangling : DiagGroup<"dangling", [DanglingAssignment, >From 0ff841ab6d2f47aee4dca6752a433d4c22a56988 Mon Sep 17 00:00:00 2001 From: Oleksandr T <oleksandr.taras...@outlook.com> Date: Fri, 21 Feb 2025 14:49:23 +0200 Subject: [PATCH 6/6] add additional test cases --- clang/test/Sema/shift-bool.cpp | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/clang/test/Sema/shift-bool.cpp b/clang/test/Sema/shift-bool.cpp index c531c3a301c90..17dc589f20790 100644 --- a/clang/test/Sema/shift-bool.cpp +++ b/clang/test/Sema/shift-bool.cpp @@ -4,9 +4,21 @@ void t() { int x = 10; bool y = true; - bool a = y >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} - bool b = false >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} - bool c = y >> 5; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} - bool d = y >> -1; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} - bool e = y >> 0; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + bool a = y << x; + bool b = y >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool c = false << x; + bool d = false >> x; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool e = y << 1; + bool f = y >> 1; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool g = y << -1; // expected-warning {{shift count is negative}} + bool h = y >> -1; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + bool i = y << 0; + bool j = y >> 0; // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} + + if ((y << 1) != 0) { } + if ((y >> 1) != 0) { } // expected-warning {{right shifting a `bool` implicitly converts it to 'int'}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits