Skip to content

Commit 9e5cfa2

Browse files
rauhulrauhul
committed
Address review comments
1 parent 9b8a660 commit 9e5cfa2

File tree

7 files changed

+197
-48
lines changed

7 files changed

+197
-48
lines changed

‎Sources/Build/BuildDescription/ClangTargetBuildDescription.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,20 @@ public final class ClangTargetBuildDescription {
207207
}
208208
}
209209

210+
/// Determines the arguments needed to run `swift-symbolgraph-extract` for
211+
/// this module.
212+
public func symbolGraphExtractArguments() throws -> [String] {
213+
var args = [String]()
214+
215+
if self.clangTarget.isCXX {
216+
args += ["-cxx-interoperability-mode=default"]
217+
}
218+
if let cxxLanguageStandard = self.clangTarget.cxxLanguageStandard {
219+
args += ["-Xcc", "-std=\(cxxLanguageStandard)"]
220+
}
221+
return args
222+
}
223+
210224
/// Builds up basic compilation arguments for a source file in this target; these arguments may be different for C++
211225
/// vs non-C++.
212226
/// NOTE: The parameter to specify whether to get C++ semantics is currently optional, but this is only for revlock

‎Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift

Lines changed: 62 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -547,26 +547,7 @@ public final class SwiftTargetBuildDescription {
547547
args += ["-color-diagnostics"]
548548
}
549549

550-
// If this is a generated test discovery target or a test entry point, it might import a test
551-
// target that is built with C++ interop enabled. In that case, the test
552-
// discovery target must enable C++ interop as well
553-
switch testTargetRole {
554-
case .discovery, .entryPoint:
555-
for dependency in try self.target.recursiveTargetDependencies() {
556-
let dependencyScope = self.buildParameters.createScope(for: dependency)
557-
let dependencySwiftFlags = dependencyScope.evaluate(.OTHER_SWIFT_FLAGS)
558-
if let interopModeFlag = dependencySwiftFlags.first(where: { $0.hasPrefix("-cxx-interoperability-mode=") }) {
559-
args += [interopModeFlag]
560-
if interopModeFlag != "-cxx-interoperability-mode=off" {
561-
if let cxxStandard = self.package.manifest.cxxLanguageStandard {
562-
args += ["-Xcc", "-std=\(cxxStandard)"]
563-
}
564-
}
565-
break
566-
}
567-
}
568-
default: break
569-
}
550+
args += try self.cxxInteroperabilityModeArguments(allowDuplicate: false)
570551

571552
// Add arguments from declared build settings.
572553
args += try self.buildSettingsFlags()
@@ -644,6 +625,67 @@ public final class SwiftTargetBuildDescription {
644625

645626
return args
646627
}
628+
629+
/// Determines the arguments needed to run `swift-symbolgraph-extract` for
630+
/// this module.
631+
public func symbolGraphExtractArguments() throws -> [String] {
632+
var args = [String]()
633+
args += try self.cxxInteroperabilityModeArguments(allowDuplicate: true)
634+
return args
635+
}
636+
637+
/// Determines the arguments needed for cxx interop for this module.
638+
///
639+
/// If the current module or any of its linked dependencies requires cxx
640+
/// interop, cxx interop will be enabled on the current module.
641+
func cxxInteroperabilityModeArguments(
642+
// FIXME: Remove argument
643+
// This argument is added as a stop gap to support generating arguments
644+
// for tools which currently dont leverage "OTHER_SWIFT_FLAGS". In the
645+
// fullness of time, this function should operate on a strongly typed
646+
// "interopMode" property of SwiftTargetBuildDescription. Instead of
647+
// digging through "OTHER_SWIFT_FLAGS" manually.
648+
allowDuplicate: Bool
649+
) throws -> [String] {
650+
func _cxxInteroperabilityMode(for module: ResolvedModule) -> String? {
651+
let scope = self.buildParameters.createScope(for: module)
652+
let flags = scope.evaluate(.OTHER_SWIFT_FLAGS)
653+
return flags.first { $0.hasPrefix("-cxx-interoperability-mode=") }
654+
}
655+
656+
// Look for cxx interop mode in the current module, if set exit early,
657+
// the flag is already present.
658+
var cxxInteroperabilityMode: String?
659+
if let mode = _cxxInteroperabilityMode(for: self.target) {
660+
if allowDuplicate {
661+
cxxInteroperabilityMode = mode
662+
}
663+
}
664+
665+
// If the current module doesn't have cxx interop mode set, search
666+
// through the module's dependencies looking for the a module that
667+
// enables cxx interop and copy it's flag.
668+
if cxxInteroperabilityMode == nil {
669+
for module in try self.target.recursiveTargetDependencies() {
670+
if let mode = _cxxInteroperabilityMode(for: module) {
671+
cxxInteroperabilityMode = mode
672+
break
673+
}
674+
}
675+
}
676+
677+
var args = [String]()
678+
if let cxxInteroperabilityMode {
679+
// FIXME: this should reference a local cxxLanguageStandard property
680+
// It definitely should _never_ reach back into the manifest
681+
args = [cxxInteroperabilityMode]
682+
if let cxxStandard = self.package.manifest.cxxLanguageStandard {
683+
args += ["-Xcc", "-std=\(cxxStandard)"]
684+
}
685+
}
686+
return args
687+
}
688+
647689

