Skip to content

Commit 345fe41

Browse files
rauhulrauhul
committed
Address review comments
1 parent 3eb2e9d commit 345fe41

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
@@ -536,26 +536,7 @@ public final class SwiftTargetBuildDescription {
536536
args += ["-color-diagnostics"]
537537
}
538538

539-
// If this is a generated test discovery target or a test entry point, it might import a test
540-
// target that is built with C++ interop enabled. In that case, the test
541-
// discovery target must enable C++ interop as well
542-
switch testTargetRole {
543-
case .discovery, .entryPoint:
544-
for dependency in try self.target.recursiveTargetDependencies() {
545-
let dependencyScope = self.buildParameters.createScope(for: dependency)
546-
let dependencySwiftFlags = dependencyScope.evaluate(.OTHER_SWIFT_FLAGS)
547-
if let interopModeFlag = dependencySwiftFlags.first(where: { $0.hasPrefix("-cxx-interoperability-mode=") }) {
548-
args += [interopModeFlag]
549-
if interopModeFlag != "-cxx-interoperability-mode=off" {
550-
if let cxxStandard = self.package.manifest.cxxLanguageStandard {
551-
args += ["-Xcc", "-std=\(cxxStandard)"]
552-
}
553-
}
554-
break
555-
}
556-
}
557-
default: break
558-
}
539+
args += try self.cxxInteroperabilityModeArguments(allowDuplicate: false)
559540

560541
// Add arguments from declared build settings.
561542
args += try self.buildSettingsFlags()
@@ -633,6 +614,67 @@ public final class SwiftTargetBuildDescription {
633614

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

637679
/// When `scanInvocation` argument is set to `true`, omit the side-effect producing arguments
638680
/// 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.targetTripleArgs(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
extension BuildPlan {

‎Tests/BuildTests/BuildPlanTests.swift

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1719,13 +1719,36 @@ final class BuildPlanTests: XCTestCase {
17191719
)
17201720
)
17211721

1722+
// Assert compile args for swift modules importing cxx modules
17221723
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
17231724
XCTAssertMatch(
17241725
swiftInteropLib,
17251726
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
17261727
)
17271728
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
17281729
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1730+
1731+
// Assert symbolgraph-extract args for swift modules importing cxx modules
1732+
do {
1733+
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
1734+
XCTAssertMatch(
1735+
swiftInteropLib,
1736+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
1737+
)
1738+
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
1739+
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1740+
}
1741+
1742+
// Assert symbolgraph-extract args for cxx modules
1743+
do {
1744+
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
1745+
XCTAssertMatch(
1746+
swiftInteropLib,
1747+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
1748+
)
1749+
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
1750+
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1751+
}
17291752
}
17301753

17311754
func testSwiftCMixed() throws {
@@ -1900,6 +1923,81 @@ final class BuildPlanTests: XCTestCase {
19001923
])
19011924
}
19021925

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

0 commit comments

Comments
 (0)