From f50d1aca992864d7edd65a449f5368b53727752a Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Thu, 20 Dec 2018 22:11:11 +0000 Subject: [PATCH] [ObjC] Messages to 'self' in class methods that return 'instancetype' should use the pointer to the class as the result type of the message Prior to this commit, messages to self in class methods were treated as instance methods to a Class value. When these methods returned instancetype the compiler only saw id through the instancetype, and not the Interface *. This caused problems when that return value was a receiver in a message send, as the compiler couldn't select the right method declaration and had to rely on a selection from the global method pool. This commit modifies the semantics of such message sends and uses class messages that are dispatched to the interface that corresponds to the class that contains the class method. This ensures that instancetypes are correctly interpreted by the compiler. This change is safe under ARC (as self can't be reassigned), however, it also applies to MRR code as we are assuming that the user isn't doing anything unreasonable. rdar://20940997 Differential Revision: https://reviews.llvm.org/D36790 llvm-svn: 349841 --- clang/include/clang/Sema/Sema.h | 13 ++-- clang/lib/Sema/SemaExprObjC.cpp | 65 ++++++++++++------- .../multiple-method-names-in-class-self.m | 39 +++++++++++ 3 files changed, 88 insertions(+), 29 deletions(-) create mode 100644 clang/test/SemaObjC/multiple-method-names-in-class-self.m diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7685b3f74ad0..fd83b374ad2d 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -9842,21 +9842,20 @@ public: /// \param Method - May be null. /// \param [out] ReturnType - The return type of the send. /// \return true iff there were any incompatible types. - bool CheckMessageArgumentTypes(QualType ReceiverType, + bool CheckMessageArgumentTypes(const Expr *Receiver, QualType ReceiverType, MultiExprArg Args, Selector Sel, ArrayRef SelectorLocs, ObjCMethodDecl *Method, bool isClassMessage, - bool isSuperMessage, - SourceLocation lbrac, SourceLocation rbrac, - SourceRange RecRange, + bool isSuperMessage, SourceLocation lbrac, + SourceLocation rbrac, SourceRange RecRange, QualType &ReturnType, ExprValueKind &VK); /// Determine the result of a message send expression based on /// the type of the receiver, the method expected to receive the message, /// and the form of the message send. - QualType getMessageSendResultType(QualType ReceiverType, - ObjCMethodDecl *Method, - bool isClassMessage, bool isSuperMessage); + QualType getMessageSendResultType(const Expr *Receiver, QualType ReceiverType, + ObjCMethodDecl *Method, bool isClassMessage, + bool isSuperMessage); /// If the given expression involves a message send to a method /// with a related result type, emit a note describing what happened. diff --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp index 9eaf747ae79e..ce80db7ca6ec 100644 --- a/clang/lib/Sema/SemaExprObjC.cpp +++ b/clang/lib/Sema/SemaExprObjC.cpp @@ -1346,7 +1346,8 @@ static QualType getBaseMessageSendResultType(Sema &S, return transferNullability(ReceiverType); } -QualType Sema::getMessageSendResultType(QualType ReceiverType, +QualType Sema::getMessageSendResultType(const Expr *Receiver, + QualType ReceiverType, ObjCMethodDecl *Method, bool isClassMessage, bool isSuperMessage) { @@ -1357,8 +1358,33 @@ QualType Sema::getMessageSendResultType(QualType ReceiverType, isSuperMessage); // If this is a class message, ignore the nullability of the receiver. - if (isClassMessage) + if (isClassMessage) { + // In a class method, class messages to 'self' that return instancetype can + // be typed as the current class. We can safely do this in ARC because self + // can't be reassigned, and we do it unsafely outside of ARC because in + // practice people never reassign self in class methods and there's some + // virtue in not being aggressively pedantic. + if (Receiver && Receiver->isObjCSelfExpr()) { + assert(ReceiverType->isObjCClassType() && "expected a Class self"); + QualType T = Method->getSendResultType(ReceiverType); + AttributedType::stripOuterNullability(T); + if (T == Context.getObjCInstanceType()) { + const ObjCMethodDecl *MD = cast( + cast( + cast(Receiver->IgnoreParenImpCasts())->getDecl()) + ->getDeclContext()); + assert(MD->isClassMethod() && "expected a class method"); + QualType NewResultType = Context.getObjCObjectPointerType( + Context.getObjCInterfaceType(MD->getClassInterface())); + if (auto Nullability = resultType->getNullability(Context)) + NewResultType = Context.getAttributedType( + AttributedType::getNullabilityAttrKind(*Nullability), + NewResultType, NewResultType); + return NewResultType; + } + } return resultType; + } // There is nothing left to do if the result type cannot have a nullability // specifier. @@ -1505,15 +1531,12 @@ void Sema::EmitRelatedResultTypeNote(const Expr *E) { << MsgSend->getType(); } -bool Sema::CheckMessageArgumentTypes(QualType ReceiverType, - MultiExprArg Args, - Selector Sel, - ArrayRef SelectorLocs, - ObjCMethodDecl *Method, - bool isClassMessage, bool isSuperMessage, - SourceLocation lbrac, SourceLocation rbrac, - SourceRange RecRange, - QualType &ReturnType, ExprValueKind &VK) { +bool Sema::CheckMessageArgumentTypes( + const Expr *Receiver, QualType ReceiverType, MultiExprArg Args, + Selector Sel, ArrayRef SelectorLocs, ObjCMethodDecl *Method, + bool isClassMessage, bool isSuperMessage, SourceLocation lbrac, + SourceLocation rbrac, SourceRange RecRange, QualType &ReturnType, + ExprValueKind &VK) { SourceLocation SelLoc; if (!SelectorLocs.empty() && SelectorLocs.front().isValid()) SelLoc = SelectorLocs.front(); @@ -1590,8 +1613,8 @@ bool Sema::CheckMessageArgumentTypes(QualType ReceiverType, return false; } - ReturnType = getMessageSendResultType(ReceiverType, Method, isClassMessage, - isSuperMessage); + ReturnType = getMessageSendResultType(Receiver, ReceiverType, Method, + isClassMessage, isSuperMessage); VK = Expr::getValueKindForType(Method->getReturnType()); unsigned NumNamedArgs = Sel.getNumArgs(); @@ -2482,12 +2505,10 @@ ExprResult Sema::BuildClassMessage(TypeSourceInfo *ReceiverTypeInfo, unsigned NumArgs = ArgsIn.size(); Expr **Args = ArgsIn.data(); - if (CheckMessageArgumentTypes(ReceiverType, MultiExprArg(Args, NumArgs), - Sel, SelectorLocs, - Method, true, - SuperLoc.isValid(), LBracLoc, RBracLoc, - SourceRange(), - ReturnType, VK)) + if (CheckMessageArgumentTypes(/*Receiver=*/nullptr, ReceiverType, + MultiExprArg(Args, NumArgs), Sel, SelectorLocs, + Method, true, SuperLoc.isValid(), LBracLoc, + RBracLoc, SourceRange(), ReturnType, VK)) return ExprError(); if (Method && !Method->getReturnType()->isVoidType() && @@ -2982,9 +3003,9 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver, ExprValueKind VK = VK_RValue; bool ClassMessage = (ReceiverType->isObjCClassType() || ReceiverType->isObjCQualifiedClassType()); - if (CheckMessageArgumentTypes(ReceiverType, MultiExprArg(Args, NumArgs), - Sel, SelectorLocs, Method, - ClassMessage, SuperLoc.isValid(), + if (CheckMessageArgumentTypes(Receiver, ReceiverType, + MultiExprArg(Args, NumArgs), Sel, SelectorLocs, + Method, ClassMessage, SuperLoc.isValid(), LBracLoc, RBracLoc, RecRange, ReturnType, VK)) return ExprError(); diff --git a/clang/test/SemaObjC/multiple-method-names-in-class-self.m b/clang/test/SemaObjC/multiple-method-names-in-class-self.m new file mode 100644 index 000000000000..47e1306d3837 --- /dev/null +++ b/clang/test/SemaObjC/multiple-method-names-in-class-self.m @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -Wobjc-multiple-method-names -x objective-c -verify %s +// RUN: %clang_cc1 -Wobjc-multiple-method-names -x objective-c -verify -fobjc-arc %s +// expected-no-diagnostics + +@interface NSObj + ++ (instancetype) alloc; + ++ (_Nonnull instancetype) globalObject; + +@end + +@interface SelfAllocReturn: NSObj + +- (instancetype)initWithFoo:(int)x; + +@end + +@interface SelfAllocReturn2: NSObj + +- (instancetype)initWithFoo:(SelfAllocReturn *)x; + +@end + +@implementation SelfAllocReturn + +- (instancetype)initWithFoo:(int)x { + return self; +} + ++ (instancetype) thingWithFoo:(int)x { + return [[self alloc] initWithFoo: x]; +} + ++ (void) initGlobal { + (void)[[self globalObject] initWithFoo: 20]; +} + +@end