commit f9db9379e4633d4c6f8282758ac3c1e8269266c6
Author: Timo Sirainen <timo.sirainen@open-xchange.com>
Date:   Mon Mar 23 18:13:30 2020 +0200

    Added -Wstrict-bool

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 11b05ce7a5f5..27558ef20e28 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -524,6 +524,10 @@ public:
   /// bit-field with width '1' will evaluate to 0/1, but doesn't necessarily
   /// semantically correspond to a bool.
   bool isKnownToHaveBooleanValue(bool Semantic = true) const;
+  /// isKnownToHaveBooleanOrTrueFalseValue - Return true if
+  /// isKnownToHaveBooleanValue() returns true or if this is a constant
+  /// expression that returns 1 or 0
+  bool isKnownToHaveBooleanOrTrueFalseValue(const ASTContext &Ctx) const;
 
   /// isIntegerConstantExpr - Return the value if this expression is a valid
   /// integer constant expression.  If not a valid i-c-e, return None and fill
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index d8e66bdefca1..5e2abe338b13 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -553,6 +553,7 @@ def StaticLocalInInline : DiagGroup<"static-local-in-inline">;
 def GNUStaticFloatInit : DiagGroup<"gnu-static-float-init">;
 def StaticFloatInit : DiagGroup<"static-float-init", [GNUStaticFloatInit]>;
 def GNUStatementExpression : DiagGroup<"gnu-statement-expression">;
+def StrictBool : DiagGroup<"strict-bool">;
 def StringConcatation : DiagGroup<"string-concatenation">;
 def StringCompare : DiagGroup<"string-compare">;
 def StringPlusInt : DiagGroup<"string-plus-int">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e1d20fbcb433..8d0412f1ce14 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6735,6 +6735,22 @@ def note_logical_not_fix : Note<
 def note_logical_not_silence_with_parens : Note<
   "add parentheses around left hand side expression to silence this warning">;
 
+def warn_strict_bool_cmp_not_eq_ne : Warning<
+  "Boolean values can be compared only with == or !=">,
+  InGroup<StrictBool>, DefaultIgnore;
+def warn_strict_bool_not : Warning<
+  "Only boolean expressions supported in !expr">,
+  InGroup<StrictBool>, DefaultIgnore;
+def warn_strict_bool_expr : Warning<
+  "Expression must be boolean">,
+  InGroup<StrictBool>, DefaultIgnore;
+def warn_strict_bool_conversion : Warning<
+  "Implicitly converting boolean">,
+  InGroup<StrictBool>, DefaultIgnore;
+def warn_strict_bool_cmp_mismatch : Warning<
+  "Boolean expressions can be compared only against another boolean expression">,
+  InGroup<StrictBool>, DefaultIgnore;
+
 def err_invalid_this_use : Error<
   "invalid use of 'this' outside of a non-static member function">;
 def err_this_static_member_func : Error<
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index c6b2c47a48fb..fef3ee8443c4 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -200,6 +200,17 @@ bool Expr::isKnownToHaveBooleanValue(bool Semantic) const {
   return false;
 }
 
