Skip to content

[Explicit Module Builds] Add 'ClangImporter' setting for explicitly-built modules #65869

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 1 commit into from
May 15, 2023

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented May 11, 2023

This will mean that -disable-implicit-swift-modules when specified to swift-frontend also automatically implies two things:

  1. Clang modules must also be explicit, and the importer's clang instance will get -fno-implicit-modules and -fno-implicit-module-maps.
  2. The importer's clang instance will no longer get a -fmodules-cache-path=, since it is not needed in explicit builds.

@artemcm artemcm requested a review from nkcsgexi May 11, 2023 20:24
@artemcm
Copy link
Contributor Author

artemcm commented May 11, 2023

We do not really want to support Swift explicit modules combined with Clang implicit modules, so these things should always be in-tandem. This will then allow us to no longer require compiler clients to specify clang flags explicitly:
https://github.com/apple/swift-driver/blob/0dfb2b7dd08604ae15e64489052b110599d42d3f/Sources/SwiftDriver/ExplicitModuleBuilds/ExplicitDependencyBuildPlanner.swift#L366

@artemcm
Copy link
Contributor Author

artemcm commented May 11, 2023

@swift-ci test

@artemcm artemcm force-pushed the ClangImporterExplicitBuildOptions branch from 7dfe832 to 0b54845 Compare May 12, 2023 21:32
@artemcm
Copy link
Contributor Author

artemcm commented May 12, 2023

@swift-ci test

@@ -1460,6 +1460,9 @@ static bool ParseClangImporterArgs(ClangImporterOptions &Opts,
Opts.PCHDisableValidation |= Args.hasArg(OPT_pch_disable_validation);
}

if (FrontendOpts.DisableImplicitModules)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an important change, it essentially prohibits implicit loading of Clang modules when Swift explicit module loading is enabled. It is not clear why such a combination would be useful, and it is not something we intend to validate, so may as well make it not possible.

@artemcm
Copy link
Contributor Author

artemcm commented May 15, 2023

@swift-ci test Windows platform

…uilt modules

This will mean that '-disable-implicit-swift-modules' also automatically implies two things:
1. Clang modules must also be explicit, and the importer's clang instance will get '-fno-implicit-modules' and '-fno-implicit-module-maps'
2. The importer's clang instance will no longer get a '-fmodules-cache-path=', since it is not needed in explicit builds
@artemcm artemcm force-pushed the ClangImporterExplicitBuildOptions branch from 0b54845 to e2ea6cb Compare May 15, 2023 16:45
@artemcm
Copy link
Contributor Author

artemcm commented May 15, 2023

@swift-ci test

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.

Very cool!

@artemcm artemcm merged commit 61ea49c into swiftlang:main May 15, 2023
@artemcm artemcm deleted the ClangImporterExplicitBuildOptions branch May 15, 2023 22:46
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