xbolva00 updated this revision to Diff 205696.
xbolva00 added a comment.

Addressed review notes. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63082/new/

https://reviews.llvm.org/D63082

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Analysis/misc-ps.c
  test/Sema/integer-overflow.c
  test/Sema/parentheses.c
  test/Sema/parentheses.cpp
  test/Sema/warn-unreachable.c
  test/SemaCXX/constexpr-printing.cpp
  test/SemaCXX/integer-overflow.cpp
  test/SemaCXX/nested-name-spec.cpp
  test/SemaCXX/warn-unreachable.cpp

Index: test/SemaCXX/warn-unreachable.cpp
===================================================================
--- test/SemaCXX/warn-unreachable.cpp
+++ test/SemaCXX/warn-unreachable.cpp
@@ -186,13 +186,13 @@
 
 
 int test_enum_sizeof_arithmetic() {
-  if (BrownCow)
+  if (BrownCow) // expected-warning {{enum constant in boolean context}}
     return 1;
   return 2;
 }
 
 int test_enum_arithmetic() {
-  if (CowBrown)
+  if (CowBrown) // expected-warning {{enum constant in boolean context}}
     return 1;
   return 2; // expected-warning {{never be executed}}
 }
Index: test/SemaCXX/nested-name-spec.cpp
===================================================================
--- test/SemaCXX/nested-name-spec.cpp
+++ test/SemaCXX/nested-name-spec.cpp
@@ -402,9 +402,9 @@
 T1<C2::N1> var_1a;
 T1<C2:N1> var_1b;  // expected-error{{unexpected ':' in nested name specifier; did you mean '::'?}}
 template<int N> int F() {}
-int (*X1)() = (B1::B2 ? F<1> : F<2>);
+int (*X1)() = (B1::B2 ? F<1> : F<2>); // expected-warning {{enum constant in boolean context}}
 int (*X2)() = (B1:B2 ? F<1> : F<2>);  // expected-error{{unexpected ':' in nested name specifier; did you mean '::'?}}
-
+// expected-warning@-1 {{enum constant in boolean context}}
 // Bit fields + templates
 struct S7a {
   T1<B1::B2>::C1 m1 : T1<B1::B2>::N1;
Index: test/SemaCXX/integer-overflow.cpp
===================================================================
--- test/SemaCXX/integer-overflow.cpp
+++ test/SemaCXX/integer-overflow.cpp
@@ -41,13 +41,13 @@
   uint64_t not_overflow2 = (1ULL * ((uint64_t)(4608) * (1024 * 1024)) + 2ULL);
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
-  overflow = 4608 * 1024 * 1024 ?  4608 * 1024 * 1024 : 0;
+  overflow = 4608 * 1024 * 1024 ?  4608 * 1024 * 1024 : 0; // expected-warning {{'*' in boolean context}}
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   overflow =  0 ? 0 : 4608 * 1024 * 1024;
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
-  if (4608 * 1024 * 1024)
+  if (4608 * 1024 * 1024) // expected-warning {{'*' in boolean context}}
     return 0;
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
@@ -100,7 +100,7 @@
 #endif
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
-  while (4608 * 1024 * 1024);
+  while (4608 * 1024 * 1024); // expected-warning {{'*' in boolean context}}
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   while ((uint64_t)(4608 * 1024 * 1024));
@@ -121,7 +121,7 @@
   while ((uint64_t)((uint64_t)(4608 * 1024 * 1024) * (uint64_t)(4608 * 1024 * 1024)));
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
-  do { } while (4608 * 1024 * 1024);
+  do { } while (4608 * 1024 * 1024); // expected-warning {{'*' in boolean context}}
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   do { } while ((uint64_t)(4608 * 1024 * 1024));
Index: test/SemaCXX/constexpr-printing.cpp
===================================================================
--- test/SemaCXX/constexpr-printing.cpp
+++ test/SemaCXX/constexpr-printing.cpp
@@ -99,6 +99,7 @@
   a:b:return;
 }
 
+// expected-warning@+1 {{'*' in boolean context}}
 constexpr bool test_bool_printing(bool b) { return 1 / !(2*b | !(2*b)); } // expected-note 2{{division by zero}}
 constexpr bool test_bool_0 = test_bool_printing(false); // expected-error {{constant expr}} expected-note {{in call to 'test_bool_printing(false)'}}
 constexpr bool test_bool_1 = test_bool_printing(true); // expected-error {{constant expr}} expected-note {{in call to 'test_bool_printing(true)'}}
Index: test/Sema/warn-unreachable.c
===================================================================
--- test/Sema/warn-unreachable.c
+++ test/Sema/warn-unreachable.c
@@ -141,7 +141,9 @@
 void test_mul_and_zero(int x) {
   if (x & 0) calledFun(); // expected-warning {{will never be executed}}
   if (0 & x) calledFun(); // expected-warning {{will never be executed}}
+  // expected-warning@+1 {{'*' in boolean context}}
   if (x * 0) calledFun(); // expected-warning {{will never be executed}}
+  // expected-warning@+1 {{'*' in boolean context}}
   if (0 * x) calledFun(); // expected-warning {{will never be executed}}
 }
 
Index: test/Sema/parentheses.cpp
===================================================================
--- test/Sema/parentheses.cpp
+++ test/Sema/parentheses.cpp
@@ -20,6 +20,7 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
   // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:23-[[@LINE-6]]:23}:")"
 
+  // expected-warning@+1 {{'*' in boolean context}}
   (void)(x * (x == y) ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '*'}} \
                                 // expected-note {{place parentheses around the '*' expression to silence this warning}} \
                                 // expected-note {{place parentheses around the '?:' expression to evaluate it first}}
Index: test/Sema/parentheses.c
===================================================================
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@ -119,6 +119,7 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
   // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:23-[[@LINE-6]]:23}:")"
 
+  // expected-warning@+1 {{'*' in boolean context}}
   (void)(x * (x == y) ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '*'}} \
                                 // expected-note {{place parentheses around the '*' expression to silence this warning}} \
                                 // expected-note {{place parentheses around the '?:' expression to evaluate it first}}
Index: test/Sema/integer-overflow.c
===================================================================
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -34,13 +34,13 @@
   uint64_t not_overflow2 = (1ULL * ((uint64_t)(4608) * (1024 * 1024)) + 2ULL);
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
-  overflow = 4608 * 1024 * 1024 ?  4608 * 1024 * 1024 : 0;
+  overflow = 4608 * 1024 * 1024 ?  4608 * 1024 * 1024 : 0; // expected-warning {{'*' in boolean context}}
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   overflow =  0 ? 0 : 4608 * 1024 * 1024;
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
-  if (4608 * 1024 * 1024)
+  if (4608 * 1024 * 1024) // expected-warning {{'*' in boolean context}}
     return 0;
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
@@ -83,7 +83,7 @@
   }
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
-  while (4608 * 1024 * 1024);
+  while (4608 * 1024 * 1024); // expected-warning {{'*' in boolean context}}
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   while ((uint64_t)(4608 * 1024 * 1024));
@@ -101,7 +101,7 @@
   while ((uint64_t)((uint64_t)(4608 * 1024 * 1024) * (uint64_t)(4608 * 1024 * 1024)));
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
-  do { } while (4608 * 1024 * 1024);
+  do { } while (4608 * 1024 * 1024); // expected-warning {{'*' in boolean context}}
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   do { } while ((uint64_t)(4608 * 1024 * 1024));
Index: test/Analysis/misc-ps.c
===================================================================
--- test/Analysis/misc-ps.c
+++ test/Analysis/misc-ps.c
@@ -96,7 +96,7 @@
 extern int nR10376675;
 void barR10376675(int *x) {
   int *pm;
-  if (nR10376675 * 2) {
+  if (nR10376675 * 2) { // expected-warning {{'*' in boolean context}}
     int *pk  = bazR10376675();
     pm = pk; //expected-warning {{never read}}
   }
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11096,6 +11096,42 @@
   return true;
 }
 
+static void DiagnoseIntInBoolContext(Sema &S, const Expr *E) {
+  E = E->IgnoreParenImpCasts();
+  SourceLocation ExprLoc = E->getExprLoc();
+
+  if (const auto *BO = dyn_cast<BinaryOperator>(E)) {
+    BinaryOperator::Opcode Opc = BO->getOpcode();
+    if (Opc == BO_Mul)
+      S.Diag(ExprLoc, diag::warn_mul_in_bool_context);
+    if (Opc == BO_Shl)
+      S.Diag(ExprLoc, diag::warn_left_shift_in_bool_context);
+  }
+
+  const auto *DRE = dyn_cast<DeclRefExpr>(E);
+  if (DRE && S.getLangOpts().CPlusPlus) {
+    const auto *ECD = dyn_cast<EnumConstantDecl>(DRE->getDecl());
+    if (ECD && ECD->getInitVal() != 0 && ECD->getInitVal() != 1)
+      S.Diag(ExprLoc, diag::warn_enum_constant_in_bool_context);
+  }
+
+  if (const auto *CO = dyn_cast<ConditionalOperator>(E)) {
+    const auto *LHS = dyn_cast<IntegerLiteral>(CO->getTrueExpr());
+    const auto *RHS = dyn_cast<IntegerLiteral>(CO->getFalseExpr());
+    if (LHS && RHS) {
+      if ((LHS->getValue() == 0 || LHS->getValue() == 1) &&
+          (RHS->getValue() == 0 || RHS->getValue() == 1))
+        // Do not diagnose common idioms
+        return;
+      if (LHS->getValue() != 0 && LHS->getValue() != 0)
+        S.Diag(ExprLoc,
+               diag::warn_integer_constants_in_conditional_always_true);
+      else
+        S.Diag(ExprLoc, diag::warn_integer_constants_in_conditional);
+    }
+  }
+}
+
 static void
 CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC,
                         bool *ICContext = nullptr) {
@@ -11319,6 +11355,9 @@
 
   S.DiscardMisalignedMemberAddress(Target, E);
 
+  if (Target->isBooleanType())
+    DiagnoseIntInBoolContext(S, E);
+
   if (!Source->isIntegerType() || !Target->isIntegerType())
     return;
 
@@ -11517,6 +11556,8 @@
   if (isa<ConditionalOperator>(E)) {
     ConditionalOperator *CO = cast<ConditionalOperator>(E);
     CheckConditionalOperator(S, CO, CC, T);
+    if (T->isBooleanType())
+      DiagnoseIntInBoolContext(S, E);
     return;
   }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5605,6 +5605,24 @@
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_mul_in_bool_context : Warning<
+  "'*' in boolean context">,
+  InGroup<IntInBoolContext>;
+def warn_left_shift_in_bool_context : Warning<
+  "'<<' in boolean context; did you mean '<'?">,
+  InGroup<IntInBoolContext>;
+def warn_enum_constant_in_bool_context : Warning<
+  "enum constant in boolean context">,
+  InGroup<IntInBoolContext>;
+
+def warn_integer_constants_in_conditional : Warning<
+  "'?:' with integer constants in boolean context">,
+  InGroup<IntInBoolContext>;
+def warn_integer_constants_in_conditional_always_true : Warning<
+  "'?:' with integer constants in boolean context, the expression will always "
+  "evaluate to 'true'">,
+  InGroup<TautologicalConstantCompare>;
+
 def warn_logical_instead_of_bitwise : Warning<
   "use of logical '%0' with constant operand">,
   InGroup<DiagGroup<"constant-logical-operand">>;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -764,7 +764,7 @@
 def TypeSafety : DiagGroup<"type-safety">;
 
 def IncompatibleExceptionSpec : DiagGroup<"incompatible-exception-spec">;
-
+def IntInBoolContext : DiagGroup<"int-in-bool-context">;
 def IntToVoidPointerCast : DiagGroup<"int-to-void-pointer-cast">;
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
                                  [IntToVoidPointerCast]>;
@@ -795,6 +795,7 @@
     ForLoopAnalysis,
     Format,
     Implicit,
+    IntInBoolContext,
     InfiniteRecursion,
     MismatchedTags,
     MissingBraces,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to