erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, steven_wu, rsmith.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.
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://problem/51954400 ObjC: Add diagnostics to catch incorrect usage of BOOL
Repository:
rC Clang
https://reviews.llvm.org/D63856
Files:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/tautological-objc-bool-compare.m
Index: clang/test/Sema/tautological-objc-bool-compare.m
===================================================================
--- /dev/null
+++ clang/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;
+}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10193,9 +10193,16 @@
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;
}
@@ -10387,11 +10394,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
@@ -10422,10 +10435,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<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
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/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<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">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/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">;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
