[PATCH] D36790: [ObjC] Messages to 'self' in class methods should use class method dispatch to avoid multiple method ambiguities

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.

Prior to this patch, 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 patch 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 `instancetype`s are correctly interpreted by the compiler. This is an ARC only change, since non-ARC code can reassign `self`. The type `dectltype(self)` is used to store the `self` receiver in the class message to ensure that `self` remains in the AST.

Thanks for taking a look.

rdar://20940997


Repository:
  rL LLVM

https://reviews.llvm.org/D36790

Files:
  lib/Sema/SemaExprObjC.cpp
  test/SemaObjC/multiple-method-names-class-self.m


Index: test/SemaObjC/multiple-method-names-class-self.m
===================================================================
--- /dev/null
+++ test/SemaObjC/multiple-method-names-class-self.m
@@ -0,0 +1,44 @@
+// 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 -DARC %s
+#ifdef ARC
+// expected-no-diagnostics
+#endif
+
+ at interface NSObj
+
++ (instancetype) alloc;
+
+ at end
+
+ at interface SelfAllocReturn: NSObj
+
+- (instancetype)initWithFoo:(int)x;
+#ifndef ARC
+// expected-note at -2 {{using}}
+#endif
+
+ at end
+
+ at interface SelfAllocReturn2: NSObj
+
+- (instancetype)initWithFoo:(SelfAllocReturn *)x;
+#ifndef ARC
+// expected-note at -2 {{also found}}
+#endif
+
+ at end
+
+ at implementation SelfAllocReturn
+
+- (instancetype)initWithFoo:(int)x {
+    return self;
+}
+
++ (instancetype) thingWithFoo:(int)x {
+    return [[self alloc] initWithFoo: x];
+#ifndef ARC
+// expected-warning at -2 {{multiple methods named 'initWithFoo:' found}}
+#endif
+}
+
+ at end
Index: lib/Sema/SemaExprObjC.cpp
===================================================================
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1320,9 +1320,11 @@
       }
   }
 
-  //   - if the receiver is the name of a class U, T is a pointer to U
+  //   - if the receiver is the name of a class U, T is a pointer to U. U should
+  //     be desugared to avoid 'decltype(self)' propagation.
   if (ReceiverType->getAsObjCInterfaceType())
-    return transferNullability(Context.getObjCObjectPointerType(ReceiverType));
+    return transferNullability(Context.getObjCObjectPointerType(
+        ReceiverType.getDesugaredType(Context)));
   //   - if the receiver is of type Class or qualified Class type,
   //     T is the declared return type of the method.
   if (ReceiverType->isObjCClassType() ||
@@ -2738,10 +2740,31 @@
     } else if (ReceiverType->isObjCClassOrClassKindOfType() ||
                ReceiverType->isObjCQualifiedClassType()) {
       // Handle messages to Class.
+      // Treat messages to 'self' in class methods as class messages when ARC
+      // is enabled (because self can't be reassigned when ARC is on).
+      if (Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount) {
+        assert(ReceiverType->isObjCClassType() && "expected a Class self");
+        const ObjCMethodDecl *MD = cast<ObjCMethodDecl>(
+            cast<ImplicitParamDecl>(
+                cast<DeclRefExpr>(Receiver->IgnoreParenImpCasts())->getDecl())
+                ->getDeclContext());
+        assert(MD->isClassMethod() && "expected a class method");
+        QualType ReceiverType =
+            Context.getObjCInterfaceType(MD->getClassInterface());
+        // Use a pseudo-decltype type to keep the expression in the AST.
+        assert(!Receiver->isInstantiationDependent() &&
+               "unexpected dependent expr");
+        ReceiverType = Context.getDecltypeType(Receiver, ReceiverType);
+        return BuildClassMessage(
+            Context.getTrivialTypeSourceInfo(ReceiverType, LBracLoc),
+            ReceiverType,
+            /*SuperLoc=*/SourceLocation(), Sel,
+            /*Method=*/nullptr, LBracLoc, SelectorLocs, RBracLoc, ArgsIn);
+      }
       // We allow sending a message to a qualified Class ("Class<foo>"), which
       // is ok as long as one of the protocols implements the selector (if not,
       // warn).
-      if (!ReceiverType->isObjCClassOrClassKindOfType()) {
+      else if (!ReceiverType->isObjCClassOrClassKindOfType()) {
         const ObjCObjectPointerType *QClassTy
           = ReceiverType->getAsObjCQualifiedClassType();
         // Search protocols for class methods.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36790.111334.patch
Type: text/x-patch
Size: 3723 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170816/839954a7/attachment.bin>