[clang] [Clang] add -Wshift-bool warning to handle shifting of bool (PR #127336)
Oleksandr T. via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 20 08:09:03 PST 2025
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.tarasiuk at outlook.com>
Date: Sat, 15 Feb 2025 18:19:44 +0200
Subject: [PATCH 1/5] [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.tarasiuk at outlook.com>
Date: Thu, 20 Feb 2025 18:06:19 +0200
Subject: [PATCH 2/5] 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.tarasiuk at outlook.com>
Date: Thu, 20 Feb 2025 18:06:44 +0200
Subject: [PATCH 3/5] 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.tarasiuk at outlook.com>
Date: Thu, 20 Feb 2025 18:07:01 +0200
Subject: [PATCH 4/5] 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.tarasiuk at outlook.com>
Date: Thu, 20 Feb 2025 18:08:47 +0200
Subject: [PATCH 5/5] 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,
More information about the cfe-commits
mailing list