Skip to content

[Dependency Scanning] By-default do not specify '-fmodule-map-file' inputs on Swift interface dependency compilation jobs #72599

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
Mar 29, 2024

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Mar 26, 2024

Swift interfaces will only ever query module inputs by-name, so parsing modulemap files to resolve their headers is not necessary. The exception to this is textual header ingestion directly into the Swift compiler (ClangImporter). Because then the compiler may need to resolve header includes to the corresponding module (.pcm) inputs. This requires explicitly-specified module maps, until we switch to eager module loading.

This occurs when there is a a dependency on a binary Swift module, which was built with a bridging header, and this bridging header was serialized into the .swiftmodule and must be ingested by all clients. In this case, we must ensure that all clients of said binary module, direct and transitive, are passed -fmodule-map-file= flags corresponding to dependencies of said header.

…ift interface dependency compilation jobs

Swift interfaces will only ever query module inputs by-name, so parsing modulemap files to resolve their headers is not necessary.
@artemcm
Copy link
Contributor Author

artemcm commented Mar 26, 2024

@cachemeifyoucan
Copy link
Contributor

This only remove the option from building interface. Do you need to drop the option from ExplicitSwiftModuleLoader as well so the compilation for the main module also doesn't use this option?

@artemcm
Copy link
Contributor Author

artemcm commented Mar 27, 2024

This only remove the option from building interface. Do you need to drop the option from ExplicitSwiftModuleLoader as well so the compilation for the main module also doesn't use this option?

Unfortunately, not yet. Furthermore, in its current state this change is not safe.
The edge case here is textual header ingestion directly into the Swift compiler (ClangImporter). Because then the compiler may need to resolve header includes to the corresponding module (.pcm) inputs. This requires explicitly-specified module maps, until we switch to eager module loading.

There are two possible cases of this:

  1. We are building a source module which includes a textual bridging header. In this case, we can drop all -fmodule-map-file= flags except those that correspond to module dependencies of the bridging header itself.
  2. We have a dependency on a binary Swift module, which was built with a bridging header, and this bridging header was serialized into the .swiftmodule and must be ingested by all clients. In this case, we must ensure that all clients of said binary module, direct and transitive, are passed -fmodule-map-file= flags corresponding to dependencies of said header.

(2) I will implement shortly in this PR.
(1) will require coordination with the driver, since the driver computes the set of dependencies we specify to main module compilation, it will then need to specify which of the dependencies require inclusion of the module map on the ClangImporter invocation.

…encies of bridging headers, direct or transitive

On all '.swiftinterface' command-line recipes, ensure we specify '-fmodule-map-file=' argument for each module which is a dependency of a header input of all swift binary module dependnecies, direct and transitive
@artemcm
Copy link
Contributor Author

artemcm commented Mar 27, 2024

@swift-ci test

@artemcm artemcm changed the title [Dependency Scanning] Do not specify '-fmodule-map-file' inputs on Swift interface dependency compilation jobs [Dependency Scanning] By-default do not specify '-fmodule-map-file' inputs on Swift interface dependency compilation jobs Mar 27, 2024
@cachemeifyoucan
Copy link
Contributor

I am not sure about why bridging header needs a module map? Should the importer already sets up all the explicit modules including the ones needed by bridging header?

@artemcm
Copy link
Contributor Author

artemcm commented Mar 28, 2024

I am not sure about why bridging header needs a module map? Should the importer already sets up all the explicit modules including the ones needed by bridging header?

Bridging headers require a module map because that's what aids header include resolution.
With lazy module loading, .modulemap parsing which happens when instantiating Clang is responsible for associating headers with modules. Then upon encountering a header include inside the bridging header the compiler knows which module corresponds to said header and is then able to load explicitly-provided PCM for that module.

Once we are able to switch to eager module loading, this will no longer be necessary because we will instead be relying on module maps serialized into PCMs.

Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

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

Thank you!

@artemcm artemcm merged commit 4a2b178 into swiftlang:main Mar 29, 2024
@artemcm artemcm deleted the NoModuleMapsOnSwiftDepsExplicit branch March 29, 2024 16:02
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.

3 participants