Merge pull request #80213 from slavapestov/protocol-order-fix

AST: Fix a problem with the protocol order

Author: slavapestov

lib/AST/Decl.cpp

@@ -15,6 +15,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "swift/Strings.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/ASTMangler.h"
@@ -5434,9 +5435,29 @@ int TypeDecl::compare(const TypeDecl *type1, const TypeDecl *type2) {
 
   // Prefer module names earlier in the alphabet.
   if (dc1->isModuleScopeContext() && dc2->isModuleScopeContext()) {
-    auto module1 = dc1->getParentModule();
-    auto module2 = dc2->getParentModule();
-    if (int result = module1->getName().str().compare(module2->getName().str()))
+    // For protocol declarations specifically, the order here is part
+    // of the ABI, and so we must take care to get the correct module
+    // name for the comparison.
+    auto getModuleNameForOrder = [&](const TypeDecl *decl) -> StringRef {
+      // Respect @_originallyDefinedIn on the type itself, so that
+      // moving a protocol across modules does not change its
+      // position in the order.
+      auto alternateName = decl->getAlternateModuleName();
+      if (!alternateName.empty())
+        return alternateName;
+
+      // This used to just call getName(), which caused accidental ABI breaks
+      // when a module is imported under different aliases.
+      //
+      // Ideally, we would use getABIName() here. However, this would
+      // cause ABI breaks with the _Concurrency and CompilerSwiftSyntax
+      // builds, which already passed in a -module-abi-name but had
+      // existing symbols mangled with the wrong order.
+      auto *module = decl->getDeclContext()->getParentModule();
+      return module->getRealName().str();
+    };
+
+    if (int result = getModuleNameForOrder(type1).compare(getModuleNameForOrder(type2)))
       return result;
   }
 

test/SILGen/conformance_requirement_order.swift

@@ -0,0 +1,16 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module %s -module-name Horse -D LIB -emit-module-path %t/Horse.swiftmodule
+// RUN: %target-swift-emit-silgen %s -module-name main -I %t -module-alias SwiftHorse=Horse | %FileCheck %s
+
+#if LIB
+  public protocol Equine {}
+#else
+  import SwiftHorse
+
+  // Make sure we use the module's real name, and not its alias name, so that
+  // we always have Horse.Equine < Swift.Equatable. If this doesn't hold, the
+  // two requirements in the mangling will be flipped.
+
+  // CHECK-LABEL: sil hidden [ossa] @$s4main21requirementOrderHorseyyx0D06EquineRzSQRzlF : $@convention(thin) <T where T : Equine, T : Equatable> (@in_guaranteed T) -> () {
+  func requirementOrderHorse<T: Equine & Equatable>(_: T) {}
+#endif

test/SILGen/conformance_requirement_order_concurrency.swift

@@ -0,0 +1,12 @@
+// RUN: %target-swift-emit-silgen %s -module-name main | %FileCheck %s
+
+// See conformance_requirement_order.swift for the general case.
+//
+// Make sure that protocols from the _Concurrency module are ordered using
+// their real module name and not their ABI module name, which is "Swift".
+//
+// This was a mistake since they mangle like protocols from "Swift", but
+// at this point we cannot break existing code.
+
+// CHECK-LABEL: sil hidden [ossa] @$s4main27requirementOrderConcurrencyyyxSTRzScFRzlF : $@convention(thin) <T where T : Sequence, T : Executor> (@guaranteed T) -> () {
+func requirementOrderConcurrency<T: Executor & Sequence>(_: T) {}

test/SILGen/conformance_requirement_order_originally_defined_in.swift

@@ -0,0 +1,14 @@
+// RUN: %target-swift-emit-silgen %s -module-name main | %FileCheck %s
+
+// REQUIRES: OS=macosx
+
+@_originallyDefinedIn(module: "AnimalKit", macOS 10.10)
+@available(macOS 10.9, *)
+public protocol Ungulate {}
+
+@_originallyDefinedIn(module: "HorseKit", macOS 10.10)
+@available(macOS 10.9, *)
+public protocol Horse {}
+
+// CHECK-LABEL: sil [ossa] @$s4main39requirementOrderWithOriginallyDefinedInyyx9AnimalKit8UngulateRz05HorseI00K0RzlF : $@convention(thin) <T where T : Ungulate, T : Horse> (@in_guaranteed T) -> () {
+public func requirementOrderWithOriginallyDefinedIn<T: Ungulate & Horse>(_: T) {}