Skip to content

Front-end: add a new module loader that loads explicitly built Swift modules #32170

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
Jun 4, 2020

Conversation

nkcsgexi
Copy link
Contributor

@nkcsgexi nkcsgexi commented Jun 3, 2020

To support -disable-implicit-swift-modules, the explicitly built modules
are passed down as compiler arguments. We need this new module loader to
handle these modules.

This patch also stops ModuleInterfaceLoader from building module from interface
when -disable-implicit-swift-modules is set.

@nkcsgexi nkcsgexi requested review from DougGregor and artemcm June 3, 2020 23:04
@nkcsgexi nkcsgexi force-pushed the explicit-module-loader branch from e663888 to be687b1 Compare June 4, 2020 01:36
@nkcsgexi
Copy link
Contributor Author

nkcsgexi commented Jun 4, 2020

@swift-ci please smoke test

…modules

To support -disable-implicit-swift-modules, the explicitly built modules
are passed down as compiler arguments. We need this new module loader to
handle these modules.

This patch also stops ModuleInterfaceLoader from building module from interface
when -disable-implicit-swift-modules is set.
@nkcsgexi nkcsgexi force-pushed the explicit-module-loader branch from be687b1 to bd782be Compare June 4, 2020 01:59
@nkcsgexi
Copy link
Contributor Author

nkcsgexi commented Jun 4, 2020

@swift-ci please smoke test

1 similar comment
@nkcsgexi
Copy link
Contributor Author

nkcsgexi commented Jun 4, 2020

@swift-ci please smoke test

@nkcsgexi nkcsgexi merged commit 96a5c81 into swiftlang:master Jun 4, 2020
@@ -2190,6 +2190,30 @@ TypeDecl *ModuleFile::lookupLocalType(StringRef MangledName) {
return cast<TypeDecl>(getDecl(*iter));
}

std::unique_ptr<llvm::MemoryBuffer>
ModuleFile::getModuleName(ASTContext &Ctx, StringRef modulePath,
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to load the module here? Can we instead provide more information on the command line, e.g., a specific mapping from module name to the module file the contains it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we could do that to avoid loading modules eagerly. How about we accept a JSON file from the command line to include module name, module path, swiftdoc path and swiftsourceinfo path?

@@ -450,6 +450,16 @@ bool CompilerInstance::setUpModuleLoaders() {
return true;
}

// If implicit modules are disabled, we need to install an explicit module
// loader.
if (Invocation.getFrontendOptions().DisableImplicitModules) {
Copy link
Member

Choose a reason for hiding this comment

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

How about modeling DisableImplicitModules as a ModuleLoadingMode? It fits in with the scheme established there.

}
// We found an explicit module matches the given name, give the buffer
// back to the caller side.
*ModuleBuffer = std::move(it->getValue().moduleBuffer);
Copy link
Member

Choose a reason for hiding this comment

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

We also need to deal with the .swiftdoc and .swiftsourceinfo as well, right?

Invocation.getSearchPathOptions().ExplicitSwiftModules,
IgnoreSourceInfoFile);
Context->addModuleLoader(std::move(ESML));
}
if (MLM != ModuleLoadingMode::OnlySerialized) {
Copy link
Member

Choose a reason for hiding this comment

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

When ExplicitSwiftModuleLoader is added, it seems like we should not add ModuleInterfaceLoader or SerializedModuleLoader at all. Then we don't need checks for DisableImplicitModules anywhere.

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