Skip to content

Commit 0cff9be

Browse files
committed
Fix swift-symbolgraph-extract search paths
Fixes and architecture and implementation bug where the include paths for swift symbolgraph extract were being determined inside build plan instead of by the target build description. This bug resulted in the wrong set of paths being determined. The new code moves the path determination logic into the proper target build descriptions and releverages the already computed include paths. The result of this is symbol graphs can be generated for Swift and C targets properly now without duplicate module in the search paths and with correct `-fmodule` usage.
1 parent af9b1b3 commit 0cff9be

File tree

6 files changed

+47
-28
lines changed

6 files changed

+47
-28
lines changed

Sources/Build/BuildDescription/ClangTargetBuildDescription.swift

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,17 @@ public final class ClangTargetBuildDescription {
211211
/// this module.
212212
public func symbolGraphExtractArguments() throws -> [String] {
213213
var args = [String]()
214-
215214
if self.clangTarget.isCXX {
216215
args += ["-cxx-interoperability-mode=default"]
217216
}
218217
if let cxxLanguageStandard = self.clangTarget.cxxLanguageStandard {
219218
args += ["-Xcc", "-std=\(cxxLanguageStandard)"]
220219
}
220+
args += ["-I", self.clangTarget.includeDir.pathString]
221+
args += self.additionalFlags
222+
// Unconditionally use clang modules with swift tools.
223+
args += try self.clangModuleArguments().asSwiftcCCompilerFlags()
224+
args += try self.currentModuleMapFileArguments().asSwiftcCCompilerFlags()
221225
return args
222226
}
223227

@@ -253,7 +257,6 @@ public final class ClangTargetBuildDescription {
253257
args += self.buildParameters.indexStoreArguments(for: target)
254258
}
255259

256-
// Enable Clang module flags, if appropriate.
257260
let triple = self.buildParameters.triple
258261
// Swift is able to use modules on non-Darwin platforms because it injects its own module maps
259262
// via vfs. However, nothing does that for C based compilation, and so non-Darwin platforms can't
@@ -263,7 +266,7 @@ public final class ClangTargetBuildDescription {
263266
// clang modules aren't fully supported in C++ mode in the current Darwin SDKs.
264267
let enableModules = triple.isDarwin() && !isCXX
265268
if enableModules {
266-
args += ["-fmodules", "-fmodule-name=" + target.c99name]
269+
args += try self.clangModuleArguments()
267270
}
268271

269272
// Only add the build path to the framework search path if there are binary frameworks to link against.
@@ -273,9 +276,7 @@ public final class ClangTargetBuildDescription {
273276

274277
args += ["-I", clangTarget.includeDir.pathString]
275278
args += additionalFlags
276-
if enableModules {
277-
args += try moduleCacheArgs
278-
}
279+
279280
args += buildParameters.sanitizers.compileCFlags()
280281

281282
// Add arguments from declared build settings.
@@ -433,11 +434,22 @@ public final class ClangTargetBuildDescription {
433434
return compilationConditions
434435
}
435436

436-
/// Module cache arguments.
437-
private var moduleCacheArgs: [String] {
438-
get throws {
439-
try ["-fmodules-cache-path=\(buildParameters.moduleCache.pathString)"]
437+
/// Enable Clang module flags.
438+
private func clangModuleArguments() throws -> [String] {
439+
let cachePath = try self.buildParameters.moduleCache.pathString
440+
return [
441+
"-fmodules",
442+
"-fmodule-name=\(self.target.c99name)",
443+
"-fmodules-cache-path=\(cachePath)",
444+
]
445+
}
446+
447+
private func currentModuleMapFileArguments() throws -> [String] {
448+
// Pass the path to the current module's module map if present.
449+
if let moduleMap = self.moduleMap {
450+
return ["-fmodule-map-file=\(moduleMap.pathString)"]
440451
}
452+
return []
441453
}
442454

443455
/// Generate the resource bundle accessor, if appropriate.

Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,7 @@ public final class SwiftTargetBuildDescription {
633633
var args = [String]()
634634
args += try self.cxxInteroperabilityModeArguments(
635635
propagateFromCurrentModuleOtherSwiftFlags: true)
636+
args += try self.createAPIToolCommonArgs(includeLibrarySearchPaths: true)
636637
return args
637638
}
638639

Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,8 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
648648
}
649649
}
650650

651+
/// Determines the arguments needed to run `swift-symbolgraph-extract` for
652+
/// a particular module.
651653
public func symbolGraphExtractArguments(for module: ResolvedModule) throws -> [String] {
652654
guard let description = self.targetMap[module.id] else {
653655
throw InternalError("Expected description for module \(module)")

Sources/Commands/Utilities/SymbolGraphExtract.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ public struct SymbolGraphExtract {
7070
// FIXME: everything here should be in symbolGraphExtractArguments
7171
commandLine += ["-module-name", module.c99name]
7272
commandLine += try buildParameters.tripleArgs(for: module)
73-
commandLine += try buildPlan.createAPIToolCommonArgs(includeLibrarySearchPaths: true)
7473
commandLine += ["-module-cache-path", try buildParameters.moduleCache.pathString]
7574
if verboseOutput {
7675
commandLine += ["-v"]

Sources/SPMBuildCore/BuildSystem/BuildSystem.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ public protocol BuildPlan {
9191
func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String]
9292
func createREPLArguments() throws -> [String]
9393

94+
/// Determines the arguments needed to run `swift-symbolgraph-extract` for
95+
/// a particular module.
9496
func symbolGraphExtractArguments(for module: ResolvedModule) throws -> [String]
9597
}
9698

Tests/BuildTests/BuildPlanTests.swift

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,13 +1410,13 @@ final class BuildPlanTests: XCTestCase {
14101410
args += ["-O0", "-DSWIFT_PACKAGE=1", "-DDEBUG=1"]
14111411
args += ["-fblocks"]
14121412
#if os(macOS) // FIXME(5473) - support modules on non-Apple platforms
1413-
args += ["-fmodules", "-fmodule-name=extlib"]
1413+
args += [
1414+
"-fmodules",
1415+
"-fmodule-name=extlib",
1416+
"-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))",
1417+
]
14141418
#endif
14151419
args += ["-I", ExtPkg.appending(components: "Sources", "extlib", "include").pathString]
1416-
#if os(macOS) // FIXME(5473) - support modules on non-Apple platforms
1417-
args += ["-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))"]
1418-
#endif
1419-
14201420
args += [hostTriple.isWindows() ? "-gdwarf" : "-g"]
14211421

14221422
if hostTriple.isLinux() {
@@ -1439,7 +1439,11 @@ final class BuildPlanTests: XCTestCase {
14391439
args += ["-O0", "-DSWIFT_PACKAGE=1", "-DDEBUG=1"]
14401440
args += ["-fblocks"]
14411441
#if os(macOS) // FIXME(5473) - support modules on non-Apple platforms
1442-
args += ["-fmodules", "-fmodule-name=exe"]
1442+
args += [
1443+
"-fmodules",
1444+
"-fmodule-name=exe",
1445+
"-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))",
1446+
]
14431447
#endif
14441448
args += [
14451449
"-I", Pkg.appending(components: "Sources", "exe", "include").pathString,
@@ -1448,9 +1452,6 @@ final class BuildPlanTests: XCTestCase {
14481452
"-I", ExtPkg.appending(components: "Sources", "extlib", "include").pathString,
14491453
"-fmodule-map-file=\(buildPath.appending(components: "extlib.build", "module.modulemap"))",
14501454
]
1451-
#if os(macOS) // FIXME(5473) - support modules on non-Apple platforms
1452-
args += ["-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))"]
1453-
#endif
14541455
args += [hostTriple.isWindows() ? "-gdwarf" : "-g"]
14551456

14561457
if hostTriple.isLinux() {
@@ -1796,12 +1797,13 @@ final class BuildPlanTests: XCTestCase {
17961797
args += ["-O0", "-DSWIFT_PACKAGE=1", "-DDEBUG=1"]
17971798
args += ["-fblocks"]
17981799
#if os(macOS) // FIXME(5473) - support modules on non-Apple platforms
1799-
args += ["-fmodules", "-fmodule-name=lib"]
1800+
args += [
1801+
"-fmodules",
1802+
"-fmodule-name=lib",
1803+
"-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))",
1804+
]
18001805
#endif
18011806
args += ["-I", Pkg.appending(components: "Sources", "lib", "include").pathString]
1802-
#if os(macOS) // FIXME(5473) - support modules on non-Apple platforms
1803-
args += ["-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))"]
1804-
#endif
18051807
args += [hostTriple.isWindows() ? "-gdwarf" : "-g"]
18061808

18071809
if hostTriple.isLinux() {
@@ -3035,12 +3037,13 @@ final class BuildPlanTests: XCTestCase {
30353037
expectedExeBasicArgs += ["-target", defaultTargetTriple]
30363038
expectedExeBasicArgs += ["-O0", "-DSWIFT_PACKAGE=1", "-DDEBUG=1", "-fblocks"]
30373039
#if os(macOS) // FIXME(5473) - support modules on non-Apple platforms
3038-
expectedExeBasicArgs += ["-fmodules", "-fmodule-name=exe"]
3040+
expectedExeBasicArgs += [
3041+
"-fmodules",
3042+
"-fmodule-name=exe",
3043+
"-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))"
3044+
]
30393045
#endif
30403046
expectedExeBasicArgs += ["-I", Pkg.appending(components: "Sources", "exe", "include").pathString]
3041-
#if os(macOS) // FIXME(5473) - support modules on non-Apple platforms
3042-
expectedExeBasicArgs += ["-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))"]
3043-
#endif
30443047

30453048
expectedExeBasicArgs += [triple.isWindows() ? "-gdwarf" : "-g"]
30463049

0 commit comments

Comments
 (0)