[clang] Warn on implicit boolean casts in more contexts (PR34180)

[m4tx]

Summary

int x;
return x = 5;

For a code like above, GCC produces a warning suggesting using parentheses about the assignment. This change makes clang produce similar warning to, suggesting either changing the operator to ==, or wrap the expression inside parentheses.

This is based off D45401.

[xbolva00]

Thanks !

You can check and take tests from my older patch https://reviews.llvm.org/D58878

[lebedev.ri]

Please always upload all patches with full context (-U99999)

[lebedev.ri]

This review seems to be stuck/dead, consider abandoning if no longer relevant.

clang/lib/Sema/SemaChecking.cpp

           << FD << getLangOpts().CPlusPlus11;
     }
   }
+
+  // C++ implicit casts are checked inside PerformImplicitConversion
+  if (!getLangOpts().CPlusPlus) {
+    if (ImplicitCastExpr *CastExpr = dyn_cast<ImplicitCastExpr>(RetValExp)) {
+      DiagnoseAssignmentAsCondition(CastExpr->IgnoreImpCasts());
+    }
+  }
 }
 
 //===--- CHECK: Floating-Point comparisons (-Wfloat-equal) ---------------===//

clang/lib/Sema/SemaExpr.cpp

 
 ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
                                        bool IsConstexpr) {
-  DiagnoseAssignmentAsCondition(E);
+  // C++ implicit casts are checked inside PerformImplicitConversion
+  if (!getLangOpts().CPlusPlus) {
+    DiagnoseAssignmentAsCondition(E);
+  }
   if (ParenExpr *parenE = dyn_cast<ParenExpr>(E))
     DiagnoseEqualityWithExtraParens(parenE);
 

clang/lib/Sema/SemaExprCXX.cpp

     FromType = From->getType();
   }
 
+  // Warn on assignments inside implicit casts (= instead of ==)
+  if (SCS.Second == ICK_Boolean_Conversion || FromType == Context.BoolTy) {
+    DiagnoseAssignmentAsCondition(From->IgnoreImpCasts());
+  }
+
   // If we're converting to an atomic type, first convert to the corresponding
   // non-atomic type.
   QualType ToAtomicType;

clang/test/Sema/parentheses.c

   if ((i = 4)) {}
 }
 
+_Bool return_assign(void) {
+  int i;
+  return i = 4; // expected-warning {{assignment as a condition}} \
+                // 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]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:12-[[@LINE-5]]:13}:"=="
+
+  return (i = 4);
+}
+
 void bitwise_rel(unsigned i) {
   (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \
                         // expected-note{{place parentheses around the '==' expression to silence this warning}} \

clang/test/Sema/parentheses.cpp

     // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+bool return_assign() {
+  int i;
+  return i = 4; // expected-warning {{assignment as a condition}} \
+                // 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]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:12-[[@LINE-5]]:13}:"=="
+
+  return (i = 4);
+}