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

[Angelo Matni via Phabricator via cfe-commits]

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())
+    Diag(Loc, diag::warn_shift_on_bool_type) << (Opc == BO_Shr)
+      << "'bool && (shift_count | desired_type_width - 1)'"
+      << "'bool & !shift_count'";
 
   // 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,9 @@
 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_on_bool_type : Warning<
+  "%select{left|right}0 shifting 'bool' will implicitly cast to 'int'; consider %select{%1|%2}0 "
+  "to avoid implicitly relying on the width of 'int'">, InGroup<DiagGroup<"shift-bool">>;
 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">,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D141414.491923.patch
Type: text/x-patch
Size: 5521 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230124/db05eea6/attachment-0001.bin>