[cxx-interop] Make test discovery compatible with C++ interop #7165
Conversation
egorzhdan
requested review from
neonichu,
tomerd and
MaxDesiatov
as code owners
| // discovery target must enable C++ interop as well | ||
| switch testTargetRole { | ||
| case .discovery: | ||
| for dependency in try self.target.recursiveTargetDependencies() { |
There was a problem hiding this comment.
This is somewhat ugly, but it should go away once we have a better story for mixing C-interop and C++-interop in a single project.
|
@swift-ci please test |
| for dependency in try self.target.recursiveTargetDependencies() { | ||
| let dependencyScope = self.buildParameters.createScope(for: dependency) | ||
| let dependencySwiftFlags = dependencyScope.evaluate(.OTHER_SWIFT_FLAGS) | ||
| if let interopModeFlag = dependencySwiftFlags.first(where: { $0.hasPrefix("-cxx-interoperability-mode=") }) { |
There was a problem hiding this comment.
are we doing these kind of checks elsewhere in the code? should it be encapsulated behind something like dependencySwiftFlags.cxxInteropMode returning an optional?
There was a problem hiding this comment.
This isn't happening elsewhere in the code, no. We basically don't want to implicitly enable C++ interop unless is it really unavoidable, like it is in the case of a test discovery target, because parsing Objective-C headers in Objective-C++ language mode sometimes changes semantics/ABI.
I only see one other usage of extraSwiftFlags.first(where: ...) in UserToolchain.swift and one other usage of .evaluate(.OTHER_SWIFT_FLAGS). I can extract a common method for either, let me know if you think it's worth it.
There was a problem hiding this comment.
Generalizing extraSwiftFlags.first(where:) (preferably its replacement that isn't stringly-typed) makes sense to me, the other seems fine as is.
There was a problem hiding this comment.
Hmm, I just realized that the other usage of extraSwiftFlags.first operates on [String], not on flags/scopes. So I don't really see a way to simplify this.
MaxDesiatov
added
build system
|
You also need to pass through the C++ standard as well, when C++ interop is enabled. |
egorzhdan
force-pushed
the
egorzhdan/cxx-interop-test-discovery
branch
from
e1b4410 to
cb66df8
Compare
|
@MaxDesiatov is there a more suitable place for this logic, perhaps |
|
@hyp thanks, done. We should now be passing |
|
@swift-ci please test |
|
|
If a Swift module is built with C++ interop enabled, all of its dependencies must also be built with C++ interop enabled. Swift packages that use test discovery get a synthesized "PackageDiscoveredTests" target which is built by SwiftPM when someone runs `swift test` command. This module is currently always built without C++ interop, which is causing issues for projects that rely on it. This patch makes sure that the `-cxx-interoperability-mode=` command line flag gets propagated to the test discovery target. rdar://117078320 / resolves #6990
egorzhdan
force-pushed
the
egorzhdan/cxx-interop-test-discovery
branch
from
cb66df8 to
6a2278f
Compare
|
@swift-ci please test |
|
@swift-ci please test Windows |
### Motivation: If a Swift module is built with C++ interop enabled, all of its dependencies must also be built with C++ interop enabled. Swift packages that use test discovery get a synthesized "PackageDiscoveredTests" target which is built by SwiftPM when someone runs `swift test` command. This module is currently always built without C++ interop, which is causing issues for projects that rely on it. ### Modifications: This patch makes sure that the `-cxx-interoperability-mode=` command line flag gets propagated to the test discovery target. ### Result: Packages that use both C++ interop and test discovery can now be built successfully by SwiftPM. rdar://117078320 / resolves #6990 (cherry picked from commit d2eb0ca)
### Motivation: If a Swift module is built with C++ interop enabled, all of its dependencies must also be built with C++ interop enabled. Swift packages that use test discovery get a synthesized "PackageDiscoveredTests" target which is built by SwiftPM when someone runs `swift test` command. This module is currently always built without C++ interop, which is causing issues for projects that rely on it. ### Modifications: This patch makes sure that the `-cxx-interoperability-mode=` command line flag gets propagated to the test discovery target. ### Result: Packages that use both C++ interop and test discovery can now be built successfully by SwiftPM. rdar://117078320 / resolves #6990 Original PR: #7165 (cherry picked from commit d2eb0ca)
Motivation:
If a Swift module is built with C++ interop enabled, all of its dependencies must also be built with C++ interop enabled.
Swift packages that use test discovery get a synthesized "PackageDiscoveredTests" target which is built by SwiftPM when someone runs
swift testcommand. This module is currently always built without C++ interop, which is causing issues for projects that rely on it.Modifications:
This patch makes sure that the
-cxx-interoperability-mode=command line flag gets propagated to the test discovery target.Result:
Packages that use both C++ interop and test discovery can now be built successfully by SwiftPM.
rdar://117078320 / resolves #6990