[clang] Added warn-assignment-bool-context

This warning is issued if an assignment is used in a context
where a boolean result is required.

Author: PhilippRados

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8477,6 +8477,9 @@ def err_incomplete_object_call : Error<
 def warn_condition_is_assignment : Warning<"using the result of an "
   "assignment as a condition without parentheses">,
   InGroup<Parentheses>;
+def warn_assignment_bool_context : Warning<"suggest parentheses around assignment used as truth value">,
+  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,48 @@ void Sema::diagnoseZeroToNullptrConversion(CastKind Kind, const Expr *E) {
       << FixItHint::CreateReplacement(E->getSourceRange(), "nullptr");
 }
 
+void Sema::DiagnoseAssignmentBoolContext(Expr *E, QualType Ty) {
+  // Use copy to not alter original expression.
+  Expr *ECopy = E;
+
+  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.
+    // NOTE: Is this robust enough or can there be other semantic expression
+    // until the assignment?
+    while (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(ECopy)) {
+      // If there is another implicit cast to bool then this warning would have
+      // been already emitted.
+      if (ICE->getType()->isBooleanType())
+        return;
+      ECopy = ICE->getSubExpr();
+    }
+
+    if (BinaryOperator *Op = dyn_cast<BinaryOperator>(ECopy)) {
+      // 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)
+            << ECopy->getSourceRange();
+
+        SourceLocation Open = ECopy->getBeginLoc();
+        SourceLocation Close =
+            getLocForEndOfToken(ECopy->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 +803,17 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
     }
   }
 
+  // FIXME: Doesn't include C89, so this warning isn't emitted when passing
+  // `std=c89`.
+  auto isC = getLangOpts().C99 || getLangOpts().C11 || getLangOpts().C17 ||
+             getLangOpts().C23;
+  // Do not emit this warning for Objective-C, since it's a common idiom.
+  // NOTE: Are there other languages that this could affect besides C and C++?
+  // Ideally would check `getLangOpts().Cplusplus || getLangOpts().C` but there
+  // is no option for C (only C99 etc.).
+  if ((getLangOpts().CPlusPlus || isC) && !getLangOpts().ObjC)
+    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

@@ -20287,7 +20287,11 @@ void Sema::DiagnoseEqualityWithExtraParens(ParenExpr *ParenE) {
 
 ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
                                        bool IsConstexpr) {
-  DiagnoseAssignmentAsCondition(E);
+  // This warning is already covered by `warn_assignment_bool_context` in C++.
+  // NOTE: Ideally both warnings would be combined
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
+    DiagnoseAssignmentAsCondition(E);
+
   if (ParenExpr *parenE = dyn_cast<ParenExpr>(E))
     DiagnoseEqualityWithExtraParens(parenE);
 

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

@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -Wparentheses -verify %s
+
+// NOTE: Don't know if tests allow includes.
+#include <stdbool.h>
+
+// Do not emit the warning for compound-assignments. 
+bool f(int x) { return x = 0; }  // expected-warning {{suggest parentheses around assignment used as truth value}}\
+                                 // 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_condition_is_assignment` warning, since
+  // C doesn't do implicit conversion booleans for conditions.
+  if (x = 0) {} // expected-warning {{using the result of an assignment as a condition 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 condition 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 {{suggest parentheses around assignment used as truth value}}\
+                   // 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 {{suggest parentheses around assignment used as truth value}} \
+                                 // 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;
+
+  // Assignemnts inside of conditions should still emit the more specific `warn_condition_is_assignment` warning.
+  if (x = 0) {} // expected-warning {{using the result of an assignment as a condition 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 condition 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 {{suggest parentheses around assignment used as truth value}}\
+                     // 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 {{suggest parentheses around assignment used as truth value}} \
+                         // 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 {{suggest parentheses around assignment used as truth value}} \
+                           // expected-note{{place parentheses around the assignment to silence this warning}}
+  (void)bool(int(bool(x = 1))); // expected-warning {{suggest parentheses around assignment used as truth value}} \
+                                // expected-note{{place parentheses around the assignment to silence this warning}}
+  (void)bool(int(x = 1));
+
+  bool _a = x = 3; // expected-warning {{suggest parentheses around assignment used as truth value}} \
+                   // 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);
+}