Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td =================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td @@ -494,11 +494,13 @@ def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">; def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">; def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">; +def TautologicalObjCBoolCompare : DiagGroup<"tautological-objc-bool-compare">; def TautologicalCompare : DiagGroup<"tautological-compare", [TautologicalConstantCompare, TautologicalPointerCompare, TautologicalOverlapCompare, - TautologicalUndefinedCompare]>; + TautologicalUndefinedCompare, + TautologicalObjCBoolCompare]>; def HeaderHygiene : DiagGroup<"header-hygiene">; def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">; def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">; Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -6026,6 +6026,10 @@ "result of comparison %select{%3|%1}0 %2 " "%select{%1|%3}0 is always %4">, InGroup, DefaultIgnore; +def warn_tautological_compare_objc_bool : Warning< + "result of comparison of constant %0 with expression of type BOOL" + " is always %1, as the only well defined values for BOOL are YES and NO">, + InGroup; def warn_mixed_sign_comparison : Warning< "comparison of integers of different signs: %0 and %1">, Index: cfe/trunk/lib/Sema/SemaChecking.cpp =================================================================== --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -10188,9 +10188,16 @@ if (isa(DR->getDecl())) return true; - // Suppress cases where the '0' value is expanded from a macro. - if (E->getBeginLoc().isMacroID()) - return true; + // Suppress cases where the value is expanded from a macro, unless that macro + // is how a language represents a boolean literal. This is the case in both C + // and Objective-C. + SourceLocation BeginLoc = E->getBeginLoc(); + if (BeginLoc.isMacroID()) { + StringRef MacroName = Lexer::getImmediateMacroName( + BeginLoc, S.getSourceManager(), S.getLangOpts()); + return MacroName != "YES" && MacroName != "NO" && + MacroName != "true" && MacroName != "false"; + } return false; } @@ -10382,11 +10389,17 @@ OtherT = AT->getValueType(); IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT); + // Special case for ObjC BOOL on targets where its a typedef for a signed char + // (Namely, macOS). + bool IsObjCSignedCharBool = S.getLangOpts().ObjC && + S.NSAPIObj->isObjCBOOLType(OtherT) && + OtherT->isSpecificBuiltinType(BuiltinType::SChar); + // Whether we're treating Other as being a bool because of the form of // expression despite it having another type (typically 'int' in C). bool OtherIsBooleanDespiteType = !OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue(); - if (OtherIsBooleanDespiteType) + if (OtherIsBooleanDespiteType || IsObjCSignedCharBool) OtherRange = IntRange::forBoolType(); // Determine the promoted range of the other type and see if a comparison of @@ -10417,10 +10430,21 @@ // Should be enough for uint128 (39 decimal digits) SmallString<64> PrettySourceValue; llvm::raw_svector_ostream OS(PrettySourceValue); - if (ED) + if (ED) { OS << '\'' << *ED << "' (" << Value << ")"; - else + } else if (auto *BL = dyn_cast( + Constant->IgnoreParenImpCasts())) { + OS << (BL->getValue() ? "YES" : "NO"); + } else { OS << Value; + } + + if (IsObjCSignedCharBool) { + S.DiagRuntimeBehavior(E->getOperatorLoc(), E, + S.PDiag(diag::warn_tautological_compare_objc_bool) + << OS.str() << *Result); + return true; + } // FIXME: We use a somewhat different formatting for the in-range cases and // cases involving boolean values for historical reasons. We should pick a Index: cfe/trunk/test/Sema/tautological-objc-bool-compare.m =================================================================== --- cfe/trunk/test/Sema/tautological-objc-bool-compare.m +++ cfe/trunk/test/Sema/tautological-objc-bool-compare.m @@ -0,0 +1,24 @@ +// RUN: %clang_cc1 %s -verify + +typedef signed char BOOL; +#define YES __objc_yes +#define NO __objc_no + +BOOL B; + +void test() { + int r; + r = B > 0; + r = B > 1; // expected-warning {{result of comparison of constant 1 with expression of type BOOL is always false, as the only well defined values for BOOL are YES and NO}} + r = B < 1; + r = B < 0; // expected-warning {{result of comparison of constant 0 with expression of type BOOL is always false, as the only well defined values for BOOL are YES and NO}} + r = B >= 0; // expected-warning {{result of comparison of constant 0 with expression of type BOOL is always true, as the only well defined values for BOOL are YES and NO}} + r = B <= 0; + + r = B > YES; // expected-warning {{result of comparison of constant YES with expression of type BOOL is always false, as the only well defined values for BOOL are YES and NO}} + r = B > NO; + r = B < NO; // expected-warning {{result of comparison of constant NO with expression of type BOOL is always false, as the only well defined values for BOOL are YES and NO}} + r = B < YES; + r = B >= NO; // expected-warning {{result of comparison of constant NO with expression of type BOOL is always true, as the only well defined values for BOOL are YES and NO}} + r = B <= NO; +}