648690
/// When `scanInvocation` argument is set to `true`, omit the side-effect producing arguments
649691
/// such as emitting a module or supplementary outputs.

‎Sources/Build/BuildDescription/TargetBuildDescription.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,13 @@ public enum TargetBuildDescription {
115115
return clangTargetBuildDescription.toolsVersion
116116
}
117117
}
118+
119+
/// Determines the arguments needed to run `swift-symbolgraph-extract` for
120+
/// this module.
121+
public func symbolGraphExtractArguments() throws -> [String] {
122+
switch self {
123+
case .swift(let target): try target.symbolGraphExtractArguments()
124+
case .clang(let target): try target.symbolGraphExtractArguments()
125+
}
126+
}
118127
}

‎Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,13 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
647647
try binaryTarget.parseXCFrameworks(for: triple, fileSystem: self.fileSystem)
648648
}
649649
}
650+
651+
public func symbolGraphExtractArguments(for module: ResolvedModule) throws -> [String] {
652+
guard let description = self.targetMap[module.id] else {
653+
throw InternalError("Expected description for module \(module)")
654+
}
655+
return try description.symbolGraphExtractArguments()
656+
}
650657
}
651658

652659
extension Basics.Diagnostic {

‎Sources/Commands/Utilities/SymbolGraphExtract.swift

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import ArgumentParser
1414
import Basics
1515
import PackageGraph
16-
@_spi(SwiftPMInternal) import PackageModel
16+
import PackageModel
1717
import SPMBuildCore
1818

1919
#if USE_IMPL_ONLY_IMPORTS
@@ -65,6 +65,9 @@ public struct SymbolGraphExtract {
6565

6666
// Construct arguments for extracting symbols for a single target.
6767
var commandLine = [self.tool.pathString]
68+
commandLine += try buildPlan.symbolGraphExtractArguments(for: module)
69+
70+
// FIXME: everything here should be in symbolGraphExtractArguments
6871
commandLine += ["-module-name", module.c99name]
6972
commandLine += try buildParameters.tripleArgs(for: module)
7073
commandLine += try buildPlan.createAPIToolCommonArgs(includeLibrarySearchPaths: true)
@@ -82,22 +85,7 @@ public struct SymbolGraphExtract {
8285
if includeSPISymbols {
8386
commandLine += ["-include-spi-symbols"]
8487
}
85-
86-
// If this target or any dependencies is is built with C++ interop
87-
// enabled then swift-symbolgraph-extract must also enable C++ interop.
88-
var cxxInterop = module.cxxInterop()
89-
if !cxxInterop {
90-
for module in try module.recursiveTargetDependencies() {
91-
if module.cxxInterop() {
92-
cxxInterop = true
93-
break
94-
}
95-
}
96-
}
97-
if cxxInterop {
98-
commandLine += ["-cxx-interoperability-mode=default"]
99-
}
100-
88+
10189
let extensionBlockSymbolsFlag = emitExtensionBlockSymbols ? "-emit-extension-block-symbols" : "-omit-extension-block-symbols"
10290
if DriverSupport.checkSupportedFrontendFlags(flags: [extensionBlockSymbolsFlag.trimmingCharacters(in: ["-"])], toolchain: buildParameters.toolchain, fileSystem: fileSystem) {
10391
commandLine += [extensionBlockSymbolsFlag]
@@ -122,14 +110,3 @@ public struct SymbolGraphExtract {
122110
return try process.waitUntilExit()
123111
}
124112
}
125-
126-
extension ResolvedModule {
127-
func cxxInterop() -> Bool {
128-
for setting in self.underlying.buildSettingsDescription {
129-
if case .interoperabilityMode(.Cxx) = setting.kind {
130-
return true
131-
}
132-
}
133-
return false
134-
}
135-
}

‎Sources/SPMBuildCore/BuildSystem/BuildSystem.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ public protocol BuildPlan {
9090

9191
func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String]
9292
func createREPLArguments() throws -> [String]
93+
94+
func symbolGraphExtractArguments(for module: ResolvedModule) throws -> [String]
9395
}
9496

9597
public protocol BuildSystemFactory {

‎Tests/BuildTests/BuildPlanTests.swift

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,13 +1715,36 @@ final class BuildPlanTests: XCTestCase {
17151715
)
17161716
)
17171717

1718+
// Assert compile args for swift modules importing cxx modules
17181719
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
17191720
XCTAssertMatch(
17201721
swiftInteropLib,
17211722
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
17221723
)
17231724
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
17241725
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1726+
1727+
// Assert symbolgraph-extract args for swift modules importing cxx modules
1728+
do {
1729+
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
1730+
XCTAssertMatch(
1731+
swiftInteropLib,
1732+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
1733+
)
1734+
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
1735+
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1736+
}
1737+
1738+
// Assert symbolgraph-extract args for cxx modules
1739+
do {
1740+
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
1741+
XCTAssertMatch(
1742+
swiftInteropLib,
1743+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
1744+
)
1745+
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
1746+
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1747+
}
17251748
}
17261749

17271750
func testSwiftCMixed() throws {
@@ -1894,6 +1917,81 @@ final class BuildPlanTests: XCTestCase {
18941917
])
18951918
}
18961919

