[PrintAsClang] Beef up extension sorting rule

A couple of the rules that `ModuleContentsWriter::write()` uses to sort declarations didn’t actually work because of an incorrect predicate. In addition, there were a number of situations that could come up in C++ interop (where overloading is permitted) where extensions could not be sorted. Rework extension sorting to look for more kinds of differences between extension members.

Author: beccadax

lib/PrintAsClang/ModuleContentsWriter.cpp

@@ -780,38 +780,43 @@ class ModuleWriter {
           std::mismatch(lhsProtos.begin(), lhsProtos.end(), rhsProtos.begin(),
                         [] (const ProtocolDecl *nextLHSProto,
                             const ProtocolDecl *nextRHSProto) {
-          return nextLHSProto->getName() != nextRHSProto->getName();
+          return nextLHSProto->getName() == nextRHSProto->getName();
         });
         if (mismatch.first != lhsProtos.end()) {
           StringRef lhsProtoName = (*mismatch.first)->getName().str();
           return lhsProtoName.compare((*mismatch.second)->getName().str());
         }
       }
 
-      // Still nothing? Fine, we'll pick the one with the alphabetically first
-      // member instead.
+      // Still nothing? Fine, we'll look for a difference between the members.
       {
-        auto mismatch = std::mismatch(
-            cast<ExtensionDecl>(*lhs)->getAllMembers().begin(),
-            cast<ExtensionDecl>(*lhs)->getAllMembers().end(),
-            cast<ExtensionDecl>(*rhs)->getAllMembers().begin(),
-            [](const Decl *nextLHSDecl, const Decl *nextRHSDecl) {
-              if (isa<ValueDecl>(nextLHSDecl) && isa<ValueDecl>(nextRHSDecl)) {
-                return cast<ValueDecl>(nextLHSDecl)->getName() !=
-                       cast<ValueDecl>(nextRHSDecl)->getName();
-              }
-              return isa<ValueDecl>(nextLHSDecl) != isa<ValueDecl>(nextRHSDecl);
-            });
-        if (mismatch.first !=
-            cast<ExtensionDecl>(*lhs)->getAllMembers().end()) {
-          auto *lhsMember = dyn_cast<ValueDecl>(*mismatch.first),
-               *rhsMember = dyn_cast<ValueDecl>(*mismatch.second);
+        for (auto pair : llvm::zip_equal(lhsMembers, rhsMembers)) {
+          auto *lhsMember = dyn_cast<ValueDecl>(std::get<0>(pair)),
+               *rhsMember = dyn_cast<ValueDecl>(std::get<1>(pair));
           if (!rhsMember && lhsMember)
             return Descending;
-          if (lhsMember && !rhsMember)
+          if (!lhsMember && rhsMember)
             return Ascending;
-          if (lhsMember && rhsMember)
-            return rhsMember->getName().compare(lhsMember->getName());
+          if (!lhsMember && !rhsMember)
+            continue;
+
+          result = rhsMember->getName().compare(lhsMember->getName());
+          if (result != 0)
+            return result;
+
+          result = rhsMember->getInterfaceType().getString().compare(
+                                   lhsMember->getInterfaceType().getString());
+          if (result != 0)
+            return result;
+
+          auto lhsGeneric = lhsMember->getAsGenericContext(),
+               rhsGeneric = rhsMember->getAsGenericContext();
+          if (lhsGeneric && rhsGeneric) {
+            result = rhsGeneric->getGenericSignature().getAsString().compare(
+                               lhsGeneric->getGenericSignature().getAsString());
+            if (result != 0)
+              return result;
+          }
         }
       }
 

test/PrintAsObjC/extensions.swift

@@ -44,11 +44,11 @@ extension A2 {
 @objc @objcMembers class A3 {}
 
 // CHECK-NEXT: @interface A3 (SWIFT_EXTENSION(extensions))
-// CHECK-NEXT: @property (nonatomic, readonly) NSInteger some;
+// CHECK-NEXT: @property (nonatomic, readonly) NSInteger more;
 // CHECK-NEXT: @end
 // CHECK-EMPTY:
 // CHECK-NEXT: @interface A3 (SWIFT_EXTENSION(extensions))
-// CHECK-NEXT: @property (nonatomic, readonly) NSInteger more;
+// CHECK-NEXT: @property (nonatomic, readonly) NSInteger some;
 // CHECK-NEXT: @end
 // CHECK-EMPTY:
 extension A3 {
@@ -96,22 +96,22 @@ extension A5 {
 // CHECK-NEXT: @interface A6
 @objc class A6 {}
 
-extension A6 {
-  @objc(skippedBool:) func skipped(_: Bool) {}
-  @objc func def() {}
-}
 extension A6 {
   @objc(skippedInt:) func skipped(_: Int) {}
   @objc func abc() {}
 }
+extension A6 {
+  @objc(skippedBool:) func skipped(_: Bool) {}
+  @objc func def() {}
+}
 // CHECK: @interface A6 (SWIFT_EXTENSION(extensions))
-// CHECK-NEXT: - (void)skippedInt:
-// CHECK-NEXT: - (void)abc
+// CHECK-NEXT: - (void)skippedBool:
+// CHECK-NEXT: - (void)def
 // CHECK-NEXT: @end
 // CHECK-EMPTY:
 // CHECK-NEXT: @interface A6 (SWIFT_EXTENSION(extensions))
-// CHECK-NEXT: - (void)skippedBool:
-// CHECK-NEXT: - (void)def
+// CHECK-NEXT: - (void)skippedInt:
+// CHECK-NEXT: - (void)abc
 // CHECK-NEXT: @end
 // CHECK-EMPTY: