Skip to content

Commit 38c478f

Browse files
committed
AST: Look through module aliases in TypeDecl::compare()
The last step in building a generic signature is to sort the requirements. Requirements are sorted by comparing their subject types. If two requirements have the same subject type, which can only happen with conformance requirements, we break the tie by comparing protocol declarations. We compare protocol declarations using TypeDecl::compare(), which is a shortlex order on the components of the fully qualified name of a protocol (eg, Swift.Sequence, etc.) While this order is part of the ABI, it has not been updated over the years for several important changes: - It did not handle module aliases; if we import a module via an alias, we should use the real module name to compare protocols, and not the aliased name. This produced inconsistent results if the same module was imported under different names, which can happen with module interface files that use module aliases. - It did not handle the -module-abi-name flag. Changing the ABI name of a module changes how we mangle protocol names, and the order should match the mangling. This change fixes the first case only. The second requires more careful staging, because of _Concurrency and CompilerSwiftSyntax. Fixes rdar://147441890.
1 parent 367e5e2 commit 38c478f

File tree

3 files changed

+45
-3
lines changed

3 files changed

+45
-3
lines changed

lib/AST/Decl.cpp

+17-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
//
1616
//===----------------------------------------------------------------------===//
1717

18+
#include "swift/Strings.h"
1819
#include "swift/AST/Decl.h"
1920
#include "swift/AST/ASTContext.h"
2021
#include "swift/AST/ASTMangler.h"
@@ -5434,9 +5435,22 @@ int TypeDecl::compare(const TypeDecl *type1, const TypeDecl *type2) {
54345435

54355436
// Prefer module names earlier in the alphabet.
54365437
if (dc1->isModuleScopeContext() && dc2->isModuleScopeContext()) {
5437-
auto module1 = dc1->getParentModule();
5438-
auto module2 = dc2->getParentModule();
5439-
if (int result = module1->getName().str().compare(module2->getName().str()))
5438+
// For protocol declarations specifically, the order here is part
5439+
// of the ABI, and so we must take care to get the correct module
5440+
// name for the comparison.
5441+
auto getModuleNameForOrder = [&](const TypeDecl *decl) -> StringRef {
5442+
// This used to just call getName(), which caused accidental ABI breaks
5443+
// when a module is imported under different aliases.
5444+
//
5445+
// Ideally, we would use getABIName() here. However, this would
5446+
// cause ABI breaks with the _Concurrency and CompilerSwiftSyntax
5447+
// builds, which already passed in a -module-abi-name but had
5448+
// existing symbols mangled with the wrong order.
5449+
auto *module = decl->getDeclContext()->getParentModule();
5450+
return module->getRealName().str();
5451+
};
5452+
5453+
if (int result = getModuleNameForOrder(type1).compare(getModuleNameForOrder(type2)))
54405454
return result;
54415455
}
54425456

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -emit-module %s -module-name Horse -D LIB -emit-module-path %t/Horse.swiftmodule
3+
// RUN: %target-swift-emit-silgen %s -module-name main -I %t -module-alias SwiftHorse=Horse | %FileCheck %s
4+
5+
#if LIB
6+
public protocol Equine {}
7+
#else
8+
import SwiftHorse
9+
10+
// Make sure we use the module's real name, and not its alias name, so that
11+
// we always have Horse.Equine < Swift.Equatable. If this doesn't hold, the
12+
// two requirements in the mangling will be flipped.
13+
14+
// CHECK-LABEL: sil hidden [ossa] @$s4main21requirementOrderHorseyyx0D06EquineRzSQRzlF : $@convention(thin) <T where T : Equine, T : Equatable> (@in_guaranteed T) -> () {
15+
func requirementOrderHorse<T: Equine & Equatable>(_: T) {}
16+
#endif
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// RUN: %target-swift-emit-silgen %s -module-name main | %FileCheck %s
2+
3+
// See conformance_requirement_order.swift for the general case.
4+
//
5+
// Make sure that protocols from the _Concurrency module are ordered using
6+
// their real module name and not their ABI module name, which is "Swift".
7+
//
8+
// This was a mistake since they mangle like protocols from "Swift", but
9+
// at this point we cannot break existing code.
10+
11+
// CHECK-LABEL: sil hidden [ossa] @$s4main27requirementOrderConcurrencyyyxSTRzScFRzlF : $@convention(thin) <T where T : Sequence, T : Executor> (@guaranteed T) -> () {
12+
func requirementOrderConcurrency<T: Executor & Sequence>(_: T) {}

0 commit comments

Comments
 (0)