Skip to content

Commit 1948907

Browse files
use RespectOriginallyDefinedIn when mangling extension contexts (#82348)
Resolves rdar://152598492 Consider the following Swift, adapted from a real-world framework: ```swift @available(macOS 10.8, *) @_originallyDefinedIn(module: "another", macOS 11.0) public struct SimpleStruct {} @available(macOS 12.0, iOS 13.0, *) public extension SimpleStruct { struct InnerStruct {} } ``` In this scenario, `SimpleStruct` was originally in a module called `another`, but was migrated to this module around the time of macOS 11.0. Since then, the module was ported to iOS and gained a nested type `SimpleStruct.InnerStruct`. When mangling USRs for this nested type, the result differs depending on whether we're targeting macOS or iOS. They're mostly the same, but the macOS build yields a USR with an `AAE` infix, designating that the `InnerStruct` was defined in an extension from a module with the name of the base module. On iOS, this infix does not exist. The reason this is happening is because of the implementation of `getAlternateModuleName` checking the availability spec in the `@_originallyDefinedIn` attribute against the currently active target. If the target matches the spec, then the alternate module name is reported, otherwise the real module name is. Since the iOS build reports the real module name, the mangling code doesn't bother including the extension-context infix, instead just opting to include the parent type's name and moving on. This PR routes around this issue by passing the `RespectOriginallyDefinedIn` variable to the `ExtensionDecl::isInSameDefiningModule` method, and using that to skip the alternate module name entirely. It also sets `RespectOriginallyDefinedIn` to `false` in more places when mangling USRs, but i'm not 100% confident that it was all necessary. The goal was to make USRs more consistent across platforms, regardless of the surrounding context.
1 parent 1cf426c commit 1948907

File tree

7 files changed

+50
-7
lines changed

7 files changed

+50
-7
lines changed

include/swift/AST/Decl.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2132,7 +2132,10 @@ class ExtensionDecl final : public GenericContext, public Decl,
21322132

21332133
/// Determine whether this extension context is in the same defining module as
21342134
/// the original nominal type context.
2135-
bool isInSameDefiningModule() const;
2135+
///
2136+
/// \param RespectOriginallyDefinedIn Whether to respect
2137+
/// \c @_originallyDefinedIn attributes or the actual location of the decls.
2138+
bool isInSameDefiningModule(bool RespectOriginallyDefinedIn = true) const;
21362139

21372140
/// Determine whether this extension is equivalent to one that requires at
21382141
/// at least some constraints to be written in the source.

lib/AST/ASTMangler.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -946,6 +946,8 @@ std::string ASTMangler::mangleObjCRuntimeName(const NominalTypeDecl *Nominal) {
946946

947947
std::string ASTMangler::mangleTypeAsContextUSR(const NominalTypeDecl *type) {
948948
beginManglingWithoutPrefix();
949+
llvm::SaveAndRestore<bool> respectOriginallyDefinedInRAII(
950+
RespectOriginallyDefinedIn, false);
949951
llvm::SaveAndRestore<bool> allowUnnamedRAII(AllowNamelessEntities, true);
950952
BaseEntitySignature base(type);
951953
appendContext(type, base, type->getAlternateModuleName());
@@ -1014,6 +1016,8 @@ ASTMangler::mangleAnyDecl(const ValueDecl *Decl,
10141016

10151017
std::string ASTMangler::mangleDeclAsUSR(const ValueDecl *Decl,
10161018
StringRef USRPrefix) {
1019+
llvm::SaveAndRestore<bool> respectOriginallyDefinedInRAII(
1020+
RespectOriginallyDefinedIn, false);
10171021
return (llvm::Twine(USRPrefix) + mangleAnyDecl(Decl, false)).str();
10181022
}
10191023

@@ -1022,6 +1026,8 @@ std::string ASTMangler::mangleAccessorEntityAsUSR(AccessorKind kind,
10221026
StringRef USRPrefix,
10231027
bool isStatic) {
10241028
beginManglingWithoutPrefix();
1029+
llvm::SaveAndRestore<bool> respectOriginallyDefinedInRAII(
1030+
RespectOriginallyDefinedIn, false);
10251031
llvm::SaveAndRestore<bool> allowUnnamedRAII(AllowNamelessEntities, true);
10261032
Buffer << USRPrefix;
10271033
appendAccessorEntity(getCodeForAccessorKind(kind), decl, isStatic);
@@ -3053,9 +3059,9 @@ void ASTMangler::appendExtension(const ExtensionDecl* ext,
30533059
// "extension is to a protocol" would no longer be a reason to use the
30543060
// extension mangling, because an extension method implementation could be
30553061
// resiliently moved into the original protocol itself.
3056-
if (ext->isInSameDefiningModule() // case 1
3057-
&& !sigParts.hasRequirements() // case 2
3058-
&& !ext->getDeclaredInterfaceType()->isExistentialType()) { // case 3
3062+
if (ext->isInSameDefiningModule(RespectOriginallyDefinedIn) // case 1
3063+
&& !sigParts.hasRequirements() // case 2
3064+
&& !ext->getDeclaredInterfaceType()->isExistentialType()) { // case 3
30593065
// skip extension mangling
30603066
return appendAnyGenericType(decl);
30613067
}

lib/AST/Decl.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2169,10 +2169,13 @@ bool ExtensionDecl::isWrittenWithConstraints() const {
21692169
return false;
21702170
}
21712171

2172-
bool ExtensionDecl::isInSameDefiningModule() const {
2172+
bool ExtensionDecl::isInSameDefiningModule(
2173+
bool RespectOriginallyDefinedIn) const {
21732174
auto decl = getExtendedNominal();
2174-
auto extensionAlterName = getAlternateModuleName();
2175-
auto typeAlterName = decl->getAlternateModuleName();
2175+
auto extensionAlterName =
2176+
RespectOriginallyDefinedIn ? getAlternateModuleName() : "";
2177+
auto typeAlterName =
2178+
RespectOriginallyDefinedIn ? decl->getAlternateModuleName() : "";
21762179

21772180
if (!extensionAlterName.empty()) {
21782181
if (!typeAlterName.empty()) {
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %empty-directory(%t/macos)
3+
// RUN: %empty-directory(%t/ios)
4+
5+
// RUN: %target-swift-frontend -target %target-cpu-apple-macos %s -module-name OriginallyDefinedInExtension -emit-module -emit-module-path %t/macos/OriginallyDefinedInExtension.swiftmodule -emit-symbol-graph -emit-symbol-graph-dir %t/macos/
6+
// RUN: %FileCheck %s --input-file %t/macos/OriginallyDefinedInExtension.symbols.json
7+
// RUN: %target-swift-frontend -target %target-cpu-apple-ios-simulator %s -module-name OriginallyDefinedInExtension -emit-module -emit-module-path %t/ios/OriginallyDefinedInExtension.swiftmodule -emit-symbol-graph -emit-symbol-graph-dir %t/ios/
8+
// RUN: %FileCheck %s --input-file %t/ios/OriginallyDefinedInExtension.symbols.json
9+
10+
// CHECK: "precise":"s:28OriginallyDefinedInExtension12SimpleStructV05InnerF0V"
11+
12+
// REQUIRES: SWIFT_SDK=osx
13+
// REQUIRES: SWIFT_SDK=ios_simulator
14+
15+
@available(macOS 10.8, *)
16+
@_originallyDefinedIn(module: "another", macOS 11.0)
17+
public struct SimpleStruct {}
18+
19+
@available(macOS 12.0, iOS 13.0, *)
20+
public extension SimpleStruct {
21+
struct InnerStruct {}
22+
}

test/lit.cfg

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,11 @@ config.available_features.add("SWIFT_VERSION=" + swift_version)
905905

906906
config.available_features.add("STDLIB_VARIANT={}".format(config.variant_suffix[1:]))
907907

908+
if "target-same-as-host" in config.available_features:
909+
# Only add SWIFT_SDKS features if we're building host tools
910+
for sdk in config.swift_sdks:
911+
config.available_features.add("SWIFT_SDK=" + sdk.lower())
912+
908913
if "optimized_stdlib" in config.available_features:
909914
config.available_features.add("optimized_stdlib_" + run_cpu)
910915

test/lit.site.cfg.in

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,8 @@ config.freestanding_sdk_name = "@SWIFT_SDK_FREESTANDING_LIB_SUBDIR@"
173173
if '@SWIFT_BUILD_SWIFT_SYNTAX@' == 'TRUE':
174174
config.available_features.add('swift_swift_parser')
175175

176+
config.swift_sdks = "@SWIFT_SDKS@".split(";")
177+
176178
# Let the main config do the real work.
177179
if config.test_exec_root is None:
178180
config.test_exec_root = os.path.dirname(lit.util.abs_path_preserve_drive(__file__))

validation-test/lit.site.cfg.in

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ config.swift_stdlib_enable_objc_interop = "@SWIFT_STDLIB_ENABLE_OBJC_INTEROP@" =
137137
# Configured in DarwinSDKs.cmake
138138
config.freestanding_sdk_name = "@SWIFT_SDK_FREESTANDING_LIB_SUBDIR@"
139139

140+
config.swift_sdks = "@SWIFT_SDKS@".split(";")
141+
140142
# Let the main config do the real work.
141143
config.test_exec_root = os.path.dirname(os.path.realpath(__file__))
142144
lit_config.load_config(config, os.path.join(config.test_exec_root, "lit.swift-features.cfg"))

0 commit comments

Comments
 (0)