[PATCH] D141414: [clang] add warning on shifting boolean type

[Angelo Matni via Phabricator via cfe-commits]

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<DiagGroup<"shift-overflow">>;
+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<DiagGroup<"shift-cast-dependent-overflow">>;
+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<DiagGroup<"shift-overflow">>;
+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<DiagGroup<"shift-cast-dependent-overflow">>;
+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">,

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits