[clang] Added warn-assignment-bool-context

clang/include/clang/AST/Expr.h

@@ -109,6 +109,7 @@ struct SubobjectAdjustment {
 /// required.
 class Expr : public ValueStmt {
   QualType TR;
+  bool IsInsideCondition;
 
 public:
   Expr() = delete;
@@ -119,7 +120,7 @@ class Expr : public ValueStmt {
 
 protected:
   Expr(StmtClass SC, QualType T, ExprValueKind VK, ExprObjectKind OK)
-      : ValueStmt(SC) {
+      : ValueStmt(SC), IsInsideCondition(false) {
     ExprBits.Dependent = 0;
     ExprBits.ValueKind = VK;
     ExprBits.ObjectKind = OK;
@@ -450,6 +451,12 @@ class Expr : public ValueStmt {
     return (OK == OK_Ordinary || OK == OK_BitField);
   }
 
+  bool isInsideCondition() const { return IsInsideCondition; }
+
+  /// setValueKind - Mark this expression to be inside a condition.
+  /// necessary for `warn_assignment_bool_context` diagnostic
+  void setIsInsideCondition() { IsInsideCondition = true; }
+
   /// setValueKind - Set the value kind produced by this expression.
   void setValueKind(ExprValueKind Cat) { ExprBits.ValueKind = Cat; }
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8474,9 +8474,9 @@ def err_cannot_form_pointer_to_member_of_reference_type : Error<
 def err_incomplete_object_call : Error<
   "incomplete type in call to object of type %0">;
 
-def warn_condition_is_assignment : Warning<"using the result of an "
-  "assignment as a condition without parentheses">,
+def warn_assignment_bool_context : Warning<"using the result of an assignment as a truth value without parentheses">,
   InGroup<Parentheses>;
+
 def warn_free_nonheap_object
   : Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression|object: lambda-to-function-pointer conversion}1">,
     InGroup<FreeNonHeapObject>;

clang/include/clang/Sema/Sema.h

@@ -7232,6 +7232,8 @@ class Sema final : public SemaBase {
   /// being used as a boolean condition, warn if it's an assignment.
   void DiagnoseAssignmentAsCondition(Expr *E);
 
+  void DiagnoseAssignmentBoolContext(Expr *E, QualType ToType);
+
   /// Redundant parentheses over an equality comparison can indicate
   /// that the user intended an assignment used as condition.
   void DiagnoseEqualityWithExtraParens(ParenExpr *ParenE);

clang/lib/Sema/Sema.cpp

@@ -687,6 +687,47 @@ void Sema::diagnoseZeroToNullptrConversion(CastKind Kind, const Expr *E) {
       << FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
 }
 
+void Sema::DiagnoseAssignmentBoolContext(Expr *E, QualType Ty) {
+  if (Ty->isBooleanType()) {
+    // `bool(x=0)` and if (x=0){} emit:
+    // - ImplicitCastExpr bool IntegralToBoolean
+    // -- ImplicitCastExpr int LValueToRValue
+    // --- Assignment ...
+    // But should still emit this warning (at least gcc does), even if bool-cast
+    // is not directly followed by assignment.
+    while (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
+      // If there is another implicit cast to bool then this warning would have
+      // been already emitted.
+      if (ICE->getType()->isBooleanType())
+        return;
+      E = ICE->getSubExpr();
+    }
+
+    // Condition-assignment warnings are already handled by
+    // `DiagnoseAssignmentAsCondition()`
+    if (E->isInsideCondition())
+      return;
+
+    if (BinaryOperator *Op = dyn_cast<BinaryOperator>(E)) {
+      // Should only be issued for regular assignment `=`,
+      // not for compound-assign like `+=`.
+      // NOTE: Might make sense to emit for all assignments even if gcc
+      // only does for regular assignment.
+      if (Op->getOpcode() == BO_Assign) {
+        SourceLocation Loc = Op->getOperatorLoc();
+        Diag(Loc, diag::warn_assignment_bool_context) << E->getSourceRange();
+
+        SourceLocation Open = E->getBeginLoc();
+        SourceLocation Close =
+            getLocForEndOfToken(E->getSourceRange().getEnd());
+        Diag(Loc, diag::note_condition_assign_silence)
+            << FixItHint::CreateInsertion(Open, "(")
+            << FixItHint::CreateInsertion(Close, ")");
+      }
+    }
+  }
+}
+
 /// ImpCastExprToType - If Expr is not of type 'Type', insert an implicit cast.
 /// If there is already an implicit cast, merge into the existing one.
 /// The result is of the given category.
@@ -761,6 +802,8 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
     }
   }
 
+  DiagnoseAssignmentBoolContext(E, Ty);
+
   if (ImplicitCastExpr *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {
     if (ImpCast->getCastKind() == Kind && (!BasePath || BasePath->empty())) {
       ImpCast->setType(Ty);

clang/lib/Sema/SemaExpr.cpp

@@ -20203,7 +20203,7 @@ bool Sema::CheckCallReturnType(QualType ReturnType, SourceLocation Loc,
 void Sema::DiagnoseAssignmentAsCondition(Expr *E) {
   SourceLocation Loc;
 
-  unsigned diagnostic = diag::warn_condition_is_assignment;
+  unsigned diagnostic = diag::warn_assignment_bool_context;
   bool IsOrAssign = false;
 
   if (BinaryOperator *Op = dyn_cast<BinaryOperator>(E)) {
@@ -20288,6 +20288,8 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr *ParenE) {
 ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
                                        bool IsConstexpr) {
   DiagnoseAssignmentAsCondition(E);
+  E->setIsInsideCondition();
+
   if (ParenExpr *parenE = dyn_cast<ParenExpr>(E))
     DiagnoseEqualityWithExtraParens(parenE);
 

clang/test/CodeCompletion/crash-skipped-bodies-template-inst.cpp

@@ -6,15 +6,15 @@ auto make_func() {
       int x;
       if (x = 10) {}
       // Check that body of this function is actually skipped.
-      // CHECK-NOT: crash-skipped-bodies-template-inst.cpp:7:{{[0-9]+}}: warning: using the result of an assignment as a condition without parentheses
+      // CHECK-NOT: crash-skipped-bodies-template-inst.cpp:7:{{[0-9]+}}: warning: using the result of an assignment as a truth value without parentheses
       return this;
     }
   };
 
   int x;
   if (x = 10) {}
   // Check that this function is not skipped.
-  // CHECK: crash-skipped-bodies-template-inst.cpp:15:9: warning: using the result of an assignment as a condition without parentheses
+  // CHECK: crash-skipped-bodies-template-inst.cpp:15:9: warning: using the result of an assignment as a truth value without parentheses
   return impl();
 }
 

clang/test/CodeCompletion/skip-auto-funcs.cpp

@@ -7,7 +7,7 @@ auto not_skipped() {
   int x;
   if (x = 10) {}
   // Check that this function is not skipped.
-  // CHECK: 8:9: warning: using the result of an assignment as a condition without parentheses
+  // CHECK: 8:9: warning: using the result of an assignment as a truth value without parentheses
   return 1;
 }
 
@@ -16,7 +16,7 @@ auto lambda_not_skipped = []() {
   int x;
   if (x = 10) {}
   // Check that this function is not skipped.
-  // CHECK: 17:9: warning: using the result of an assignment as a condition without parentheses
+  // CHECK: 17:9: warning: using the result of an assignment as a truth value without parentheses
   return 1;
 }
 
@@ -25,15 +25,15 @@ auto skipped() -> T {
   int x;
   if (x = 10) {}
   // Check that this function is skipped.
-  // CHECK-NOT: 26:9: warning: using the result of an assignment as a condition without parentheses
+  // CHECK-NOT: 26:9: warning: using the result of an assignment as a truth value without parentheses
   return 1;
 };
 
 auto lambda_skipped = []() -> int {
   int x;
   if (x = 10) {}
   // This could potentially be skipped, but it isn't at the moment.
-  // CHECK: 34:9: warning: using the result of an assignment as a condition without parentheses
+  // CHECK: 34:9: warning: using the result of an assignment as a truth value without parentheses
   return 1;
 };
 
@@ -42,7 +42,7 @@ decltype(auto)** not_skipped_ptr() {
   int x;
   if (x = 10) {}
   // Check that this function is not skipped.
-  // CHECK: 43:9: warning: using the result of an assignment as a condition without parentheses
+  // CHECK: 43:9: warning: using the result of an assignment as a truth value without parentheses
   return T();
 }
 
@@ -51,7 +51,7 @@ decltype(auto) not_skipped_decltypeauto() {
   int x;
   if (x = 10) {}
   // Check that this function is not skipped.
-  // CHECK: 52:9: warning: using the result of an assignment as a condition without parentheses
+  // CHECK: 52:9: warning: using the result of an assignment as a truth value without parentheses
   return 1;
 }
 

clang/test/Misc/warning-flags-enabled.c

@@ -4,7 +4,7 @@
 // We just check a few to make sure it's doing something sensible.
 //
 // CHECK: ext_unterminated_char_or_string
-// CHECK: warn_condition_is_assignment
+// CHECK: warn_assignment_bool_context
 // CHECK: warn_null_arg
 
 

clang/test/Modules/diag-pragma.c

@@ -6,7 +6,7 @@
 @import diag_pragma;
 
 int foo(int x) {
-  if (x = DIAG_PRAGMA_MACRO) // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  if (x = DIAG_PRAGMA_MACRO) // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
                              // expected-note {{place parentheses}} expected-note {{use '=='}}
     return 0;
   return 1;

clang/test/Modules/diag-pragma.cpp

@@ -40,7 +40,7 @@ int foo(int x) {
 
   void("bar" + x);
 
-  if (x = DIAG_PRAGMA_MACRO) // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+  if (x = DIAG_PRAGMA_MACRO) // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
                              // expected-note {{place parentheses}} expected-note {{use '=='}}
     return 0;
   return 1;

clang/test/OpenMP/crash-skipped-bodies-template-inst.cpp

@@ -9,15 +9,15 @@ auto make_func() {
         ;
       }
       // Check that body of this function is actually skipped.
-      // CHECK-NOT: crash-skipped-bodies-template-inst.cpp:7:{{[0-9]+}}: warning: using the result of an assignment as a condition without parentheses
+      // CHECK-NOT: crash-skipped-bodies-template-inst.cpp:7:{{[0-9]+}}: warning: using the result of an assignment as a truth value without parentheses
       return this;
     }
   };
 
   int x;
   if (x = 10) {}
   // Check that this function is not skipped.
-  // CHECK: crash-skipped-bodies-template-inst.cpp:18:9: warning: using the result of an assignment as a condition without parentheses
+  // CHECK: crash-skipped-bodies-template-inst.cpp:18:9: warning: using the result of an assignment as a truth value without parentheses
   return impl();
 }
 

clang/test/Sema/parentheses.c

@@ -5,7 +5,7 @@
 // Test the various warnings under -Wparentheses
 void if_assign(void) {
   int i;
-  if (i = 4) {} // expected-warning {{assignment as a condition}} \
+  if (i = 4) {} // expected-warning {{assignment as a truth value}} \
                 // expected-note{{place parentheses around the assignment to silence this warning}} \
                 // expected-note{{use '==' to turn this assignment into an equality comparison}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:7-[[@LINE-3]]:7}:"("
@@ -118,4 +118,4 @@ void conditional_op(int x, int y, _Bool b, void* p) {
 }
 
 // RUN: not %clang_cc1 -fsyntax-only -Wparentheses -Werror %s 2>&1 | FileCheck %s -check-prefix=CHECK-FLAG
-// CHECK-FLAG: error: using the result of an assignment as a condition without parentheses [-Werror,-Wparentheses]
+// CHECK-FLAG: error: using the result of an assignment as a truth value without parentheses [-Werror,-Wparentheses]

clang/test/Sema/warn-assignment-bool-context.c

@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -Wparentheses -verify %s
+
+#define bool _Bool
+#define true 1
+#define false 0
+
+// Do not emit the warning for compound-assignments. 
+bool f(int x) { return x = 0; }  // expected-warning {{using the result of an assignment as a truth value without parentheses}}\
+                                 // expected-note{{place parentheses around the assignment to silence this warning}}
+bool f2(int x) { return x += 0; }
+
+bool f3(bool x) { return x = 0; }
+
+void test() {
+  int x;
+
+  // This should emit the `warn_assignment_bool_context` warning once, since
+  // C doesn't do implicit conversion booleans for conditions.
+  if (x = 0) {} // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                // expected-note{{place parentheses around the assignment to silence this warning}}\
+                // expected-note{{use '==' to turn this assignment into an equality comparison}}
+  if (x = 4 && x){} // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                    // expected-note{{place parentheses around the assignment to silence this warning}}\
+                    // expected-note{{use '==' to turn this assignment into an equality comparison}}
+
+  (void)(bool)(x = 1);
+  (void)(bool)(int)(x = 1);
+
+
+  bool _a = x = 3; // expected-warning {{using the result of an assignment as a truth value without parentheses}}\
+                   // expected-note{{place parentheses around the assignment to silence this warning}}
+
+  // Shouldn't warn for above cases if parentheses were provided.
+  if ((x = 0)) {}
+  bool _b = (x = 3);
+}

clang/test/SemaCXX/warn-assignment-bool-context.cpp

@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wparentheses -verify %s
+
+// Do not emit the warning for compound-assignments. 
+bool f(int x) { return x = 0; }  // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                                 // expected-note{{place parentheses around the assignment to silence this warning}}
+bool f2(int x) { return x += 0; }
+
+bool f3(bool x) { return x = 0; }
+
+void test() {
+  int x;
+
+  // Assignments inside of conditions should still emit the more specific `==` fixits.
+  if (x = 0) {} // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                // expected-note{{use '==' to turn this assignment into an equality comparison}} \
+                // expected-note{{place parentheses around the assignment to silence this warning}}
+  if (x = 4 && x){} // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                    // expected-note{{use '==' to turn this assignment into an equality comparison}} \
+                    // expected-note{{place parentheses around the assignment to silence this warning}}
+
+
+  (void)bool(x = 1); // expected-warning {{using the result of an assignment as a truth value without parentheses}}\
+                     // expected-note{{place parentheses around the assignment to silence this warning}}
+  (void)(bool)(x = 1);
+
+  // This should still emit since the RHS is casted to `int` before being casted back to `bool`.
+  (void)bool(x = false); // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                         // expected-note{{place parentheses around the assignment to silence this warning}}
+
+  // Should only issue warning once, even if multiple implicit casts.
+  // FIXME: This only checks that warning occurs not how often.
+  (void)bool(bool(x = 1)); // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                           // expected-note{{place parentheses around the assignment to silence this warning}}
+  (void)bool(int(bool(x = 1))); // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                                // expected-note{{place parentheses around the assignment to silence this warning}}
+  (void)bool(int(x = 1));
+
+  bool _a = x = 3; // expected-warning {{using the result of an assignment as a truth value without parentheses}} \
+                   // expected-note{{place parentheses around the assignment to silence this warning}}
+
+  // Shouldn't warn for above cases if parentheses were provided.
+  if ((x = 0)) {}
+  (void)bool((x = 1));
+  bool _b= (x = 3);
+}