Skip to content

[Explicit Module Builds] Restore prior behavior of consuming .h dependencies of binary module dependencies directly, instead of attempting to load their PCH #69109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Oct 10, 2023

  • [Serialization] Serialize source .h path when an explicit .pch bridging header is provided. In explicit module builds, bridging header is passed directly as a .pch input. Loading clients may not be able to directly import this PCH because it was built against mis-matched dependencies with a different context hash. So instead they should directly ingest the .h dependency and build it against their own set of dependencies.
  • [Dependency Scanning] Do now write out bridging header dependencies of binary modules unless in CAS mode. We only record these dependencies in CAS mode, because we require explicit PCH tasks to be produced for imported header of binary module dependencies. In the meantime, in non-CAS mode loading clients will consume the .h files encoded in the .swiftmodules directly. Followup changes to SwiftDriver will enable explicit PCH compilation of such dependencies, but for the time being restore prior behavior for non-CAS explicit module builds.

Resolves rdar://116006619

@artemcm artemcm force-pushed the EBM_BridgingHeader_Fix branch from 632138a to 3e9bfc6 Compare October 10, 2023 23:05
@artemcm
Copy link
Contributor Author

artemcm commented Oct 10, 2023

@swift-ci smoke test

Copy link
Contributor

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small comments. The change LGTM.

@@ -1297,15 +1299,27 @@ void Serializer::writeInputBlock() {
off_t importedHeaderSize = 0;
time_t importedHeaderModTime = 0;
std::string contents;
if (!Options.ImportedHeader.empty()) {
auto importedHeaderPath = Options.ImportedHeader;
// We do not want to serialize the explicitly-specified .pch path if one was
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have test coverage for this? That is what happens when -disable-bridging-pch is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

header_deps_of_binary.swift is testing this very thing. A binary module is being built with a direct .pch file and what is serialized in it is a .h of the source.

-disable-bridging-pch is a driver flag to always feed the .h directly to the TU compilation, instead of ever creating a PCH.

@artemcm artemcm force-pushed the EBM_BridgingHeader_Fix branch from 3e9bfc6 to 48e5d4d Compare October 11, 2023 04:17
@artemcm
Copy link
Contributor Author

artemcm commented Oct 11, 2023

@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented Oct 11, 2023

Failed Tests (2):
  lldb-api :: lang/swift/array_bridged_enum/TestArrayBridgedEnum.py
  lldb-api :: lang/swift/clangimporter/objcmain_conflicting_dylibs_bridging_headers/TestSwiftObjCMainConflictingDylibsBridgingHeader.py

Hmm.

@artemcm artemcm force-pushed the EBM_BridgingHeader_Fix branch from 48e5d4d to 9fa8f5a Compare October 11, 2023 18:00
@artemcm
Copy link
Contributor Author

artemcm commented Oct 11, 2023

@swift-ci test and merge

…idging header is provided

In explicit module builds, bridging header is passed directly as a '.pch' input.

Loading clients may not be able to directly import this PCH because it was built against mis-matched dependencies with a different context hash. So instead they should directly injest the '.h' depndency and build it against their own set of dependencies.
@artemcm artemcm force-pushed the EBM_BridgingHeader_Fix branch from 9fa8f5a to d2ad931 Compare October 11, 2023 20:15
@artemcm
Copy link
Contributor Author

artemcm commented Oct 11, 2023

@swift-ci smoke test and merge

artemcm added a commit to swiftlang/swift-driver that referenced this pull request Oct 12, 2023
…ng dependency PCH

As of swiftlang/swift#69109 we no longer propagate the PCH to dependencies in non-cached builds.
For cached builds, this functionality is tested in 'testCacheBuildEndToEndWithBinaryHeaderDeps'.
artemcm added a commit to swiftlang/swift-driver that referenced this pull request Oct 12, 2023
…ng dependency PCH

As of swiftlang/swift#69109 we no longer propagate the PCH to dependencies in non-cached builds.
For cached builds, this functionality is tested in 'testCacheBuildEndToEndWithBinaryHeaderDeps'.
…f binary modules unless in CAS mode

We only record these dependencies in CAS mode, because we require explicit PCH tasks to be produced for imported header of binary module dependencies. In the meantime, in non-CAS mode loading clients will consume the `.h` files encoded in the `.swiftmodules` directly.

Followup changes to SwiftDriver will enable explicit PCH compilation of such dependenceis, but for the time being restore prior behavior for non-CAS explicit module builds.

Resolves rdar://116006619
@artemcm artemcm force-pushed the EBM_BridgingHeader_Fix branch from d2ad931 to 75ee48c Compare October 12, 2023 13:24
@artemcm
Copy link
Contributor Author

artemcm commented Oct 12, 2023

artemcm added a commit to swiftlang/swift-driver that referenced this pull request Oct 12, 2023
…ng dependency PCH

As of swiftlang/swift#69109 we no longer propagate the PCH to dependencies in non-cached builds.
For cached builds, this functionality is tested in 'testCacheBuildEndToEndWithBinaryHeaderDeps'.
artemcm added a commit to swiftlang/swift-driver that referenced this pull request Oct 13, 2023
…ng dependency PCH

As of swiftlang/swift#69109 we no longer propagate the PCH to dependencies in non-cached builds.
For cached builds, this functionality is tested in 'testCacheBuildEndToEndWithBinaryHeaderDeps'.
@artemcm artemcm merged commit 6a699b6 into swiftlang:main Oct 13, 2023
@artemcm artemcm deleted the EBM_BridgingHeader_Fix branch October 13, 2023 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants