diff options
-rw-r--r-- | include/clang/AST/NSAPI.h | 8 | ||||
-rw-r--r-- | include/clang/Basic/DiagnosticSemaKinds.td | 4 | ||||
-rw-r--r-- | lib/Sema/SemaExpr.cpp | 157 | ||||
-rw-r--r-- | test/FixIt/objc-literals.m | 38 | ||||
-rw-r--r-- | test/SemaObjC/objc-literal-comparison.m | 68 |
5 files changed, 266 insertions, 9 deletions
diff --git a/include/clang/AST/NSAPI.h b/include/clang/AST/NSAPI.h index ec5e542f72..51ae1daf14 100644 --- a/include/clang/AST/NSAPI.h +++ b/include/clang/AST/NSAPI.h @@ -132,6 +132,11 @@ public: return getOrInitSelector(Ids, setObjectAtIndexedSubscriptSel); } + /// \brief Returns selector for "isEqual:". + Selector getIsEqualSelector() const { + return getOrInitSelector(StringRef("isEqual"), isEqualSel); + } + /// \brief Enumerates the NSNumber methods used to generate literals. enum NSNumberLiteralMethodKind { NSNumberWithChar, @@ -203,7 +208,8 @@ private: mutable Selector NSNumberInstanceSelectors[NumNSNumberLiteralMethods]; mutable Selector objectForKeyedSubscriptSel, objectAtIndexedSubscriptSel, - setObjectForKeyedSubscriptSel,setObjectAtIndexedSubscriptSel; + setObjectForKeyedSubscriptSel,setObjectAtIndexedSubscriptSel, + isEqualSel; mutable IdentifierInfo *BOOLId, *NSIntegerId, *NSUIntegerId; mutable IdentifierInfo *NSASCIIStringEncodingId, *NSUTF8StringEncodingId; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 3dc03fa802..e50138dccf 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1593,6 +1593,10 @@ def err_invalid_collection_element : Error< def err_box_literal_collection : Error< "%select{string|character|boolean|numeric}0 literal must be prefixed by '@' " "in a collection">; +def err_objc_literal_comparison : Error< + "direct comparison of %select{a string literal|an array literal|" + "a dictionary literal|a numeric literal|a boxed expression|}0 is not " + "allowed%select{|; use -isEqual: instead}1">; let CategoryName = "Cocoa API Issue" in { def warn_objc_redundant_literal_use : Warning< diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index d2377f5905..8423a2d97f 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -6589,6 +6589,153 @@ static void diagnoseFunctionPointerToVoidComparison(Sema &S, SourceLocation Loc, << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); } +static bool isObjCObjectLiteral(ExprResult &E) { + switch (E.get()->getStmtClass()) { + case Stmt::ObjCArrayLiteralClass: + case Stmt::ObjCDictionaryLiteralClass: + case Stmt::ObjCStringLiteralClass: + case Stmt::ObjCBoxedExprClass: + return true; + default: + // Note that ObjCBoolLiteral is NOT an object literal! + return false; + } +} + +static DiagnosticBuilder diagnoseObjCLiteralComparison(Sema &S, + SourceLocation Loc, + ExprResult &LHS, + ExprResult &RHS, + bool CanFix = false) { + Expr *Literal = (isObjCObjectLiteral(LHS) ? LHS : RHS).get(); + + unsigned LiteralKind; + switch (Literal->getStmtClass()) { + case Stmt::ObjCStringLiteralClass: + // "string literal" + LiteralKind = 0; + break; + case Stmt::ObjCArrayLiteralClass: + // "array literal" + LiteralKind = 1; + break; + case Stmt::ObjCDictionaryLiteralClass: + // "dictionary literal" + LiteralKind = 2; + break; + case Stmt::ObjCBoxedExprClass: { + Expr *Inner = cast<ObjCBoxedExpr>(Literal)->getSubExpr(); + switch (Inner->getStmtClass()) { + case Stmt::IntegerLiteralClass: + case Stmt::FloatingLiteralClass: + case Stmt::CharacterLiteralClass: + case Stmt::ObjCBoolLiteralExprClass: + case Stmt::CXXBoolLiteralExprClass: + // "numeric literal" + LiteralKind = 3; + break; + case Stmt::ImplicitCastExprClass: { + CastKind CK = cast<CastExpr>(Inner)->getCastKind(); + // Boolean literals can be represented by implicit casts. + if (CK == CK_IntegralToBoolean || CK == CK_IntegralCast) { + LiteralKind = 3; + break; + } + // FALLTHROUGH + } + default: + // "boxed expression" + LiteralKind = 4; + break; + } + break; + } + default: + llvm_unreachable("Unknown Objective-C object literal kind"); + } + + return S.Diag(Loc, diag::err_objc_literal_comparison) + << LiteralKind << CanFix << Literal->getSourceRange(); +} + +static ExprResult fixObjCLiteralComparison(Sema &S, SourceLocation OpLoc, + ExprResult &LHS, + ExprResult &RHS, + BinaryOperatorKind Op) { + assert((Op == BO_EQ || Op == BO_NE) && "Cannot fix other operations."); + + // Get the LHS object's interface type. + QualType Type = LHS.get()->getType(); + QualType InterfaceType; + if (const ObjCObjectPointerType *PTy = Type->getAs<ObjCObjectPointerType>()) { + InterfaceType = PTy->getPointeeType(); + if (const ObjCObjectType *iQFaceTy = + InterfaceType->getAsObjCQualifiedInterfaceType()) + InterfaceType = iQFaceTy->getBaseType(); + } else { + // If this is not actually an Objective-C object, bail out. + return ExprEmpty(); + } + + // If the RHS isn't an Objective-C object, bail out. + if (!RHS.get()->getType()->isObjCObjectPointerType()) + return ExprEmpty(); + + // Try to find the -isEqual: method. + Selector IsEqualSel = S.NSAPIObj->getIsEqualSelector(); + ObjCMethodDecl *Method = S.LookupMethodInObjectType(IsEqualSel, + InterfaceType, + /*instance=*/true); + bool ReceiverIsId = (Type->isObjCIdType() || Type->isObjCQualifiedIdType()); + + if (!Method && ReceiverIsId) { + Method = S.LookupInstanceMethodInGlobalPool(IsEqualSel, SourceRange(), + /*receiverId=*/true, + /*warn=*/false); + } + + if (!Method) + return ExprEmpty(); + + QualType T = Method->param_begin()[0]->getType(); + if (!T->isObjCObjectPointerType()) + return ExprEmpty(); + + QualType R = Method->getResultType(); + if (!R->isScalarType()) + return ExprEmpty(); + + // At this point we know we have a good -isEqual: method. + // Emit the diagnostic and fixit. + DiagnosticBuilder Diag = diagnoseObjCLiteralComparison(S, OpLoc, + LHS, RHS, true); + + Expr *LHSExpr = LHS.take(); + Expr *RHSExpr = RHS.take(); + + SourceLocation Start = LHSExpr->getLocStart(); + SourceLocation End = S.PP.getLocForEndOfToken(RHSExpr->getLocEnd()); + SourceRange OpRange(OpLoc, S.PP.getLocForEndOfToken(OpLoc)); + + Diag << FixItHint::CreateInsertion(Start, Op == BO_EQ ? "[" : "![") + << FixItHint::CreateReplacement(OpRange, "isEqual:") + << FixItHint::CreateInsertion(End, "]"); + + // Finally, build the call to -isEqual: (and possible logical not). + ExprResult Call = S.BuildInstanceMessage(LHSExpr, LHSExpr->getType(), + /*SuperLoc=*/SourceLocation(), + IsEqualSel, Method, + OpLoc, OpLoc, OpLoc, + MultiExprArg(S, &RHSExpr, 1), + /*isImplicit=*/false); + + ExprResult CallCond = S.CheckBooleanCondition(Call.get(), OpLoc); + + if (Op == BO_NE) + return S.CreateBuiltinUnaryOp(OpLoc, UO_LNot, CallCond.get()); + return CallCond; +} + // C99 6.5.8, C++ [expr.rel] QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, unsigned OpaqueOpc, @@ -6913,6 +7060,9 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS, if (!Context.areComparableObjCPointerTypes(LHSType, RHSType)) diagnoseDistinctPointerComparison(*this, Loc, LHS, RHS, /*isError*/false); + if (isObjCObjectLiteral(LHS) || isObjCObjectLiteral(RHS)) + diagnoseObjCLiteralComparison(*this, Loc, LHS, RHS); + if (LHSIsNull && !RHSIsNull) LHS = ImpCastExprToType(LHS.take(), RHSType, CK_BitCast); else @@ -7971,6 +8121,13 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, break; case BO_EQ: case BO_NE: + if (isObjCObjectLiteral(LHS) || isObjCObjectLiteral(RHS)) { + ExprResult IsEqualCall = fixObjCLiteralComparison(*this, OpLoc, + LHS, RHS, Opc); + if (IsEqualCall.isUsable()) + return IsEqualCall; + // Otherwise, fall back to the normal diagnostic in CheckCompareOperands. + } ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc, false); break; case BO_And: diff --git a/test/FixIt/objc-literals.m b/test/FixIt/objc-literals.m index 03d64b1564..356e26348e 100644 --- a/test/FixIt/objc-literals.m +++ b/test/FixIt/objc-literals.m @@ -2,13 +2,15 @@ // RUN: cp %s %t // RUN: not %clang_cc1 -fsyntax-only -fixit -x objective-c %t // RUN: %clang_cc1 -fsyntax-only -pedantic -Werror -x objective-c %t -// RUN: grep arrayWithObjects %t +// RUN: FileCheck -input-file=%t %s typedef unsigned char BOOL; -@interface NSNumber @end +@interface NSObject +- (BOOL)isEqual:(id)other; +@end -@interface NSNumber (NSNumberCreation) +@interface NSNumber : NSObject + (NSNumber *)numberWithChar:(char)value; + (NSNumber *)numberWithUnsignedChar:(unsigned char)value; + (NSNumber *)numberWithShort:(short)value; @@ -24,17 +26,17 @@ typedef unsigned char BOOL; + (NSNumber *)numberWithBool:(BOOL)value; @end -@interface NSArray -@end - -@interface NSArray (NSArrayCreation) +@interface NSArray : NSObject + (id)arrayWithObjects:(const id [])objects count:(unsigned long)cnt; @end -@interface NSDictionary +@interface NSDictionary : NSObject + (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(unsigned long)cnt; @end +@interface NSString : NSObject +@end + void fixes() { id arr = @[ 17, // expected-error{{numeric literal must be prefixed by '@' in a collection}} @@ -42,3 +44,23 @@ void fixes() { "blah" // expected-error{{string literal must be prefixed by '@'}} ]; } + +void testComparisons(id obj) { + if (obj == @"abc") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}} + if (obj != @"def") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}} + if (@"ghi" == obj) return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}} + + // CHECK: void testComparisons(id obj) { + // Make sure these three substitutions aren't matching the CHECK lines. + // CHECK-NEXT: if ([obj isEqual: @"abc"]) return; + // CHECK-NEXT: if (![obj isEqual: @"def"]) return; + // CHECK-NEXT: if ([@"ghi" isEqual: obj]) return; + + if (@[] == obj) return; // expected-error{{direct comparison of an array literal is not allowed; use -isEqual: instead}} + if (@{} == obj) return; // expected-error{{direct comparison of a dictionary literal is not allowed; use -isEqual: instead}} + if (@12 == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}} + if (@1.0 == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}} + if (@__objc_yes == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}} + if (@(1+1) == obj) return; // expected-error{{direct comparison of a boxed expression is not allowed; use -isEqual: instead}} +} + diff --git a/test/SemaObjC/objc-literal-comparison.m b/test/SemaObjC/objc-literal-comparison.m new file mode 100644 index 0000000000..23cb42bc62 --- /dev/null +++ b/test/SemaObjC/objc-literal-comparison.m @@ -0,0 +1,68 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +typedef unsigned char BOOL; + +@interface BaseObject ++ (instancetype)new; +@end + +@interface NSObject : BaseObject +- (BOOL)isEqual:(id)other; +@end + +@interface NSNumber : NSObject ++ (NSNumber *)numberWithInt:(int)value; ++ (NSNumber *)numberWithDouble:(double)value; ++ (NSNumber *)numberWithBool:(BOOL)value; +@end + +@interface NSArray : NSObject ++ (id)arrayWithObjects:(const id [])objects count:(unsigned long)cnt; +@end + +@interface NSDictionary : NSObject ++ (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(unsigned long)cnt; +@end + +@interface NSString : NSObject +@end + +void testComparisonsWithFixits(id obj) { + if (obj == @"") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}} + if (obj != @"") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}} + if (@"" == obj) return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}} + if (@"" == @"") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}} + + if (@[] == obj) return; // expected-error{{direct comparison of an array literal is not allowed; use -isEqual: instead}} + if (@{} == obj) return; // expected-error{{direct comparison of a dictionary literal is not allowed; use -isEqual: instead}} + if (@12 == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}} + if (@1.0 == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}} + if (@__objc_yes == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}} + if (@(1+1) == obj) return; // expected-error{{direct comparison of a boxed expression is not allowed; use -isEqual: instead}} +} + + +@interface BadEqualReturnString : NSString +- (void)isEqual:(id)other; +@end + +@interface BadEqualArgString : NSString +- (BOOL)isEqual:(int)other; +@end + + +void testComparisonsWithoutFixits() { + // All of these verifications use regex form to ensure we /don't/ append + // "use -isEqual: instead" to any of these. + + if ([BaseObject new] == @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}} + + if ([BadEqualReturnString new] == @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}} + if ([BadEqualArgString new] == @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}} + + if (@"" < @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}} + if (@"" > @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}} + if (@"" <= @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}} + if (@"" >= @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}} +} + |