Skip to content

Commit f9bb69f

Browse files
rauhulrauhul
committed
Address review comments
1 parent 49f3604 commit f9bb69f

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
@@ -552,26 +552,7 @@ public final class SwiftTargetBuildDescription {
552552
args += ["-color-diagnostics"]
553553
}
554554

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

576557
// Add arguments from declared build settings.
577558
args += try self.buildSettingsFlags()
@@ -649,6 +630,67 @@ public final class SwiftTargetBuildDescription {
649630

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

653695
/// When `scanInvocation` argument is set to `true`, omit the side-effect producing arguments
654696
/// 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
@@ -660,6 +660,13 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
660660
try binaryTarget.parseXCFrameworks(for: triple, fileSystem: self.fileSystem)
661661
}
662662
}
663+
664+
public func symbolGraphExtractArguments(for module: ResolvedModule) throws -> [String] {
665+
guard let description = self.targetMap[module.id] else {
666+
throw InternalError("Expected description for module \(module)")
667+
}
668+
return try description.symbolGraphExtractArguments()
669+
}
663670
}
664671

665672
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
extension BuildPlan {

‎Tests/BuildTests/BuildPlanTests.swift

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,13 +1727,36 @@ final class BuildPlanTests: XCTestCase {
17271727
)
17281728
)
17291729

1730+
// Assert compile args for swift modules importing cxx modules
17301731
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
17311732
XCTAssertMatch(
17321733
swiftInteropLib,
17331734
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
17341735
)
17351736
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
17361737
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1738+
1739+
// Assert symbolgraph-extract args for swift modules importing cxx modules
1740+
do {
1741+
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
1742+
XCTAssertMatch(
1743+
swiftInteropLib,
1744+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
1745+
)
1746+
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
1747+
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1748+
}
1749+
1750+
// Assert symbolgraph-extract args for cxx modules
1751+
do {
1752+
let swiftInteropLib = try result.target(for: "swiftInteropLib").swiftTarget().compileArguments()
1753+
XCTAssertMatch(
1754+
swiftInteropLib,
1755+
[.anySequence, "-cxx-interoperability-mode=default", "-Xcc", "-std=c++1z", .anySequence]
1756+
)
1757+
let swiftLib = try result.target(for: "swiftLib").swiftTarget().compileArguments()
1758+
XCTAssertNoMatch(swiftLib, [.anySequence, "-Xcc", "-std=c++1z", .anySequence])
1759+
}
17371760
}
17381761

17391762
func testSwiftCMixed() throws {
@@ -1910,6 +1933,81 @@ final class BuildPlanTests: XCTestCase {
19101933
])
19111934
}
19121935

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

0 commit comments

Comments
 (0)