+bool Expr::isKnownToHaveBooleanOrTrueFalseValue(const ASTContext &Ctx) const {
+  if (isKnownToHaveBooleanValue())
+    return true;
+
+  if (isIntegerConstantExpr(Ctx)) {
+    uint64_t Value = getIntegerConstantExpr(Ctx)->getExtValue();
+    return Value == 1 || Value == 0;
+  }
+  return false;
+}
+
 // Amusing macro metaprogramming hack: check whether a class provides
 // a more specific implementation of getExprLoc().
 //
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index e87adf8ff302..3b8dab13eac5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10104,6 +10104,10 @@ Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType,
                          bool isObjCMethod,
                          const AttrVec *Attrs,
                          const FunctionDecl *FD) {
+  if (!lhsType->isBooleanType() && RetValExp->isKnownToHaveBooleanValue()) {
+    Diag(ReturnLoc, diag::warn_strict_bool_conversion);
+  }
+
   // Check if the return value is null but should not be.
   if (((Attrs && hasSpecificAttr<ReturnsNonNullAttr>(*Attrs)) ||
        (!isObjCMethod && isNonNullType(Context, lhsType))) &&
@@ -11277,12 +11281,26 @@ static bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
   return true;
 }
 
+static void CheckImplicitAssignmentBoolConversion(Sema &S, BinaryOperator *E) {
+  const Type *Target = S.Context.getCanonicalType(E->getLHS()->getType()).getTypePtr();
+  if (Target->isDependentType())
+    return;
+
+  // Diagnose implicit casts to bool.
+  if (!Target->isBooleanType()) {
+    if (E->getRHS()->isKnownToHaveBooleanValue())
+      S.Diag(E->getRHS()->getBeginLoc(), diag::warn_strict_bool_conversion);
+  }
+}
+
 /// Analyze the given simple or compound assignment for warning-worthy
 /// operations.
 static void AnalyzeAssignment(Sema &S, BinaryOperator *E) {
   // Just recurse on the LHS.
   AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
 
+  CheckImplicitAssignmentBoolConversion(S, E);
+
   // We want to recurse on the RHS as normal unless we're assigning to
   // a bitfield.
   if (FieldDecl *Bitfield = E->getLHS()->getSourceBitField()) {
@@ -11526,11 +11544,31 @@ static bool IsImplicitBoolFloatConversion(Sema &S, Expr *Ex, bool ToBool) {
           FloatCandidateBT && (FloatCandidateBT->isFloatingPoint()));
 }
 
+static void CheckImplicitArgumentBoolConversion(Sema &S, Expr *Ex) {
+  if (!isa<ImplicitCastExpr>(Ex))
+    return;
+
+  Expr *InnerE = Ex->IgnoreParenImpCasts();
+  const Type *Target = S.Context.getCanonicalType(Ex->getType()).getTypePtr();
+  if (Target->isDependentType())
+    return;
+
+  // Diagnose implicit casts to bool.
+  if (!Target->isBooleanType()) {
+    if (InnerE->isKnownToHaveBooleanValue())
+      S.Diag(Ex->getBeginLoc(), diag::warn_strict_bool_conversion);
+  } else {
+    if (!InnerE->isKnownToHaveBooleanValue())
+      S.Diag(Ex->getBeginLoc(), diag::warn_strict_bool_conversion);
+  }
+}
+
 static void CheckImplicitArgumentConversions(Sema &S, CallExpr *TheCall,
                                              SourceLocation CC) {
   unsigned NumArgs = TheCall->getNumArgs();
   for (unsigned i = 0; i < NumArgs; ++i) {
     Expr *CurrA = TheCall->getArg(i);
+    CheckImplicitArgumentBoolConversion(S, CurrA);
     if (!IsImplicitBoolFloatConversion(S, CurrA, true))
       continue;
 
@@ -11807,6 +11845,8 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
       S.DiagnoseAlwaysNonNullPointer(E, Expr::NPCK_NotNull, /*IsEqual*/ false,
                                      SourceRange(CC));
     }
+    if (!E->isKnownToHaveBooleanOrTrueFalseValue(S.Context))
+      S.Diag(E->getBeginLoc(), diag::warn_strict_bool_expr);
   }
 
   // If the we're converting a constant to an ObjC BOOL on a platform where BOOL
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 13db8362e065..371e81329059 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11081,6 +11081,23 @@ static void diagnoseLogicalNotOnLHSofCheck(Sema &S, ExprResult &LHS,
       << FixItHint::CreateInsertion(SecondClose, ")");
 }
 
+static void checkStrictBool(Sema &S, ExprResult &LHS,
+                            ExprResult &RHS,
+                            SourceLocation Loc,
+                            BinaryOperatorKind Opc) {
+  // only bool1 == bool2 and bool1 != bool2 comparisons are allowed
+  if (LHS.get()->isKnownToHaveBooleanValue() ||
+      RHS.get()->isKnownToHaveBooleanValue()) {
+    if (Opc != BO_EQ && Opc != BO_NE) {
+      S.Diag(Loc, diag::warn_strict_bool_cmp_not_eq_ne);
+    }
+  }
+  // boolean can be compared only against another boolean
+  if (LHS.get()->isKnownToHaveBooleanValue() != RHS.get()->isKnownToHaveBooleanValue()) {
+    S.Diag(Loc, diag::warn_strict_bool_cmp_mismatch);
+  }
+}
+
 // Returns true if E refers to a non-weak array.
 static bool checkForArray(const Expr *E) {
   const ValueDecl *D = nullptr;
@@ -11472,6 +11489,7 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
   }
 
   checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/true);
+  checkStrictBool(*this, LHS, RHS, Loc, Opc);
   if (!getLangOpts().CPlusPlus && BinaryOperator::isEqualityOp(Opc)) {
     CheckPtrComparisonWithNullChar(LHS, RHS);
     CheckPtrComparisonWithNullChar(RHS, LHS);
