r365408 - [ObjC] Add a -Wtautological-compare warning for BOOL
Erik Pilkington via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 8 16:42:53 PDT 2019
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;
+}
More information about the cfe-commits
mailing list