Author: epilk Date: Mon Jul 8 16:42:52 2019 New Revision: 365408 URL: http://llvm.org/viewvc/llvm-project?rev=365408&view=rev Log: [ObjC] Add a -Wtautological-compare warning for BOOL
On macOS, BOOL is a typedef for signed char, but it should never hold a value that isn't 1 or 0. Any code that expects a different value in their BOOL should be fixed. rdar://51954400 Differential revision: https://reviews.llvm.org/D63856 Added: cfe/trunk/test/Sema/tautological-objc-bool-compare.m Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaChecking.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=365408&r1=365407&r2=365408&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jul 8 16:42:52 2019 @@ -494,11 +494,13 @@ def TautologicalConstantCompare : DiagGr 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">; Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=365408&r1=365407&r2=365408&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 8 16:42:52 2019 @@ -6026,6 +6026,10 @@ def warn_tautological_constant_compare : "result of comparison %select{%3|%1}0 %2 " "%select{%1|%3}0 is always %4">, InGroup<TautologicalTypeLimitCompare>, 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<TautologicalObjCBoolCompare>; def warn_mixed_sign_comparison : Warning< "comparison of integers of different signs: %0 and %1">, Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=365408&r1=365407&r2=365408&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jul 8 16:42:52 2019 @@ -10188,9 +10188,16 @@ static bool IsEnumConstOrFromMacro(Sema if (isa<EnumConstantDecl>(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 @@ static bool CheckTautologicalComparison( 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 @@ static bool CheckTautologicalComparison( // 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<ObjCBoolLiteralExpr>( + 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 Added: cfe/trunk/test/Sema/tautological-objc-bool-compare.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-objc-bool-compare.m?rev=365408&view=auto ============================================================================== --- cfe/trunk/test/Sema/tautological-objc-bool-compare.m (added) +++ cfe/trunk/test/Sema/tautological-objc-bool-compare.m Mon Jul 8 16:42:52 2019 @@ -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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
