Index: test/SemaCXX/warn-bool-compare.cpp =================================================================== --- test/SemaCXX/warn-bool-compare.cpp (revision 0) +++ test/SemaCXX/warn-bool-compare.cpp (revision 0) @@ -0,0 +1,245 @@ +// RUN: %clang-cc1 -fsyntax-only -verify %s -Wbool-compare +// RUN: cp %s %t +// RUN: %clang-cc1 -fsyntax-only %t -x c++ -Wbool-compare -fixit +// RUN: %clang-cc1 -fsyntax-only %t -x c++ -Werror -Wno-bool-compare -Wbool-compare-logical-not -Wbool-compare-redundant -Wbool-compare-equivalent + +void testlogicalnot(int i, int j) { + // expected-warning@+1 {{logical not is only applied to the left hand side of this comparison}} + if (!i < j) {} + // expected-warning@+1 {{logical not is only applied to the left hand side of this comparison}} + if (!i > j) {} + // expected-warning@+1 {{logical not is only applied to the left hand side of this comparison}} + if (!i <= j) {} + // expected-warning@+1 {{logical not is only applied to the left hand side of this comparison}} + if (!i >= j) {} + // expected-warning@+1 {{logical not is only applied to the left hand side of this comparison}} + if (!i == j) {} + // expected-warning@+1 {{logical not is only applied to the left hand side of this comparison}} + if (!i != j) {} +} + +void testfloat(bool b, float f) { + // expected-warning@+1 {{comparison of boolean with float}} + if (b < f) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (b > f) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (b <= f) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (b >= f) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (b == f) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (b != f) {} + + // expected-warning@+1 {{comparison of boolean with float}} + if (f < b) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (f > b) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (f <= b) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (f >= b) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (f == b) {} + // expected-warning@+1 {{comparison of boolean with float}} + if (f != b) {} +} + +void testenum(bool b) { + enum E { A }; + + // expected-warning@+1 {{comparison of boolean with enum}} + if (b < A) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (b > A) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (b <= A) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (b >= A) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (b == A) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (b != A) {} + + // expected-warning@+1 {{comparison of boolean with enum}} + if (A < b) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (A > b) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (A <= b) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (A >= b) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (A == b) {} + // expected-warning@+1 {{comparison of boolean with enum}} + if (A != b) {} +} + +void testredundant(bool b) { + // expected-warning@+1 {{comparison of boolean expression greater than 0 is redundant}} + if (b > 0) {} + // expected-warning@+1 {{comparison of boolean expression not equal to 0 is redundant}} + if (b != 0) {} + // expected-warning@+1 {{comparison of 0 less than bool expression is redundant}} + if (0 < b) {} + // expected-warning@+1 {{comparison of boolean expression not equal to 0 is redundant}} + if (0 != b) {} + + // expected-warning@+1 {{comparison of boolean expression greater than or equal 1 is redundant}} + if (b >= 1) {} + // expected-warning@+1 {{comparison of boolean expression equal to 1 is redundant}} + if (b == 1) {} + // expected-warning@+1 {{comparison of 1 greater than or equal to bool expression is redundant}} + if (1 <= b) {} + // expected-warning@+1 {{comparison of boolean expression equal to 1 is redundant}} + if (1 == b) {} + + // expected-warning@+1 {{comparison of boolean expression greater than false is redundant}} + if (b > false) {} + // expected-warning@+1 {{comparison of boolean expression not equal to false is redundant}} + if (b != false) {} + // expected-warning@+1 {{comparison of false less than bool expression is redundant}} + if (false < b) {} + // expected-warning@+1 {{comparison of boolean expression not equal to false is redundant}} + if (false != b) {} + + // expected-warning@+1 {{comparison of boolean expression greater than or equal true is redundant}} + if (b >= true) {} + // expected-warning@+1 {{comparison of boolean expression equal to true is redundant}} + if (b == true) {} + // expected-warning@+1 {{comparison of true greater than or equal to bool expression is redundant}} + if (true <= b) {} + // expected-warning@+1 {{comparison of boolean expression equal to true is redundant}} + if (true == b) {} +} + +void testequivalent(bool b) { + // expected-warning@+1 {{comparison of boolean expression less than or equal to 0 is equivalent to negating the boolean expression}} + if (b <= 0) {} + // expected-warning@+1 {{comparison of boolean expression equal to 0 is equivalent to negating the boolean expression}} + if (b == 0) {} + // expected-warning@+1 {{comparison of 0 greater than or equal to boolean expression is equivalent to negating the boolean expression}} + if (0 >= b) {} + // expected-warning@+1 {{comparison of boolean expression equal to 0 is equivalent to negating the boolean expression}} + if (0 == b) {} + + // expected-warning@+1 {{comparison of boolean expression less than 1 is equivalent to negating the boolean expression}} + if (b < 1) {} + // expected-warning@+1 {{comparison of boolean expression not equal to 1 is equivalent to negating the boolean expression}} + if (b != 1) {} + // expected-warning@+1 {{comparison of 1 greater that boolean expression is equivalent to negating the boolean expression}} + if (1 > b) {} + // expected-warning@+1 {{comparison of boolean expression not equal to 1 is equivalent to negating the boolean expression}} + if (1 != b) {} + + // expected-warning@+1 {{comparison of boolean expression less than or equal to false is equivalent to negating the boolean expression}} + if (b <= false) {} + // expected-warning@+1 {{comparison of boolean expression equal to false is equivalent to negating the boolean expression}} + if (b == false) {} + // expected-warning@+1 {{comparison of false greater than or equal to boolean expression is equivalent to negating the boolean expression}} + if (false >= b) {} + // expected-warning@+1 {{comparison of boolean expression equal to false is equivalent to negating the boolean expression}} + if (false == b) {} + + // expected-warning@+1 {{comparison of boolean expression less than true is equivalent to negating the boolean expression}} + if (b < true) {} + // expected-warning@+1 {{comparison of boolean expression not equal to true is equivalent to negating the boolean expression}} + if (b != true) {} + // expected-warning@+1 {{comparison of true greater that boolean expression is equivalent to negating the boolean expression}} + if (true > b) {} + // expected-warning@+1 {{comparison of boolean expression not equal to true is equivalent to negating the boolean expression}} + if (true != b) {} +} + +void testtaut(bool b) { + // Tautological false + // expected-warning@+1 {{comparison of a boolean expression less than 0 is always false}} + if (b < 0) {} + // expected-warning@+1 {{comparison of 0 greater than a boolean expression is always false}} + if (0 > b) {} + // expected-warning@+1 {{comparison of a boolean expression greater than 1 is always false}} + if (b > 1) {} + // expected-warning@+1 {{comparison of 1 less than a boolean expression is always false}} + if (1 < b) {} + // expected-warning@+1 {{comparison of a boolean expression less than false is always false}} + if (b < false) {} + // expected-warning@+1 {{comparison of false greater than a boolean expression is always false}} + if (false > b) {} + // expected-warning@+1 {{comparison of false less than or equal to a boolean expression is always true}} + if (false <= b) {} + // expected-warning@+1 {{comparison of a boolean expression greater than true is always false}} + if (b > true) {} + + // Tautological true + // expected-warning@+1 {{comparison of a boolean expression greater than or equal to 0 is always true}} + if (b >= 0) {} + // expected-warning@+1 {{comparison of 0 less than or equal to a boolean expression is always true}} + if (0 <= b) {} + // expected-warning@+1 {{comparison of a boolean expression less than or equal to 1 is always true}} + if (b <= 1) {} + // expected-warning@+1 {{comparison of 1 greater than or equal to a boolean expression is always true}} + if (1 >= b) {} + // expected-warning@+1 {{comparison of a boolean expression greater than or equal to false is always true}} + if (b >= false) {} + // expected-warning@+1 {{comparison of a boolean expression less than or equal to true is always true}} + if (b <= true) {} + // expected-warning@+1 {{comparison of true less than a boolean expression is always false}} + if (true < b) {} + // expected-warning@+1 {{comparison of true greater than or equal to a boolean expression is always true}} + if (true >= b) {} +} + +void testoutofrange(bool b) { + // Always false + // expected-warning@+1 {{comparison of boolean expression to 5 is always false}} + if (b > 5) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always false}} + if (b >= 5) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always false}} + if (b == 5) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always false}} + if (5 < b) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always false}} + if (5 <= b) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always false}} + if (5 == b) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always false}} + if (b < -1) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always false}} + if (b <= -1) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always false}} + if (b == -1) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always false}} + if (-1 > b) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always false}} + if (-1 >= b) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always false}} + if (-1 == b) {} + + // Always true + // expected-warning@+1 {{comparison of boolean expression to 5 is always true}} + if (b < 5) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always true}} + if (b <= 5) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always true}} + if (b != 5) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always true}} + if (5 > b) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always true}} + if (5 >= b) {} + // expected-warning@+1 {{comparison of boolean expression to 5 is always true}} + if (5 != b) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always true}} + if (b > -1) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always true}} + if (b >= -1) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always true}} + if (b != -1) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always true}} + if (-1 < b) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always true}} + if (-1 <= b) {} + // expected-warning@+1 {{comparison of boolean expression to -1 is always true}} + if (-1 != b) {} +} Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td (revision 166499) +++ include/clang/Basic/DiagnosticGroups.td (working copy) @@ -27,6 +27,27 @@ def Availability : DiagGroup<"availability">; def Section : DiagGroup<"section">; def AutoImport : DiagGroup<"auto-import">; + +def BoolCompareFloat : DiagGroup<"bool-compare-float">; +def BoolCompareEnum : DiagGroup<"bool-compare-enum">; +def BoolCompareLogicalNot : DiagGroup<"bool-compare-logical-not">; +def BoolCompareTautologicalOutOfRange : + DiagGroup<"bool-compare-tautological-out-of-range">; +def BoolCompareTautological : DiagGroup<"bool-compare-tautological", + [BoolCompareTautologicalOutOfRange]>; +def BoolCompareRedundant : DiagGroup<"bool-compare-redundant">; +def BoolCompareEquivalent : DiagGroup<"bool-compare-equivalent">; + +def BoolCompare : DiagGroup<"bool-compare", [ + BoolCompareFloat, + BoolCompareEnum, + BoolCompareLogicalNot, + BoolCompareTautological, + BoolCompareRedundant, + BoolCompareEquivalent, + BoolCompareTautologicalOutOfRange + ]>; + def ConstantConversion : DiagGroup<"constant-conversion">; def LiteralConversion : DiagGroup<"literal-conversion">; def StringConversion : DiagGroup<"string-conversion">; @@ -204,9 +225,12 @@ def SizeofArrayArgument : DiagGroup<"sizeof-array-argument">; def StringPlusInt : DiagGroup<"string-plus-int">; def StrncatSize : DiagGroup<"strncat-size">; -def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">; +def TautologicalOutOfRangeCompare : + DiagGroup<"tautological-constant-out-of-range-compare", + [BoolCompareTautologicalOutOfRange]>; def TautologicalCompare : DiagGroup<"tautological-compare", - [TautologicalOutOfRangeCompare]>; + [TautologicalOutOfRangeCompare, + BoolCompareTautological]>; def HeaderHygiene : DiagGroup<"header-hygiene">; def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">; def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-type">; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td (revision 166499) +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy) @@ -4129,6 +4129,66 @@ "comparison between NULL and non-pointer " "%select{(%1 and NULL)|(NULL and %1)}0">, InGroup>; +def warn_logical_not_on_lhs_of_comparison : Warning< + "logical not is only applied to the left hand side of this comparison">, + InGroup; +def warn_bool_enum_comparison : Warning< + "comparison of boolean with enum">, + InGroup; +def warn_bool_float_comparison : Warning< + "comparison of boolean with float">, + InGroup; +def warn_lbool_taut_compare_0 : Warning< + "comparison of a boolean expression " + "%select{less than|greater than or equal to}0 %select{0|false}1 is always " + "%select{false|true}0">, InGroup; +def warn_rbool_taut_compare_0 : Warning< + "comparison of %select{0|false}1 " + "%select{greater than|less than or equal to}0 " + "a boolean expression is always %select{false|true}0">, + InGroup; +def warn_lbool_taut_compare_1 : Warning< + "comparison of a boolean expression " + "%select{greater than|less than or equal to}0 %select{1|true}1 is always " + "%select{false|true}0">, InGroup; +def warn_rbool_taut_compare_1 : Warning< + "comparison of %select{1|true}1 %select{less than|greater than or equal to}0 " + "a boolean expression is always %select{false|true}0">, + InGroup; +def warn_redundant_bool_equality_compare : Warning< + "comparison of boolean expression " + "%select{not equal to %select{0|false}1|equal to %select{1|true}1}0 " + "is redundant">, + InGroup, DefaultIgnore; +def warn_redundant_lbool_compare : Warning< + "comparison of boolean expression %select{" + "greater than %select{0|false}1|greater than or equal %select{1|true}1}0 " + "is redundant">, + InGroup, DefaultIgnore; +def warn_redundant_rbool_compare : Warning< + "comparison of %select{" + "%select{0|false}1 less than|%select{1|true}1 greater than or equal to}0 " + "bool expression is redundant">, + InGroup, DefaultIgnore; +def warn_equivalent_bool_equality_compare : Warning< + "comparison of boolean expression " + "%select{equal to %select{0|false}1|not equal to %select{1|true}1}0 " + "is equivalent to negating the boolean expression">, + InGroup, DefaultIgnore; +def warn_equivalent_lbool_compare : Warning< + "comparison of boolean expression " + "%select{less than or equal to " + "%select{0|false}1|less than %select{1|true}1}0 " + "is equivalent to negating the boolean expression">, + InGroup, DefaultIgnore; +def warn_equivalent_rbool_compare : Warning< + "comparison of %select{" + "%select{0|false}1 greater than or equal to|%select{1|true}1 greater that}0 " + "boolean expression is equivalent to negating the boolean expression">, + InGroup, DefaultIgnore; +def warn_bool_compare_out_of_range : Warning< + "comparison of boolean expression to %0 is always %select{false|true}1">, + InGroup; def err_invalid_this_use : Error< "invalid use of 'this' outside of a non-static member function">; Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp (revision 166499) +++ lib/Sema/SemaChecking.cpp (working copy) @@ -4369,6 +4369,212 @@ AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); } +// Warn on !x < y since the ! binds tighter than <. Suggest !(x < y) as a fix. +static bool AnalyzeLHSLogicalNot(Sema &S, BinaryOperator *BO) { + if (S.getDiagnostics().getDiagnosticLevel( + diag::warn_logical_not_on_lhs_of_comparison, + BO->getLocStart()) + == DiagnosticsEngine::Ignored) + return false; + + UnaryOperator *UO = dyn_cast(BO->getLHS()->IgnoreImpCasts()); + if (!UO || UO->getOpcode() != UO_LNot) return false; + + S.Diag(UO->getOperatorLoc(), diag::warn_logical_not_on_lhs_of_comparison) + << BO->getOperatorLoc() + << FixItHint::CreateInsertion(UO->getSubExpr()->getLocStart(), "(") + << FixItHint::CreateInsertion(S.PP.getLocForEndOfToken(BO->getLocEnd()), + ")"); + + return true; +} + +static bool AnalyzeBoolConstantCompare(Sema &S, BinaryOperator *BO, + bool LeftBoolean, Expr* BoolExpr, + Expr* ConstantExpr) { + llvm::APSInt Val; + // Ignore constant bool expressions. + if (BoolExpr->isIntegerConstantExpr(S.Context)) + return false; + // Only warn if ConstantExpr is known. + if (!ConstantExpr->EvaluateAsInt(Val, S.Context)) + return false; + + bool ConstantIsBool = ConstantExpr->getType()->isBooleanType(); + + BinaryOperatorKind Op = BO->getOpcode(); + SourceLocation LHSStart = LeftBoolean ? BoolExpr->getLocStart() + : ConstantExpr->getLocStart(); + SourceLocation RHSStart = LeftBoolean ? ConstantExpr->getLocStart() + : BoolExpr->getLocStart(); + SourceLocation LHSEnd = LeftBoolean ? BoolExpr->getLocEnd() + : ConstantExpr->getLocEnd(); + SourceLocation RHSEnd = LeftBoolean ? ConstantExpr->getLocEnd() + : BoolExpr->getLocEnd(); + SourceRange LHSAndOp(LHSStart, RHSStart.getLocWithOffset(-1)); + SourceRange RHSAndOp(S.PP.getLocForEndOfToken(LHSEnd), RHSEnd); + + if (Val == 0) { + if (LeftBoolean) { + if (Op == BO_LT || Op == BO_GE) { // (bool < 0) or (bool >= 0) + S.Diag(BO->getOperatorLoc(), diag::warn_lbool_taut_compare_0) + << (Op == BO_GE) << ConstantIsBool; + } else if (Op == BO_GT) { // (bool > 0) + S.Diag(BO->getOperatorLoc(), diag::warn_redundant_lbool_compare) + << 0 << ConstantIsBool + << FixItHint::CreateRemoval(RHSAndOp); + } else if (Op == BO_LE) { // (bool < 0) + S.Diag(BO->getOperatorLoc(), diag::warn_equivalent_lbool_compare) + << 0 << ConstantIsBool + << FixItHint::CreateRemoval(RHSAndOp) + << FixItHint::CreateInsertion(LHSStart, "!(") + << FixItHint::CreateInsertion(S.PP.getLocForEndOfToken(LHSEnd), + ")"); + } + } else { // Boolean on right. + if (Op == BO_GT || Op == BO_LE) { // (0 > bool) or (0 <= bool) + S.Diag(BO->getOperatorLoc(), diag::warn_rbool_taut_compare_0) + << (Op == BO_LE) << ConstantIsBool; + } else if (Op == BO_GE) { // (0 > bool) + S.Diag(BO->getOperatorLoc(), diag::warn_equivalent_rbool_compare) + << 0 << ConstantIsBool + << FixItHint::CreateRemoval(LHSAndOp) + << FixItHint::CreateInsertion(RHSStart, "!(") + << FixItHint::CreateInsertion(S.PP.getLocForEndOfToken(RHSEnd), + ")"); + } else if (Op == BO_LT) { // (0 < bool) + S.Diag(BO->getOperatorLoc(), diag::warn_redundant_rbool_compare) + << 0 << ConstantIsBool + << FixItHint::CreateRemoval(LHSAndOp); + } + } + if (Op == BO_NE) { // (bool != 0) or (0 != bool) + S.Diag(BO->getOperatorLoc(), + diag::warn_redundant_bool_equality_compare) + << 0 << ConstantIsBool + << FixItHint::CreateRemoval(LeftBoolean ? RHSAndOp : LHSAndOp); + } else if (Op == BO_EQ) { // (bool == 0) or (0 == bool) + S.Diag(BO->getOperatorLoc(), + diag::warn_equivalent_bool_equality_compare) + << 0 << ConstantIsBool + << FixItHint::CreateRemoval(LeftBoolean ? RHSAndOp : LHSAndOp) + << FixItHint::CreateInsertion(LeftBoolean ? LHSStart : RHSStart, "!(") + << FixItHint::CreateInsertion( + S.PP.getLocForEndOfToken(LeftBoolean ? LHSEnd : RHSEnd), ")"); + } + } else if (Val == 1) { + if (LeftBoolean) { + if (Op == BO_LE || Op == BO_GT) { // (bool <= 1) or (bool > 1) + S.Diag(BO->getOperatorLoc(), diag::warn_lbool_taut_compare_1) + << (Op == BO_LE) << ConstantIsBool; + } else if (Op == BO_GE) { // (bool >= 1) + S.Diag(BO->getOperatorLoc(), diag::warn_redundant_lbool_compare) + << 1 << ConstantIsBool + << FixItHint::CreateRemoval(RHSAndOp); + } else if (Op == BO_LT) { // (bool < 1) + S.Diag(BO->getOperatorLoc(), diag::warn_equivalent_lbool_compare) + << 1 << ConstantIsBool + << FixItHint::CreateRemoval(RHSAndOp) + << FixItHint::CreateInsertion(LHSStart, "!(") + << FixItHint::CreateInsertion(S.PP.getLocForEndOfToken(LHSEnd), + ")"); + } + } else { // Boolean on right. + if (Op == BO_LT || Op == BO_GE) { // (1 < bool) or (1 >= bool) + S.Diag(BO->getOperatorLoc(), diag::warn_rbool_taut_compare_1) + << (Op == BO_GE) << ConstantIsBool; + } else if (Op == BO_GT) { // (1 > bool) + S.Diag(BO->getOperatorLoc(), diag::warn_equivalent_rbool_compare) + << 1 << ConstantIsBool + << FixItHint::CreateRemoval(LHSAndOp) + << FixItHint::CreateInsertion(RHSStart, "!(") + << FixItHint::CreateInsertion(S.PP.getLocForEndOfToken(RHSEnd), + ")"); + } else if (Op == BO_LE) { // 1 <= bool) + S.Diag(BO->getOperatorLoc(), diag::warn_redundant_rbool_compare) + << 1 << ConstantIsBool + << FixItHint::CreateRemoval(LHSAndOp); + } + } + if (Op == BO_EQ) { // (bool == 1) or (1 == bool) + S.Diag(BO->getOperatorLoc(), + diag::warn_redundant_bool_equality_compare) + << 1 << ConstantIsBool + << FixItHint::CreateRemoval(LeftBoolean ? RHSAndOp : LHSAndOp); + } else if (Op == BO_NE) { // (bool != 1) or (1 != bool) + S.Diag(BO->getOperatorLoc(), + diag::warn_equivalent_bool_equality_compare) + << 1 << ConstantIsBool + << FixItHint::CreateRemoval(RHSAndOp) + << FixItHint::CreateInsertion(LHSStart, "!(") + << FixItHint::CreateInsertion(S.PP.getLocForEndOfToken(LHSEnd), + ")"); + } + } else { + // Since 0 and 1 are handled above, any non-negative number will be + // greater than 1. + bool Greater = Val.isNonNegative(); + bool AlwaysTrue = false; + if (Op == BO_EQ) { + AlwaysTrue = false; + } else if (Op == BO_NE) { + AlwaysTrue = true; + } else if (Op == BO_LE || Op == BO_LT) { + AlwaysTrue = LeftBoolean == Greater; + } else if (Op == BO_GE || Op == BO_GT) { + AlwaysTrue = LeftBoolean != Greater; + } + S.Diag(BO->getOperatorLoc(), diag::warn_bool_compare_out_of_range) + << Val.toString(10) << AlwaysTrue; + } + + return true; +} + +// Returns true if a diagnostic is emitted. +static bool AnalyzeBoolComparison(Sema &S, Expr* LHS, Expr* RHS, + BinaryOperator *BO) { + bool LeftBoolean = LHS->getType()->isBooleanType(); + bool RightBoolean = RHS->getType()->isBooleanType(); + if (!LeftBoolean && !RightBoolean) + return false; + + if (LeftBoolean && RightBoolean) { + return (AnalyzeBoolConstantCompare(S, BO, true, LHS, RHS) || + AnalyzeBoolConstantCompare(S, BO, false, RHS, LHS)); + } + + Expr *BoolExpr = LeftBoolean ? LHS : RHS; + Expr *OtherExpr = LeftBoolean ? RHS : LHS; + + // If there is a logical not on LHS, diagnose it and skip the other checks. + if (LeftBoolean) + if (AnalyzeLHSLogicalNot(S, BO)) + return true; + + if (!S.ActiveTemplateInstantiations.empty()) + return false; + + // Warn on bool/float compare + if (OtherExpr->getType()->isFloatingType()) { + S.Diag(BO->getOperatorLoc(), diag::warn_bool_float_comparison) + << BoolExpr->getSourceRange() << OtherExpr->getSourceRange(); + return true; + } + + // Warn on bool/enum compare + if (OtherExpr->getType()->isEnumeralType()) { + S.Diag(BO->getOperatorLoc(), diag::warn_bool_enum_comparison) + << BoolExpr->getSourceRange() << OtherExpr->getSourceRange(); + return true; + } + + if (AnalyzeBoolConstantCompare(S, BO, LeftBoolean, BoolExpr, OtherExpr)) + return true; + + return false; +} + /// \brief Implements -Wsign-compare. /// /// \param E the binary operator to check for warnings @@ -4382,6 +4588,10 @@ Expr *LHS = E->getLHS()->IgnoreParenImpCasts(); Expr *RHS = E->getRHS()->IgnoreParenImpCasts(); + + if (LHS->getType()->isBooleanType() || RHS->getType()->isBooleanType()) + if (AnalyzeBoolComparison(S, LHS, RHS, E)) + return; bool IsComparisonConstant = false;