[clang] 8d2f091 - [clang] Extract `CompilerInstance` creation out of `compileModuleImpl()` (#134887)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 11 09:39:25 PDT 2025
Author: Jan Svoboda
Date: 2025-04-11T09:39:22-07:00
New Revision: 8d2f0911ce8dddb37d064b3065b3be71e3233c2c
URL: https://github.com/llvm/llvm-project/commit/8d2f0911ce8dddb37d064b3065b3be71e3233c2c
DIFF: https://github.com/llvm/llvm-project/commit/8d2f0911ce8dddb37d064b3065b3be71e3233c2c.diff
LOG: [clang] Extract `CompilerInstance` creation out of `compileModuleImpl()` (#134887)
This PR extracts the creation of `CompilerInstance` for compiling an
implicitly-discovered module out of `compileModuleImpl()` into its own
separate function and passes it into `compileModuleImpl()` from the
outside. This makes the instance creation logic reusable (useful for my
experiments) and also simplifies the API, removing the `PreBuildStep`
and `PostBuildStep` hooks from `compileModuleImpl()`.
Added:
Modified:
clang/lib/Frontend/CompilerInstance.cpp
Removed:
################################################################################
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 9cab17ae70eeb..55265af90f917 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1150,29 +1150,16 @@ static Language getLanguageFromOptions(const LangOptions &LangOpts) {
return LangOpts.CPlusPlus ? Language::CXX : Language::C;
}
-/// Compile a module file for the given module, using the options
-/// provided by the importing compiler instance. Returns true if the module
-/// was built without errors.
-static bool
-compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
- StringRef ModuleName, FrontendInputFile Input,
- StringRef OriginalModuleMapFile, StringRef ModuleFileName,
- llvm::function_ref<void(CompilerInstance &)> PreBuildStep =
- [](CompilerInstance &) {},
- llvm::function_ref<void(CompilerInstance &)> PostBuildStep =
- [](CompilerInstance &) {}) {
- llvm::TimeTraceScope TimeScope("Module Compile", ModuleName);
-
- // Never compile a module that's already finalized - this would cause the
- // existing module to be freed, causing crashes if it is later referenced
- if (ImportingInstance.getModuleCache().getInMemoryModuleCache().isPCMFinal(
- ModuleFileName)) {
- ImportingInstance.getDiagnostics().Report(
- ImportLoc, diag::err_module_rebuild_finalized)
- << ModuleName;
- return false;
- }
-
+/// Creates a \c CompilerInstance for compiling a module.
+///
+/// This expects a properly initialized \c FrontendInputFile.
+static std::unique_ptr<CompilerInstance>
+createCompilerInstanceForModuleCompileImpl(CompilerInstance &ImportingInstance,
+ SourceLocation ImportLoc,
+ StringRef ModuleName,
+ FrontendInputFile Input,
+ StringRef OriginalModuleMapFile,
+ StringRef ModuleFileName) {
// Construct a compiler invocation for creating this module.
auto Invocation =
std::make_shared<CompilerInvocation>(ImportingInstance.getInvocation());
@@ -1226,8 +1213,11 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
// module. Since we're sharing an in-memory module cache,
// CompilerInstance::CompilerInstance is responsible for finalizing the
// buffers to prevent use-after-frees.
- CompilerInstance Instance(ImportingInstance.getPCHContainerOperations(),
- &ImportingInstance.getModuleCache());
+ auto InstancePtr = std::make_unique<CompilerInstance>(
+ ImportingInstance.getPCHContainerOperations(),
+ &ImportingInstance.getModuleCache());
+ auto &Instance = *InstancePtr;
+
auto &Inv = *Invocation;
Instance.setInvocation(std::move(Invocation));
@@ -1267,12 +1257,32 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
Instance.setModuleDepCollector(ImportingInstance.getModuleDepCollector());
Inv.getDependencyOutputOpts() = DependencyOutputOptions();
+ return InstancePtr;
+}
+
+/// Compile a module file for the given module, using the options
+/// provided by the importing compiler instance. Returns true if the module
+/// was built without errors.
+static bool compileModule(CompilerInstance &ImportingInstance,
+ SourceLocation ImportLoc, StringRef ModuleName,
+ StringRef ModuleFileName,
+ CompilerInstance &Instance) {
+ llvm::TimeTraceScope TimeScope("Module Compile", ModuleName);
+
+ // Never compile a module that's already finalized - this would cause the
+ // existing module to be freed, causing crashes if it is later referenced
+ if (ImportingInstance.getModuleCache().getInMemoryModuleCache().isPCMFinal(
+ ModuleFileName)) {
+ ImportingInstance.getDiagnostics().Report(
+ ImportLoc, diag::err_module_rebuild_finalized)
+ << ModuleName;
+ return false;
+ }
+
ImportingInstance.getDiagnostics().Report(ImportLoc,
diag::remark_module_build)
<< ModuleName << ModuleFileName;
- PreBuildStep(Instance);
-
// Execute the action to actually build the module in-place. Use a separate
// thread so that we get a stack large enough.
bool Crashed = !llvm::CrashRecoveryContext().RunSafelyOnThread(
@@ -1282,14 +1292,12 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
},
DesiredStackSize);
- PostBuildStep(Instance);
-
ImportingInstance.getDiagnostics().Report(ImportLoc,
diag::remark_module_build_done)
<< ModuleName;
// Propagate the statistics to the parent FileManager.
- if (!FrontendOpts.ModulesShareFileManager)
+ if (!ImportingInstance.getFrontendOpts().ModulesShareFileManager)
ImportingInstance.getFileManager().AddStats(Instance.getFileManager());
if (Crashed) {
@@ -1302,6 +1310,12 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
Instance.clearOutputFiles(/*EraseFiles=*/true);
}
+ // We've rebuilt a module. If we're allowed to generate or update the global
+ // module index, record that fact in the importing compiler instance.
+ if (ImportingInstance.getFrontendOpts().GenerateGlobalModuleIndex) {
+ ImportingInstance.setBuildGlobalModuleIndex(true);
+ }
+
// If \p AllowPCMWithCompilerErrors is set return 'success' even if errors
// occurred.
return !Instance.getDiagnostics().hasErrorOccurred() ||
@@ -1321,20 +1335,24 @@ static OptionalFileEntryRef getPublicModuleMap(FileEntryRef File,
return FileMgr.getOptionalFileRef(PublicFilename);
}
-/// Compile a module file for the given module in a separate compiler instance,
-/// using the options provided by the importing compiler instance. Returns true
-/// if the module was built without errors.
-static bool compileModule(CompilerInstance &ImportingInstance,
- SourceLocation ImportLoc, Module *Module,
- StringRef ModuleFileName) {
+/// Creates a \c CompilerInstance for compiling a module.
+///
+/// This takes care of creating appropriate \c FrontendInputFile for
+/// public/private frameworks, inferred modules and such.
+static std::unique_ptr<CompilerInstance>
+createCompilerInstanceForModuleCompile(CompilerInstance &ImportingInstance,
+ SourceLocation ImportLoc, Module *Module,
+ StringRef ModuleFileName) {
+ StringRef ModuleName = Module->getTopLevelModuleName();
+
InputKind IK(getLanguageFromOptions(ImportingInstance.getLangOpts()),
InputKind::ModuleMap);
// Get or create the module map that we'll use to build this module.
- ModuleMap &ModMap
- = ImportingInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap();
+ ModuleMap &ModMap =
+ ImportingInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap();
SourceManager &SourceMgr = ImportingInstance.getSourceManager();
- bool Result;
+
if (FileID ModuleMapFID = ModMap.getContainingModuleMapFileID(Module);
ModuleMapFID.isValid()) {
// We want to use the top-level module map. If we don't, the compiling
@@ -1368,44 +1386,36 @@ static bool compileModule(CompilerInstance &ImportingInstance,
bool IsSystem = isSystem(SLoc.getFile().getFileCharacteristic());
// Use the module map where this module resides.
- Result = compileModuleImpl(
- ImportingInstance, ImportLoc, Module->getTopLevelModuleName(),
+ return createCompilerInstanceForModuleCompileImpl(
+ ImportingInstance, ImportLoc, ModuleName,
FrontendInputFile(ModuleMapFilePath, IK, IsSystem),
ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName);
- } else {
- // FIXME: We only need to fake up an input file here as a way of
- // transporting the module's directory to the module map parser. We should
- // be able to do that more directly, and parse from a memory buffer without
- // inventing this file.
- SmallString<128> FakeModuleMapFile(Module->Directory->getName());
- llvm::sys::path::append(FakeModuleMapFile, "__inferred_module.map");
-
- std::string InferredModuleMapContent;
- llvm::raw_string_ostream OS(InferredModuleMapContent);
- Module->print(OS);
-
- Result = compileModuleImpl(
- ImportingInstance, ImportLoc, Module->getTopLevelModuleName(),
- FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem),
- ModMap.getModuleMapFileForUniquing(Module)->getName(),
- ModuleFileName,
- [&](CompilerInstance &Instance) {
- std::unique_ptr<llvm::MemoryBuffer> ModuleMapBuffer =
- llvm::MemoryBuffer::getMemBuffer(InferredModuleMapContent);
- FileEntryRef ModuleMapFile = Instance.getFileManager().getVirtualFileRef(
- FakeModuleMapFile, InferredModuleMapContent.size(), 0);
- Instance.getSourceManager().overrideFileContents(
- ModuleMapFile, std::move(ModuleMapBuffer));
- });
}
- // We've rebuilt a module. If we're allowed to generate or update the global
- // module index, record that fact in the importing compiler instance.
- if (ImportingInstance.getFrontendOpts().GenerateGlobalModuleIndex) {
- ImportingInstance.setBuildGlobalModuleIndex(true);
- }
+ // FIXME: We only need to fake up an input file here as a way of
+ // transporting the module's directory to the module map parser. We should
+ // be able to do that more directly, and parse from a memory buffer without
+ // inventing this file.
+ SmallString<128> FakeModuleMapFile(Module->Directory->getName());
+ llvm::sys::path::append(FakeModuleMapFile, "__inferred_module.map");
+
+ std::string InferredModuleMapContent;
+ llvm::raw_string_ostream OS(InferredModuleMapContent);
+ Module->print(OS);
+
+ auto Instance = createCompilerInstanceForModuleCompileImpl(
+ ImportingInstance, ImportLoc, ModuleName,
+ FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem),
+ ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName);
+
+ std::unique_ptr<llvm::MemoryBuffer> ModuleMapBuffer =
+ llvm::MemoryBuffer::getMemBufferCopy(InferredModuleMapContent);
+ FileEntryRef ModuleMapFile = Instance->getFileManager().getVirtualFileRef(
+ FakeModuleMapFile, InferredModuleMapContent.size(), 0);
+ Instance->getSourceManager().overrideFileContents(ModuleMapFile,
+ std::move(ModuleMapBuffer));
- return Result;
+ return Instance;
}
/// Read the AST right after compiling the module.
@@ -1455,8 +1465,12 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance,
SourceLocation ModuleNameLoc,
Module *Module,
StringRef ModuleFileName) {
- if (!compileModule(ImportingInstance, ModuleNameLoc, Module,
- ModuleFileName)) {
+ auto Instance = createCompilerInstanceForModuleCompile(
+ ImportingInstance, ModuleNameLoc, Module, ModuleFileName);
+
+ if (!compileModule(ImportingInstance, ModuleNameLoc,
+ Module->getTopLevelModuleName(), ModuleFileName,
+ *Instance)) {
ImportingInstance.getDiagnostics().Report(ModuleNameLoc,
diag::err_module_not_built)
<< Module->Name << SourceRange(ImportLoc, ModuleNameLoc);
@@ -2232,25 +2246,26 @@ void CompilerInstance::createModuleFromSource(SourceLocation ImportLoc,
std::string NullTerminatedSource(Source.str());
- auto PreBuildStep = [&](CompilerInstance &Other) {
- // Create a virtual file containing our desired source.
- // FIXME: We shouldn't need to do this.
- FileEntryRef ModuleMapFile = Other.getFileManager().getVirtualFileRef(
- ModuleMapFileName, NullTerminatedSource.size(), 0);
- Other.getSourceManager().overrideFileContents(
- ModuleMapFile, llvm::MemoryBuffer::getMemBuffer(NullTerminatedSource));
+ auto Other = createCompilerInstanceForModuleCompileImpl(
+ *this, ImportLoc, ModuleName, Input, StringRef(), ModuleFileName);
- Other.BuiltModules = std::move(BuiltModules);
- Other.DeleteBuiltModules = false;
- };
+ // Create a virtual file containing our desired source.
+ // FIXME: We shouldn't need to do this.
+ FileEntryRef ModuleMapFile = Other->getFileManager().getVirtualFileRef(
+ ModuleMapFileName, NullTerminatedSource.size(), 0);
+ Other->getSourceManager().overrideFileContents(
+ ModuleMapFile, llvm::MemoryBuffer::getMemBuffer(NullTerminatedSource));
- auto PostBuildStep = [this](CompilerInstance &Other) {
- BuiltModules = std::move(Other.BuiltModules);
- };
+ Other->BuiltModules = std::move(BuiltModules);
+ Other->DeleteBuiltModules = false;
// Build the module, inheriting any modules that we've built locally.
- if (compileModuleImpl(*this, ImportLoc, ModuleName, Input, StringRef(),
- ModuleFileName, PreBuildStep, PostBuildStep)) {
+ bool Success =
+ compileModule(*this, ImportLoc, ModuleName, ModuleFileName, *Other);
+
+ BuiltModules = std::move(Other->BuiltModules);
+
+ if (Success) {
BuiltModules[std::string(ModuleName)] = std::string(ModuleFileName);
llvm::sys::RemoveFileOnSignal(ModuleFileName);
}
More information about the cfe-commits
mailing list