r266264 - ObjC kindof: order the methods in global pool relative to availability.

[Manman Ren via cfe-commits]

Author: mren
Date: Wed Apr 13 18:43:56 2016
New Revision: 266264

URL: http://llvm.org/viewvc/llvm-project?rev=266264&view=rev
Log:
ObjC kindof: order the methods in global pool relative to availability.

r265877 tries to put methods that are deprecated or unavailable to the
front of the global pool to emit diagnostics, but it breaks some of
our existing codes that depend on choosing a certain method for id
lookup.

This commit orders the methods with the same declaration with respect
to the availability, but do not order methods with different declaration.

rdar://25707511

Modified:
    cfe/trunk/lib/Sema/SemaDeclObjC.cpp
    cfe/trunk/test/SemaObjC/kindof.m
    cfe/trunk/test/SemaObjC/multiple-method-names.m

Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=266264&r1=266263&r2=266264&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Wed Apr 13 18:43:56 2016
@@ -3213,22 +3213,43 @@ void Sema::addMethodToGlobalList(ObjCMet
 
   // We've seen a method with this name, see if we have already seen this type
   // signature.
-  ObjCMethodList *Head = List;
   ObjCMethodList *Previous = List;
+  ObjCMethodList *ListWithSameDeclaration = nullptr;
   for (; List; Previous = List, List = List->getNext()) {
     // If we are building a module, keep all of the methods.
     if (getLangOpts().CompilingModule)
       continue;
 
+    bool SameDeclaration = MatchTwoMethodDeclarations(Method,
+                                                      List->getMethod());
     // Looking for method with a type bound requires the correct context exists.
-    // We need to insert this method into the list if the context is different.
-    if (!MatchTwoMethodDeclarations(Method, List->getMethod()) ||
+    // We need to insert a method into the list if the context is different.
+    // If the method's declaration matches the list
+    // a> the method belongs to a different context: we need to insert it, in
+    //    order to emit the availability message, we need to prioritize over
+    //    availability among the methods with the same declaration.
+    // b> the method belongs to the same context: there is no need to insert a
+    //    new entry.
+    // If the method's declaration does not match the list, we insert it to the
+    // end.
+    if (!SameDeclaration ||
         !isMethodContextSameForKindofLookup(Method, List->getMethod())) {
       // Even if two method types do not match, we would like to say
       // there is more than one declaration so unavailability/deprecated
       // warning is not too noisy.
       if (!Method->isDefined())
         List->setHasMoreThanOneDecl(true);
+
+      // For methods with the same declaration, the one that is deprecated
+      // should be put in the front for better diagnostics.
+      if (Method->isDeprecated() && SameDeclaration &&
+          !ListWithSameDeclaration && !List->getMethod()->isDeprecated())
+        ListWithSameDeclaration = List;
+
+      if (Method->isUnavailable() && SameDeclaration &&
+          !ListWithSameDeclaration &&
+          List->getMethod()->getAvailability() < AR_Deprecated)
+        ListWithSameDeclaration = List;
       continue;
     }
 
@@ -3265,15 +3286,12 @@ void Sema::addMethodToGlobalList(ObjCMet
   // This is extremely rare. Only 1% of Cocoa selectors are "overloaded".
   ObjCMethodList *Mem = BumpAlloc.Allocate<ObjCMethodList>();
 
-  // We tried to prioritize the list by putting deprecated and unavailable
-  // methods in the front.
-  if ((Method->isDeprecated() && !Head->getMethod()->isDeprecated()) ||
-      (Method->isUnavailable() &&
-       Head->getMethod()->getAvailability() < AR_Deprecated)) {
-    auto *List = new (Mem) ObjCMethodList(*Head);
-    // FIXME: should we clear the other bits in Head?
-    Head->setMethod(Method);
-    Head->setNext(List);
+  // We insert it right before ListWithSameDeclaration.
+  if (ListWithSameDeclaration) {
+    auto *List = new (Mem) ObjCMethodList(*ListWithSameDeclaration);
+    // FIXME: should we clear the other bits in ListWithSameDeclaration?
+    ListWithSameDeclaration->setMethod(Method);
+    ListWithSameDeclaration->setNext(List);
     return;
   }
 

Modified: cfe/trunk/test/SemaObjC/kindof.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/kindof.m?rev=266264&r1=266263&r2=266264&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/kindof.m (original)
+++ cfe/trunk/test/SemaObjC/kindof.m Wed Apr 13 18:43:56 2016
@@ -25,6 +25,7 @@ __attribute__((objc_root_class))
 @end
 
 @interface NSString : NSObject <NSCopying> // expected-note{{receiver is instance of class declared here}}
+- (void)compare:(NSString *)string;
 - (NSString *)stringByAppendingString:(NSString *)string;
 + (instancetype)string;
 @end
@@ -289,6 +290,18 @@ void foo() {
   }
 }
 
+typedef const struct CGPath *CGPathRef;
+ at interface C : NSObject
+ at property (copy) NSString *path;
+ at end
+ at interface D : NSObject
+ at property CGPathRef path __attribute__((availability(macosx,unavailable)));
+ at end
+// Make sure we choose "NSString *path" for [s1 path].
+void bar(id s1, id s2) {
+  return [[s1 path] compare:[s2 path]];
+}
+
 // ---------------------------------------------------------------------------
 // __kindof within specialized types
 // ---------------------------------------------------------------------------

Modified: cfe/trunk/test/SemaObjC/multiple-method-names.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/multiple-method-names.m?rev=266264&r1=266263&r2=266264&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/multiple-method-names.m (original)
+++ cfe/trunk/test/SemaObjC/multiple-method-names.m Wed Apr 13 18:43:56 2016
@@ -2,11 +2,11 @@
 // PR22047
 
 @interface Face0
-- (void)foo:(float)i; // expected-note {{also found}}
+- (void)foo:(float)i; // expected-note {{using}}
 @end
 
 @interface Face1
-- (void)foo:(int)i __attribute__((unavailable)); // expected-note {{using}}
+- (void)foo:(int)i __attribute__((unavailable));
 @end
 
 @interface Face2