1920+
func testSwiftSettings_interoperabilityMode_cxx() throws {
1921+
let Pkg: AbsolutePath = "/Pkg"
1922+
1923+
let fs: FileSystem = InMemoryFileSystem(
1924+
emptyFiles:
1925+
Pkg.appending(components: "Sources", "cxxLib", "lib.cpp").pathString,
1926+
Pkg.appending(components: "Sources", "cxxLib", "include", "lib.h").pathString,
1927+
Pkg.appending(components: "Sources", "swiftLib", "lib.swift").pathString,
1928+
Pkg.appending(components: "Sources", "swiftLib2", "lib2.swift").pathString
1929+
)
1930+
1931+
let observability = ObservabilitySystem.makeForTesting()
1932+
let graph = try loadModulesGraph(
1933+
fileSystem: fs,
1934+
manifests: [
1935+
Manifest.createRootManifest(
1936+
displayName: "Pkg",
1937+
path: .init(validating: Pkg.pathString),
1938+
cxxLanguageStandard: "c++20",
1939+
targets: [
1940+
TargetDescription(name: "cxxLib", dependencies: []),
1941+
TargetDescription(
1942+
name: "swiftLib",
1943+
dependencies: ["cxxLib"],
1944+
settings: [.init(tool: .swift, kind: .interoperabilityMode(.Cxx))]
1945+
),
1946+
TargetDescription(name: "swiftLib2", dependencies: ["swiftLib"]),
1947+
]
1948+
),
1949+
],
1950+
observabilityScope: observability.topScope
1951+
)
1952+
XCTAssertNoDiagnostics(observability.diagnostics)
1953+
1954+
let plan = try BuildPlan(
1955+
buildParameters: mockBuildParameters(),
1956+
graph: graph,
1957+
fileSystem: fs,
1958+
observabilityScope: observability.topScope
1959+
)
1960+
let result = try BuildPlanResult(plan: plan)
1961+
1962+
// Cxx module
1963+
do {
1964+
try XCTAssertMatch(
1965+
result.target(for: "cxxLib").clangTarget().symbolGraphExtractArguments(),
1966+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++20", .anySequence]
1967+
)
1968+
}
1969+
1970+
// Swift module directly importing cxx module
1971+
do {
1972+
try XCTAssertMatch(
1973+
result.target(for: "swiftLib").swiftTarget().compileArguments(),
1974+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++20", .anySequence]
1975+
)
1976+
try XCTAssertMatch(
1977+
result.target(for: "swiftLib").swiftTarget().symbolGraphExtractArguments(),
1978+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++20", .anySequence]
1979+
)
1980+
}
1981+
1982+
// Swift module transitively importing cxx module
1983+
do {
1984+
try XCTAssertMatch(
1985+
result.target(for: "swiftLib2").swiftTarget().compileArguments(),
1986+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++20", .anySequence]
1987+
)
1988+
try XCTAssertMatch(
1989+
result.target(for: "swiftLib2").swiftTarget().symbolGraphExtractArguments(),
1990+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++20", .anySequence]
1991+
)
1992+
}
1993+
}
1994+
18971995
func testREPLArguments() throws {
18981996
let Dep = AbsolutePath("/Dep")
18991997
let fs = InMemoryFileSystem(

0 commit comments

Comments
 (0)