From: Erik Pilkington Date: Mon, 8 Jul 2019 23:42:52 +0000 (+0000) Subject: [ObjC] Add a -Wtautological-compare warning for BOOL X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d2c117d5302aab5c70f07b72a5ce36b841f44622;p=clang [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@365408 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index 9bfea83db4..27671df0cf 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -494,11 +494,13 @@ def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare", 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">; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 5fd191a030..0536552015 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -6026,6 +6026,10 @@ def warn_tautological_constant_compare : Warning< "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">, diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 92951a8e3b..702a230596 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -10188,9 +10188,16 @@ static bool IsEnumConstOrFromMacro(Sema &S, Expr *E) { 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 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E, 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(Sema &S, BinaryOperator *E, // 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 diff --git a/test/Sema/tautological-objc-bool-compare.m b/test/Sema/tautological-objc-bool-compare.m new file mode 100644 index 0000000000..525862ed9e --- /dev/null +++ b/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; +}