From 9a0598cda73aa04b32f6a47b3568c60dc39f7455 Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Tue, 5 Mar 2024 09:12:13 -0800 Subject: [PATCH 1/2] TBDGen: Use LinkerPlatformId enum type instead of casting to uint8_t. Increase type safety by consistently using the `LinkerPlatformId` enum type, instead of casting to/from `uint8_t` unnecessarily. NFC. --- lib/IRGen/TBDGen.cpp | 24 ++++++++++++------------ lib/IRGen/TBDGenVisitor.h | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/IRGen/TBDGen.cpp b/lib/IRGen/TBDGen.cpp index ea5d6a7d33c2a..b88850406b3ec 100644 --- a/lib/IRGen/TBDGen.cpp +++ b/lib/IRGen/TBDGen.cpp @@ -100,24 +100,23 @@ getAllMovedPlatformVersions(Decl *D) { return Results; } -static StringRef getLinkerPlatformName(uint8_t Id) { +static StringRef getLinkerPlatformName(LinkerPlatformId Id) { switch (Id) { -#define LD_PLATFORM(Name, Id) case Id: return #Name; +#define LD_PLATFORM(Name, Id) case LinkerPlatformId::Name: return #Name; #include "ldPlatformKinds.def" - default: - llvm_unreachable("unrecognized platform id"); } + llvm_unreachable("unrecognized platform id"); } -static std::optional getLinkerPlatformId(StringRef Platform) { - return llvm::StringSwitch>(Platform) -#define LD_PLATFORM(Name, Id) .Case(#Name, Id) +static std::optional getLinkerPlatformId(StringRef Platform) { + return llvm::StringSwitch>(Platform) +#define LD_PLATFORM(Name, Id) .Case(#Name, LinkerPlatformId::Name) #include "ldPlatformKinds.def" .Default(std::nullopt); } StringRef InstallNameStore::getInstallName(LinkerPlatformId Id) const { - auto It = PlatformInstallName.find((uint8_t)Id); + auto It = PlatformInstallName.find(Id); if (It == PlatformInstallName.end()) return InstallName; else @@ -129,8 +128,9 @@ static std::string getScalaNodeText(Node *N) { return cast(N)->getValue(Buffer).str(); } -static std::set getSequenceNodePlatformList(ASTContext &Ctx, Node *N) { - std::set Results; +static std::set getSequenceNodePlatformList(ASTContext &Ctx, + Node *N) { + std::set Results; for (auto &E: *cast(N)) { auto Platform = getScalaNodeText(&E); auto Id = getLinkerPlatformId(Platform); @@ -158,7 +158,7 @@ parseEntry(ASTContext &Ctx, auto *MN = cast(&*It); std::string ModuleName; std::string InstallName; - std::optional> Platforms; + std::optional> Platforms; for (auto &Pair: *MN) { auto Key = getScalaNodeText(Pair.getKey()); auto* Value = Pair.getValue(); @@ -333,7 +333,7 @@ void TBDGenVisitor::addLinkerDirectiveSymbolsLdPrevious( OS << "$ld$previous$"; OS << InstallName << "$"; OS << ComptibleVersion << "$"; - OS << std::to_string((uint8_t)PlatformNumber) << "$"; + OS << std::to_string(static_cast(PlatformNumber)) << "$"; static auto getMinor = [](std::optional Minor) { return Minor.has_value() ? *Minor : 0; }; diff --git a/lib/IRGen/TBDGenVisitor.h b/lib/IRGen/TBDGenVisitor.h index d50e8e1023480..53c6cdec50c07 100644 --- a/lib/IRGen/TBDGenVisitor.h +++ b/lib/IRGen/TBDGenVisitor.h @@ -52,7 +52,7 @@ struct InstallNameStore { std::string InstallName; // The install name specific to the platform id. This takes precedence over // the default install name. - std::map PlatformInstallName; + std::map PlatformInstallName; StringRef getInstallName(LinkerPlatformId Id) const; }; From f49dbb06b65c825e7c4060da9591c29bd12db4ae Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Tue, 5 Mar 2024 09:14:51 -0800 Subject: [PATCH 2/2] TBDGen: Specify the correct macCatalyst platform ID in `$ld$previous` directives. Resolves rdar://123491072 --- include/swift/AST/Attr.h | 2 +- lib/AST/Attr.cpp | 3 +- lib/IRGen/TBDGen.cpp | 30 +++++++++------ .../Inputs/install-name-map-toasterkit.json | 5 +++ .../linker-directives-ld-previous-macos.swift | 37 ++++++++++--------- 5 files changed, 45 insertions(+), 32 deletions(-) diff --git a/include/swift/AST/Attr.h b/include/swift/AST/Attr.h index 1119c156042de..02f2bda42cd23 100644 --- a/include/swift/AST/Attr.h +++ b/include/swift/AST/Attr.h @@ -1862,8 +1862,8 @@ class OriginallyDefinedInAttr: public DeclAttribute { struct ActiveVersion { StringRef ModuleName; PlatformKind Platform; - bool IsSimulator; llvm::VersionTuple Version; + bool ForTargetVariant = false; }; /// Returns non-optional if this attribute is active given the current platform. diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp index cfcb0449307e7..ec4cfdd289f75 100644 --- a/lib/AST/Attr.cpp +++ b/lib/AST/Attr.cpp @@ -2215,7 +2215,6 @@ OriginallyDefinedInAttr::isActivePlatform(const ASTContext &ctx) const { Result.Version = MovedVersion; Result.ModuleName = OriginalModuleName; if (isPlatformActive(Platform, ctx.LangOpts, /*TargetVariant*/false)) { - Result.IsSimulator = ctx.LangOpts.Target.isSimulatorEnvironment(); return Result; } @@ -2224,7 +2223,7 @@ OriginallyDefinedInAttr::isActivePlatform(const ASTContext &ctx) const { // libraries. if (ctx.LangOpts.TargetVariant.has_value() && isPlatformActive(Platform, ctx.LangOpts, /*TargetVariant*/true)) { - Result.IsSimulator = ctx.LangOpts.TargetVariant->isSimulatorEnvironment(); + Result.ForTargetVariant = true; return Result; } return std::nullopt; diff --git a/lib/IRGen/TBDGen.cpp b/lib/IRGen/TBDGen.cpp index b88850406b3ec..fb2762253db6b 100644 --- a/lib/IRGen/TBDGen.cpp +++ b/lib/IRGen/TBDGen.cpp @@ -233,7 +233,12 @@ TBDGenVisitor::parsePreviousModuleInstallNameMap() { } static LinkerPlatformId -getLinkerPlatformId(OriginallyDefinedInAttr::ActiveVersion Ver) { +getLinkerPlatformId(OriginallyDefinedInAttr::ActiveVersion Ver, + ASTContext &Ctx) { + auto target = + Ver.ForTargetVariant ? Ctx.LangOpts.TargetVariant : Ctx.LangOpts.Target; + bool isSimulator = target ? target->isSimulatorEnvironment() : false; + switch(Ver.Platform) { case swift::PlatformKind::none: llvm_unreachable("cannot find platform kind"); @@ -243,16 +248,16 @@ getLinkerPlatformId(OriginallyDefinedInAttr::ActiveVersion Ver) { llvm_unreachable("not used for this platform"); case swift::PlatformKind::iOS: case swift::PlatformKind::iOSApplicationExtension: - return Ver.IsSimulator ? LinkerPlatformId::iOS_sim: - LinkerPlatformId::iOS; + if (target && target->isMacCatalystEnvironment()) + return LinkerPlatformId::macCatalyst; + return isSimulator ? LinkerPlatformId::iOS_sim : LinkerPlatformId::iOS; case swift::PlatformKind::tvOS: case swift::PlatformKind::tvOSApplicationExtension: - return Ver.IsSimulator ? LinkerPlatformId::tvOS_sim: - LinkerPlatformId::tvOS; + return isSimulator ? LinkerPlatformId::tvOS_sim : LinkerPlatformId::tvOS; case swift::PlatformKind::watchOS: case swift::PlatformKind::watchOSApplicationExtension: - return Ver.IsSimulator ? LinkerPlatformId::watchOS_sim: - LinkerPlatformId::watchOS; + return isSimulator ? LinkerPlatformId::watchOS_sim + : LinkerPlatformId::watchOS; case swift::PlatformKind::macOS: case swift::PlatformKind::macOSApplicationExtension: return LinkerPlatformId::macOS; @@ -264,8 +269,9 @@ getLinkerPlatformId(OriginallyDefinedInAttr::ActiveVersion Ver) { } static StringRef -getLinkerPlatformName(OriginallyDefinedInAttr::ActiveVersion Ver) { - return getLinkerPlatformName((uint8_t)getLinkerPlatformId(Ver)); +getLinkerPlatformName(OriginallyDefinedInAttr::ActiveVersion Ver, + ASTContext &Ctx) { + return getLinkerPlatformName(getLinkerPlatformId(Ver, Ctx)); } /// Find the most relevant introducing version of the decl stack we have visited @@ -313,17 +319,17 @@ void TBDGenVisitor::addLinkerDirectiveSymbolsLdPrevious( // so we don't need the linker directives. if (*IntroVer >= Ver.Version) continue; - auto PlatformNumber = getLinkerPlatformId(Ver); + auto PlatformNumber = getLinkerPlatformId(Ver, Ctx); auto It = previousInstallNameMap->find(Ver.ModuleName.str()); if (It == previousInstallNameMap->end()) { Ctx.Diags.diagnose(SourceLoc(), diag::cannot_find_install_name, - Ver.ModuleName, getLinkerPlatformName(Ver)); + Ver.ModuleName, getLinkerPlatformName(Ver, Ctx)); continue; } auto InstallName = It->second.getInstallName(PlatformNumber); if (InstallName.empty()) { Ctx.Diags.diagnose(SourceLoc(), diag::cannot_find_install_name, - Ver.ModuleName, getLinkerPlatformName(Ver)); + Ver.ModuleName, getLinkerPlatformName(Ver, Ctx)); continue; } llvm::SmallString<64> Buffer; diff --git a/test/TBD/Inputs/install-name-map-toasterkit.json b/test/TBD/Inputs/install-name-map-toasterkit.json index f6262f6726e4b..700305526a5a6 100644 --- a/test/TBD/Inputs/install-name-map-toasterkit.json +++ b/test/TBD/Inputs/install-name-map-toasterkit.json @@ -8,5 +8,10 @@ "module": "ToasterKit", "install_name": "/System/Previous/iOS/ToasterKit.dylib", "platforms": ["iOS", "iOS_sim"] + }, + { + "module": "ToasterKit", + "install_name": "/System/Previous/macCatalyst/ToasterKit.dylib", + "platforms": ["macCatalyst"] } ] diff --git a/test/TBD/linker-directives-ld-previous-macos.swift b/test/TBD/linker-directives-ld-previous-macos.swift index 05afdac613534..3e75b2a7daca9 100644 --- a/test/TBD/linker-directives-ld-previous-macos.swift +++ b/test/TBD/linker-directives-ld-previous-macos.swift @@ -2,24 +2,27 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-frontend -typecheck %S/Inputs/linker-directive.swift -tbd-is-installapi -emit-tbd -emit-tbd-path %t/linker_directives.tbd -previous-module-installname-map-file %S/Inputs/install-name-map-toasterkit.json -tbd-install_name toasterkit -// RUN: %llvm-nm %t/linker_directives.tbd | %FileCheck %s -// RUN: %llvm-nm %t/linker_directives.tbd | %FileCheck -check-prefix=CHECK-NO-NEW-SYMBOL %s -// RUN: %target-swift-frontend -typecheck %S/Inputs/linker-directive.swift -emit-tbd -emit-tbd-path %t/linker_directives.tbd -previous-module-installname-map-file %S/Inputs/install-name-map-toasterkit.json -tbd-install_name toasterkit -// RUN: %llvm-nm %t/linker_directives.tbd | %FileCheck %s -// RUN: %llvm-nm %t/linker_directives.tbd | %FileCheck -check-prefix=CHECK-NO-NEW-SYMBOL %s +// RUN: %target-swift-frontend -typecheck %S/Inputs/linker-directive.swift -tbd-is-installapi -emit-tbd -emit-tbd-path %t/linker_directives_installapi.tbd -previous-module-installname-map-file %S/Inputs/install-name-map-toasterkit.json -tbd-install_name toasterkit +// RUN: %llvm-nm %t/linker_directives_installapi.tbd | %FileCheck %s --check-prefixes=CHECK,CHECK-MAC --implicit-check-not "System/Previous/macCatalyst" +// RUN: %target-swift-frontend -typecheck %S/Inputs/linker-directive.swift -emit-tbd -emit-tbd-path %t/linker_directives_macos.tbd -previous-module-installname-map-file %S/Inputs/install-name-map-toasterkit.json -tbd-install_name toasterkit +// RUN: %llvm-nm %t/linker_directives_macos.tbd | %FileCheck %s --check-prefixes=CHECK,CHECK-MAC --implicit-check-not "System/Previous/macCatalyst" -// RUN: %target-swift-frontend -target-variant x86_64-apple-ios13.1-macabi -typecheck %S/Inputs/linker-directive.swift -emit-tbd -emit-tbd-path %t/linker_directives.tbd -previous-module-installname-map-file %S/Inputs/install-name-map-toasterkit.json -tbd-install_name toasterkit -// RUN: %llvm-nm %t/linker_directives.tbd | %FileCheck -check-prefix=CHECK-ZIPPERED %s -// RUN: %llvm-nm %t/linker_directives.tbd | %FileCheck -check-prefix=CHECK-NO-NEW-SYMBOL %s +// RUN: %target-swift-frontend -target-variant x86_64-apple-ios13.1-macabi -typecheck %S/Inputs/linker-directive.swift -emit-tbd -emit-tbd-path %t/linker_directives_macos_macabi.tbd -previous-module-installname-map-file %S/Inputs/install-name-map-toasterkit.json -tbd-install_name toasterkit +// RUN: %llvm-nm %t/linker_directives_macos_macabi.tbd | %FileCheck -check-prefixes=CHECK,CHECK-MAC,CHECK-MACCATALYST %s -// CHECK: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit5toastyyF$ -// CHECK: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit7VehicleV4moveyyF$ -// CHECK: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit7VehicleVMa$ -// CHECK: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit7VehicleVMn$ -// CHECK: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit7VehicleVN$ +// RUN: %target-swift-frontend -target x86_64-apple-ios13.1-macabi -typecheck %S/Inputs/linker-directive.swift -emit-tbd -emit-tbd-path %t/linker_directives_macabi.tbd -previous-module-installname-map-file %S/Inputs/install-name-map-toasterkit.json -tbd-install_name toasterkit +// R/UN: %llvm-nm %t/linker_directives_macabi.tbd | %FileCheck -check-prefixes=CHECK,CHECK-MACCATALYST %s --implicit-check-not "System/Previous/macOS" -// CHECK-ZIPPERED: $ld$previous$/System/Previous/iOS/ToasterKit.dylib$$2$10.2$13.0$_$s10ToasterKit5toastyyF$ -// CHECK-ZIPPERED: $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit5toastyyF$ +// CHECK-MACCATALYST: D $ld$previous$/System/Previous/macCatalyst/ToasterKit.dylib$$6$10.2$13.0$_$s10ToasterKit5toastyyF$ +// CHECK-MACCATALYST: D $ld$previous$/System/Previous/macCatalyst/ToasterKit.dylib$$6$10.2$13.0$_$s10ToasterKit7VehicleV4moveyyF$ +// CHECK-MACCATALYST: D $ld$previous$/System/Previous/macCatalyst/ToasterKit.dylib$$6$10.2$13.0$_$s10ToasterKit7VehicleVMa$ +// CHECK-MACCATALYST: D $ld$previous$/System/Previous/macCatalyst/ToasterKit.dylib$$6$10.2$13.0$_$s10ToasterKit7VehicleVMn$ +// CHECK-MACCATALYST: D $ld$previous$/System/Previous/macCatalyst/ToasterKit.dylib$$6$10.2$13.0$_$s10ToasterKit7VehicleVN$ -// CHECK-NO-NEW-SYMBOL-NOT: $_$s10ToasterKit7VehicleV32originallyDefinedInCurrentModuleyyF +// CHECK-MAC: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit5toastyyF$ +// CHECK-MAC: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit7VehicleV4moveyyF$ +// CHECK-MAC: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit7VehicleVMa$ +// CHECK-MAC: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit7VehicleVMn$ +// CHECK-MAC: D $ld$previous$/System/Previous/macOS/ToasterKit.dylib$$1$10.8$10.15$_$s10ToasterKit7VehicleVN$ + +// CHECK-NOT: $_$s10ToasterKit7VehicleV32originallyDefinedInCurrentModuleyyF