Pass IPI Clang module names to the Swift compiler#1404
Conversation
|
@swift-ci please test |
|
@swift-ci please test Windows |
|
@swift-ci please test |
|
@swift-ci please test macOS |
|
@swift-ci please test Linux |
|
@swift-ci please test |
|
@swift-ci please test |
sepy97
requested review from
aciidgh,
jakepetroules,
mhrawdon,
neonichu and
owenv
as code owners
| /// For each consumer target, the names of Clang modules in its transitive deps that should | ||
| /// be treated as IPI — a dep with `SKIP_INSTALL=YES` and a `MODULEMAP_FILE` resolving to a | ||
| /// path under any `SRCROOT` in the closure. | ||
| package private(set) var ipiClangModuleNamesByTarget: [ConfiguredTarget: [String]] |
There was a problem hiding this comment.
Instead of exposing this via settings and adding it to all the cache keys, can we access it from the task producer directly via the globalProductPlan?
| }() | ||
| guard skipInstall || modulemapUnderSRCROOT else { continue } | ||
| let name = scope.evaluate(BuiltinMacros.PRODUCT_MODULE_NAME) | ||
| if !name.isEmpty { |
There was a problem hiding this comment.
It might be an ok heuristic to use, but if MODULEMAP_FILE or MODULEMAP_FILE_CONTENTS is set the module(s) defined by the modulemap might not actually be named $(PRODUCT_MODULE_NAME)
There was a problem hiding this comment.
Great catch! This change should also support manually written modulemaps. I was thinking of using knownClangModuleNames via computeModuleInfo(), but this case is there under FIXME...
| // Test -ipi-clang-module emission for a Clang IPI (SKIP_INSTALL=YES + MODULEMAP_FILE under SRCROOT). | ||
| @Test(.requireSDKs(.macOS)) | ||
| func ipiClangModuleBasic() async throws { | ||
| if try await !swiftFeatures.has(.ipiClangModule) { |
There was a problem hiding this comment.
you can use requireSwiftFeatures(.foo) to skip the test entirely instead
|
@swift-ci please test |
| let allTargets = [configuredTarget] + deps | ||
| // Collect the set of project-internal roots for this consumer. | ||
| var srcroots: Set<Path> = [] | ||
| for target in allTargets { |
There was a problem hiding this comment.
This will repeat the module name calculation for a target every time it appears in the transitive closure of another. Can we have a top level loop over all the targets which first computes the module name, then do the transitive closure computation later?
There was a problem hiding this comment.
Makes sense, thanks for catching it!
|
@swift-ci please test |
| } | ||
| var names: Set<String> = [] | ||
| for target in allTargets { | ||
| guard let info = ipiInfoByTarget[target], info.producesClangModule else { continue } |
There was a problem hiding this comment.
This inner loop is still pretty expensive, can we build the sets up incrementally while doing a postorder DFS of the graph?
sepy97
force-pushed
the
ipi-clang-modules
branch
from
2b8779e to
a1418a2
Compare
sepy97
requested review from
cmcgee1024,
daveinglis and
rconnell9
as code owners
When a Swift target is compiled, swift-build now computes which Clang modules in its transitive dependency closure should be treated as IPI (project-internal) and passes their names via compiler flags. The SKIP_INSTALL condition covers target-produced modules including auto-generated modulemaps. The SRCROOT condition covers explicit modulemaps committed to a project's source tree regardless of SKIP_INSTALL. rdar://177010683
sepy97
force-pushed
the
ipi-clang-modules
branch
from
a1418a2 to
20ce517
Compare
|
@swift-ci please test |
2 participants
When a Swift target is compiled, swift-build now computes which Clang modules in its transitive dependency closure should be treated as IPI (project-internal) and passes their names via frontend flags.
The SKIP_INSTALL condition covers target-produced modules including auto-generated modulemaps. The SRCROOT condition covers explicit modulemaps committed to a project's source tree regardless of SKIP_INSTALL.
rdar://177010683