[PATCH] D141414: [clang] add warning on shifting boolean type
angelomatnigoogle created this revision.
angelomatnigoogle added reviewers: arsenm, shafik.
Herald added a project: All.
angelomatnigoogle requested review of this revision.
Herald added subscribers: cfe-commits, wdng.
Herald added a project: clang.
Warn on operations "b << x" and "b >> x"
Github issue #28334
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D141414
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/warn-shift-bool.cpp
Index: clang/test/Sema/warn-shift-bool.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-shift-bool.cpp
@@ -0,0 +1,7 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wshift-cast-dependent-overflow -fblocks
-verify %s
+
+int f(int x) {
+ const bool b = 1;
+ return b << x; // expected-warning{{left shifting bool relies on implicit
cast type width; consider re-write 'bool && (shift_count | desired_type_width -
1)'}}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11945,11 +11945,22 @@
return LHSType;
}
+static void checkBooleanShift(Sema &S, ExprResult &LHS, SourceLocation Loc,
BinaryOperatorKind Opc) {
+ if (LHS.get()->IgnoreParenImpCasts()->getType()->isBooleanType()) {
+if (Opc == BO_Shl) {
+ S.Diag(Loc, diag::warn_shift_left_on_bool_type);
+} else if (Opc == BO_Shr) {
+ S.Diag(Loc, diag::warn_shift_right_on_bool_type);
+}
+ }
+}
+
// C99 6.5.7
QualType Sema::CheckShiftOperands(ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc, BinaryOperatorKind Opc,
bool IsCompAssign) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/false);
+ checkBooleanShift(*this, LHS, Loc, Opc);
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6603,6 +6603,11 @@
def warn_shift_result_gt_typewidth : Warning<
"signed shift result (%0) requires %1 bits to represent, but %2 only has "
"%3 bits">, InGroup>;
+def warn_shift_left_on_bool_type : Warning<
+ "left shifting bool relies on implicit cast type width; consider re-write
'bool && (shift_count | desired_type_width - 1)'">,
+ InGroup>;
+def warn_shift_right_on_bool_type : Warning<
+ "right shifting bool equals itself if shifting 0 bits or false otherwise;
consider re-write 'bool & !shift_count'">;
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">,
Index: clang/test/Sema/warn-shift-bool.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-shift-bool.cpp
@@ -0,0 +1,7 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wshift-cast-dependent-overflow -fblocks -verify %s
+
+int f(int x) {
+ const bool b = 1;
+ return b << x; // expected-warning{{left shifting bool relies on implicit cast type width; consider re-write 'bool && (shift_count | desired_type_width - 1)'}}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11945,11 +11945,22 @@
return LHSType;
}
+static void checkBooleanShift(Sema &S, ExprResult &LHS, SourceLocation Loc, BinaryOperatorKind Opc) {
+ if (LHS.get()->IgnoreParenImpCasts()->getType()->isBooleanType()) {
+if (Opc == BO_Shl) {
+ S.Diag(Loc, diag::warn_shift_left_on_bool_type);
+} else if (Opc == BO_Shr) {
+ S.Diag(Loc, diag::warn_shift_right_on_bool_type);
+}
+ }
+}
+
// C99 6.5.7
QualType Sema::CheckShiftOperands(ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc, BinaryOperatorKind Opc,
bool IsCompAssign) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/false);
+ checkBooleanShift(*this, LHS, Loc, Opc);
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6603,6 +6603,11 @@
def warn_shift_result_gt_typewidth : Warning<
"signed shift result (%0) requires %1 bits to represent, but %2 only has "
"%3 bits">, InGroup>;
+def warn_shift_left_on_bool_type : Warning<
+ "left shifting bool relies on implicit cast type width; consider re-write 'bool && (shift_count | desired_type_widt
[PATCH] D141414: [clang] add warning on shifting boolean type
angelomatnigoogle updated this revision to Diff 491923.
angelomatnigoogle added a comment.
Consolidated shift-bool warning; fixed relevant unit tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141414/new/
https://reviews.llvm.org/D141414
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExpr.cpp
clang/test/AST/Interp/shifts.cpp
clang/test/Analysis/svalbuilder-simplify-no-crash.c
clang/test/CXX/over/over.built/p18.cpp
clang/test/Sema/warn-shift-bool.cpp
Index: clang/test/Sema/warn-shift-bool.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-shift-bool.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int f(int x) {
+ const bool b = 1;
+ const bool c = b << x; // expected-warning {{left shifting 'bool' will
implicitly cast to 'int'; consider 'bool && (shift_count | desired_type_width -
1)' to avoid implicitly relying on the width of 'int'}}
+ const bool d = b >> x; // expected-warning {{right shifting 'bool' will
implicitly cast to 'int'; consider 'bool & !shift_count' to avoid implicitly
relying on the width of 'int'}}
+ return 0;
+}
Index: clang/test/CXX/over/over.built/p18.cpp
===
--- clang/test/CXX/over/over.built/p18.cpp
+++ clang/test/CXX/over/over.built/p18.cpp
@@ -56,7 +56,7 @@
(void)(i << 3);
(void)(f << 3); // expected-error {{invalid operands}}
- (void)(b << 3);
+ (void)(b << 3); // expected-warning {{left shifting 'bool' will implicitly
cast to 'int'; consider 'bool && (shift_count | desired_type_width - 1)' to
avoid implicitly relying on the width of 'int'}}
(void)(pi << 3); // expected-error {{invalid operands}}
(void)(pt << 3); // FIXME
(void)(t << 3);
@@ -69,7 +69,7 @@
(void)(i >> 3);
(void)(f >> 3); // expected-error {{invalid operands}}
- (void)(b >> 3);
+ (void)(b >> 3); // expected-warning {{right shifting 'bool' will implicitly
cast to 'int'; consider 'bool & !shift_count' to avoid implicitly relying on
the width of 'int'}}
(void)(pi >> 3); // expected-error {{invalid operands}}
(void)(pt >> 3); // FIXME
(void)(t >> 3);
Index: clang/test/Analysis/svalbuilder-simplify-no-crash.c
===
--- clang/test/Analysis/svalbuilder-simplify-no-crash.c
+++ clang/test/Analysis/svalbuilder-simplify-no-crash.c
@@ -9,5 +9,6 @@
void crashing(long a, _Bool b) {
(void)(a & 1 && 0);
b = a & 1;
- (void)(b << 1); // expected-warning{{core.UndefinedBinaryOperatorResult}}
+ (void)(b << 1); // expected-warning{{core.UndefinedBinaryOperatorResult}} \
+ // expected-warning {{left shifting 'bool' will implicitly
cast to 'int'; consider 'bool && (shift_count | desired_type_width - 1)' to
avoid implicitly relying on the width of 'int'}}
}
Index: clang/test/AST/Interp/shifts.cpp
===
--- clang/test/AST/Interp/shifts.cpp
+++ clang/test/AST/Interp/shifts.cpp
@@ -109,7 +109,13 @@
static_assert(m == 1, "");
constexpr unsigned char c = 0 << 8;
static_assert(c == 0, "");
- static_assert(true << 1, "");
+
+ constexpr unsigned b = true << 1; // expected-warning {{left shifting 'bool'
will implicitly cast to 'int'; consider 'bool && (shift_count |
desired_type_width - 1)' to avoid implicitly relying on the width of 'int'}} \
+// cxx17-warning {{left shifting 'bool'
will implicitly cast to 'int'; consider 'bool && (shift_count |
desired_type_width - 1)' to avoid implicitly relying on the width of 'int'}} \
+// ref-warning {{left shifting 'bool' will
implicitly cast to 'int'; consider 'bool && (shift_count | desired_type_width -
1)' to avoid implicitly relying on the width of 'int'}} \
+// ref-cxx17-warning {{left shifting
'bool' will implicitly cast to 'int'; consider 'bool && (shift_count |
desired_type_width - 1)' to avoid implicitly relying on the width of 'int'}}
+ static_assert(b, "");
+
static_assert(1 << (__INT_WIDTH__ +1) == 0, ""); // expected-error {{not an
integral constant expression}} \
// expected-note {{>=
width of type 'int'}} \
// cxx17-error {{not an
integral constant expression}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11950,6 +11950,10 @@
SourceLocation Loc, BinaryOperatorKind Opc,
bool IsCompAssign) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/false);
+ if (LHS.get()->IgnoreParenImpCasts()->getType()->isBooleanType())
+
[PATCH] D141414: [clang] add warning on shifting boolean type
angelomatnigoogle added a comment. Pardon me for losing sight on this change. Within the week, I will revisit this change based on the current outstanding comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141414/new/ https://reviews.llvm.org/D141414 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
