https://github.com/jansvoboda11 updated
https://github.com/llvm/llvm-project/pull/135473
>From b58c683e4bd51558e72d1cb6ced88cb6fc372048 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Fri, 11 Apr 2025 10:13:49 -0700
Subject: [PATCH 1/4] [clang][frontend] Promote `cloneForModuleCompileImpl` to
`CompilerInstance` member
---
.../include/clang/Frontend/CompilerInstance.h | 7 +++
clang/lib/Frontend/CompilerInstance.cpp | 56 ++++++++-----------
2 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/clang/include/clang/Frontend/CompilerInstance.h
b/clang/include/clang/Frontend/CompilerInstance.h
index 66f56932c51a3..bde1b1f36759e 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -870,6 +870,13 @@ class CompilerInstance : public ModuleLoader {
SourceLocation ModuleNameLoc,
bool IsInclusionDirective);
+ /// Creates a \c CompilerInstance for compiling a module.
+ ///
+ /// This expects a properly initialized \c FrontendInputFile.
+ std::unique_ptr<CompilerInstance> cloneForModuleCompileImpl(
+ SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input,
+ StringRef OriginalModuleMapFile, StringRef ModuleFileName);
+
public:
/// Creates a new \c CompilerInstance for compiling a module.
///
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index eb138de9d20cc..97987d568e729 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1150,19 +1150,11 @@ static Language getLanguageFromOptions(const
LangOptions &LangOpts) {
return LangOpts.CPlusPlus ? Language::CXX : Language::C;
}
-/// 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) {
+std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
+ 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());
+ auto Invocation = std::make_shared<CompilerInvocation>(getInvocation());
PreprocessorOptions &PPOpts = Invocation->getPreprocessorOpts();
@@ -1182,7 +1174,7 @@
createCompilerInstanceForModuleCompileImpl(CompilerInstance &ImportingInstance,
// If the original compiler invocation had -fmodule-name, pass it through.
Invocation->getLangOpts().ModuleName =
- ImportingInstance.getInvocation().getLangOpts().ModuleName;
+ getInvocation().getLangOpts().ModuleName;
// Note the name of the module we're building.
Invocation->getLangOpts().CurrentModule = std::string(ModuleName);
@@ -1206,7 +1198,7 @@
createCompilerInstanceForModuleCompileImpl(CompilerInstance &ImportingInstance,
DiagnosticOptions &DiagOpts = Invocation->getDiagnosticOpts();
DiagOpts.VerifyDiagnostics = 0;
- assert(ImportingInstance.getInvocation().getModuleHash() ==
+ assert(getInvocation().getModuleHash() ==
Invocation->getModuleHash() && "Module hash mismatch!");
// Construct a compiler instance that will be used to actually create the
@@ -1214,25 +1206,25 @@
createCompilerInstanceForModuleCompileImpl(CompilerInstance &ImportingInstance,
// CompilerInstance::CompilerInstance is responsible for finalizing the
// buffers to prevent use-after-frees.
auto InstancePtr = std::make_unique<CompilerInstance>(
- ImportingInstance.getPCHContainerOperations(),
- &ImportingInstance.getModuleCache());
+ getPCHContainerOperations(),
+ &getModuleCache());
auto &Instance = *InstancePtr;
auto &Inv = *Invocation;
Instance.setInvocation(std::move(Invocation));
Instance.createDiagnostics(
- ImportingInstance.getVirtualFileSystem(),
- new
ForwardingDiagnosticConsumer(ImportingInstance.getDiagnosticClient()),
+ getVirtualFileSystem(),
+ new ForwardingDiagnosticConsumer(getDiagnosticClient()),
/*ShouldOwnClient=*/true);
if (llvm::is_contained(DiagOpts.SystemHeaderWarningsModules, ModuleName))
Instance.getDiagnostics().setSuppressSystemWarnings(false);
if (FrontendOpts.ModulesShareFileManager) {
- Instance.setFileManager(&ImportingInstance.getFileManager());
+ Instance.setFileManager(&getFileManager());
} else {
- Instance.createFileManager(&ImportingInstance.getVirtualFileSystem());
+ Instance.createFileManager(&getVirtualFileSystem());
}
Instance.createSourceManager(Instance.getFileManager());
SourceManager &SourceMgr = Instance.getSourceManager();
@@ -1240,21 +1232,21 @@
createCompilerInstanceForModuleCompileImpl(CompilerInstance &ImportingInstance,
// Note that this module is part of the module build stack, so that we
// can detect cycles in the module graph.
SourceMgr.setModuleBuildStack(
- ImportingInstance.getSourceManager().getModuleBuildStack());
+ getSourceManager().getModuleBuildStack());
SourceMgr.pushModuleBuildStack(ModuleName,
- FullSourceLoc(ImportLoc, ImportingInstance.getSourceManager()));
+ FullSourceLoc(ImportLoc, getSourceManager()));
// Make sure that the failed-module structure has been allocated in
// the importing instance, and propagate the pointer to the newly-created
// instance.
- if (!ImportingInstance.hasFailedModulesSet())
- ImportingInstance.createFailedModulesSet();
- Instance.setFailedModulesSet(ImportingInstance.getFailedModulesSetPtr());
+ if (!hasFailedModulesSet())
+ createFailedModulesSet();
+ Instance.setFailedModulesSet(getFailedModulesSetPtr());
// If we're collecting module dependencies, we need to share a collector
// between all of the module CompilerInstances. Other than that, we don't
// want to produce any dependency output from the module build.
- Instance.setModuleDepCollector(ImportingInstance.getModuleDepCollector());
+ Instance.setModuleDepCollector(getModuleDepCollector());
Inv.getDependencyOutputOpts() = DependencyOutputOptions();
return InstancePtr;
@@ -1378,8 +1370,8 @@ std::unique_ptr<CompilerInstance>
CompilerInstance::cloneForModuleCompile(
bool IsSystem = isSystem(SLoc.getFile().getFileCharacteristic());
// Use the module map where this module resides.
- return createCompilerInstanceForModuleCompileImpl(
- *this, ImportLoc, ModuleName,
+ return cloneForModuleCompileImpl(
+ ImportLoc, ModuleName,
FrontendInputFile(ModuleMapFilePath, IK, IsSystem),
ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName);
}
@@ -1395,8 +1387,8 @@ std::unique_ptr<CompilerInstance>
CompilerInstance::cloneForModuleCompile(
llvm::raw_string_ostream OS(InferredModuleMapContent);
Module->print(OS);
- auto Instance = createCompilerInstanceForModuleCompileImpl(
- *this, ImportLoc, ModuleName,
+ auto Instance = cloneForModuleCompileImpl(
+ ImportLoc, ModuleName,
FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem),
ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName);
@@ -2238,8 +2230,8 @@ void
CompilerInstance::createModuleFromSource(SourceLocation ImportLoc,
std::string NullTerminatedSource(Source.str());
- auto Other = createCompilerInstanceForModuleCompileImpl(
- *this, ImportLoc, ModuleName, Input, StringRef(), ModuleFileName);
+ auto Other = cloneForModuleCompileImpl(
+ ImportLoc, ModuleName, Input, StringRef(), ModuleFileName);
// Create a virtual file containing our desired source.
// FIXME: We shouldn't need to do this.
>From d29fe56ea7aa2537fa07e21b37383a043900511e Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Fri, 11 Apr 2025 10:16:24 -0700
Subject: [PATCH 2/4] [clang][frontend] Promote `compileModule` to
`CompilerInstance` member
---
.../include/clang/Frontend/CompilerInstance.h | 8 ++++
clang/lib/Frontend/CompilerInstance.cpp | 44 ++++++++-----------
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/clang/include/clang/Frontend/CompilerInstance.h
b/clang/include/clang/Frontend/CompilerInstance.h
index bde1b1f36759e..f386524bfc17c 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -886,6 +886,14 @@ class CompilerInstance : public ModuleLoader {
cloneForModuleCompile(SourceLocation ImportLoc, Module *Module,
StringRef ModuleFileName);
+ /// 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.
+ // FIXME: This should be private, but it's called from static non-member
+ // functions in the implementation file.
+ bool compileModule(SourceLocation ImportLoc, StringRef ModuleName,
+ StringRef ModuleFileName, CompilerInstance &Instance);
+
ModuleLoadResult loadModule(SourceLocation ImportLoc, ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective) override;
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index 97987d568e729..74385cb18ba80 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1252,28 +1252,22 @@ std::unique_ptr<CompilerInstance>
CompilerInstance::cloneForModuleCompileImpl(
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) {
+bool CompilerInstance::compileModule(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)
+ if (getModuleCache().getInMemoryModuleCache().isPCMFinal(ModuleFileName)) {
+ getDiagnostics().Report(ImportLoc, diag::err_module_rebuild_finalized)
<< ModuleName;
return false;
}
- ImportingInstance.getDiagnostics().Report(ImportLoc,
- diag::remark_module_build)
- << ModuleName << ModuleFileName;
+ getDiagnostics().Report(ImportLoc, diag::remark_module_build)
+ << ModuleName << ModuleFileName;
// Execute the action to actually build the module in-place. Use a separate
// thread so that we get a stack large enough.
@@ -1284,13 +1278,12 @@ static bool compileModule(CompilerInstance
&ImportingInstance,
},
DesiredStackSize);
- ImportingInstance.getDiagnostics().Report(ImportLoc,
- diag::remark_module_build_done)
- << ModuleName;
+ getDiagnostics().Report(ImportLoc, diag::remark_module_build_done)
+ << ModuleName;
// Propagate the statistics to the parent FileManager.
- if (!ImportingInstance.getFrontendOpts().ModulesShareFileManager)
- ImportingInstance.getFileManager().AddStats(Instance.getFileManager());
+ if (!getFrontendOpts().ModulesShareFileManager)
+ getFileManager().AddStats(Instance.getFileManager());
if (Crashed) {
// Clear the ASTConsumer if it hasn't been already, in case it owns streams
@@ -1304,8 +1297,8 @@ static bool compileModule(CompilerInstance
&ImportingInstance,
// 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 (getFrontendOpts().GenerateGlobalModuleIndex) {
+ setBuildGlobalModuleIndex(true);
}
// If \p AllowPCMWithCompilerErrors is set return 'success' even if errors
@@ -1452,9 +1445,9 @@ static bool compileModuleAndReadASTImpl(CompilerInstance
&ImportingInstance,
auto Instance = ImportingInstance.cloneForModuleCompile(ModuleNameLoc,
Module,
ModuleFileName);
- if (!compileModule(ImportingInstance, ModuleNameLoc,
- Module->getTopLevelModuleName(), ModuleFileName,
- *Instance)) {
+ if (!ImportingInstance.compileModule(ModuleNameLoc,
+ Module->getTopLevelModuleName(),
+ ModuleFileName, *Instance)) {
ImportingInstance.getDiagnostics().Report(ModuleNameLoc,
diag::err_module_not_built)
<< Module->Name << SourceRange(ImportLoc, ModuleNameLoc);
@@ -2244,8 +2237,7 @@ void
CompilerInstance::createModuleFromSource(SourceLocation ImportLoc,
Other->DeleteBuiltModules = false;
// Build the module, inheriting any modules that we've built locally.
- bool Success =
- compileModule(*this, ImportLoc, ModuleName, ModuleFileName, *Other);
+ bool Success = compileModule(ImportLoc, ModuleName, ModuleFileName, *Other);
BuiltModules = std::move(Other->BuiltModules);
>From f98b9896ed25c67df47b1a4f976ecae44c2cf99a Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Fri, 11 Apr 2025 10:35:24 -0700
Subject: [PATCH 3/4] [clang][frontend] Make `CompilerInstance::FailedModules`
thread-safe
---
.../include/clang/Frontend/CompilerInstance.h | 32 ++-----------------
clang/lib/Frontend/CompilerInstance.cpp | 16 ++++------
2 files changed, 9 insertions(+), 39 deletions(-)
diff --git a/clang/include/clang/Frontend/CompilerInstance.h
b/clang/include/clang/Frontend/CompilerInstance.h
index f386524bfc17c..41dc7f1fef461 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -134,23 +134,13 @@ class CompilerInstance : public ModuleLoader {
std::vector<std::shared_ptr<DependencyCollector>> DependencyCollectors;
- /// Records the set of modules
- class FailedModulesSet {
- llvm::StringSet<> Failed;
-
- public:
- bool hasAlreadyFailed(StringRef module) { return Failed.count(module) > 0;
}
-
- void addFailed(StringRef module) { Failed.insert(module); }
- };
-
/// The set of modules that failed to build.
///
- /// This pointer will be shared among all of the compiler instances created
+ /// This value will be passed among all of the compiler instances created
/// to (re)build modules, so that once a module fails to build anywhere,
/// other instances will see that the module has failed and won't try to
/// build it again.
- std::shared_ptr<FailedModulesSet> FailedModules;
+ llvm::StringSet<> FailedModules;
/// The set of top-level modules that has already been built on the
/// fly as part of this overall compilation action.
@@ -637,24 +627,6 @@ class CompilerInstance : public ModuleLoader {
return *FrontendTimer;
}
- /// @}
- /// @name Failed modules set
- /// @{
-
- bool hasFailedModulesSet() const { return (bool)FailedModules; }
-
- void createFailedModulesSet() {
- FailedModules = std::make_shared<FailedModulesSet>();
- }
-
- std::shared_ptr<FailedModulesSet> getFailedModulesSetPtr() const {
- return FailedModules;
- }
-
- void setFailedModulesSet(std::shared_ptr<FailedModulesSet> FMS) {
- FailedModules = FMS;
- }
-
/// }
/// @name Output Files
/// @{
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index 74385cb18ba80..7ff711df171fa 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1236,12 +1236,8 @@ std::unique_ptr<CompilerInstance>
CompilerInstance::cloneForModuleCompileImpl(
SourceMgr.pushModuleBuildStack(ModuleName,
FullSourceLoc(ImportLoc, getSourceManager()));
- // Make sure that the failed-module structure has been allocated in
- // the importing instance, and propagate the pointer to the newly-created
- // instance.
- if (!hasFailedModulesSet())
- createFailedModulesSet();
- Instance.setFailedModulesSet(getFailedModulesSetPtr());
+ // Make a copy for the new instance.
+ Instance.FailedModules = FailedModules;
// If we're collecting module dependencies, we need to share a collector
// between all of the module CompilerInstances. Other than that, we don't
@@ -1285,6 +1281,9 @@ bool CompilerInstance::compileModule(SourceLocation
ImportLoc,
if (!getFrontendOpts().ModulesShareFileManager)
getFileManager().AddStats(Instance.getFileManager());
+ // Propagate the failed modules to the parent instance.
+ FailedModules = std::move(Instance.FailedModules);
+
if (Crashed) {
// Clear the ASTConsumer if it hasn't been already, in case it owns streams
// that must be closed before clearing output files.
@@ -1987,7 +1986,7 @@ ModuleLoadResult
CompilerInstance::findOrCompileModuleAndReadAST(
}
// Check whether we have already attempted to build this module (but failed).
- if (FailedModules && FailedModules->hasAlreadyFailed(ModuleName)) {
+ if (FailedModules.contains(ModuleName)) {
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
<< ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
return nullptr;
@@ -1998,8 +1997,7 @@ ModuleLoadResult
CompilerInstance::findOrCompileModuleAndReadAST(
ModuleFilename)) {
assert(getDiagnostics().hasErrorOccurred() &&
"undiagnosed error in compileModuleAndReadAST");
- if (FailedModules)
- FailedModules->addFailed(ModuleName);
+ FailedModules.insert(ModuleName);
return nullptr;
}
>From 2a1d2748f7cd2513018b3902b45a05d9694c3e2e Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Fri, 11 Apr 2025 20:59:45 -0700
Subject: [PATCH 4/4] clang-format
---
clang/lib/Frontend/CompilerInstance.cpp | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index 7ff711df171fa..243e0a3c15b05 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1198,16 +1198,15 @@ std::unique_ptr<CompilerInstance>
CompilerInstance::cloneForModuleCompileImpl(
DiagnosticOptions &DiagOpts = Invocation->getDiagnosticOpts();
DiagOpts.VerifyDiagnostics = 0;
- assert(getInvocation().getModuleHash() ==
- Invocation->getModuleHash() && "Module hash mismatch!");
+ assert(getInvocation().getModuleHash() == Invocation->getModuleHash() &&
+ "Module hash mismatch!");
// Construct a compiler instance that will be used to actually create the
// module. Since we're sharing an in-memory module cache,
// CompilerInstance::CompilerInstance is responsible for finalizing the
// buffers to prevent use-after-frees.
auto InstancePtr = std::make_unique<CompilerInstance>(
- getPCHContainerOperations(),
- &getModuleCache());
+ getPCHContainerOperations(), &getModuleCache());
auto &Instance = *InstancePtr;
auto &Inv = *Invocation;
@@ -1231,10 +1230,9 @@ std::unique_ptr<CompilerInstance>
CompilerInstance::cloneForModuleCompileImpl(
// Note that this module is part of the module build stack, so that we
// can detect cycles in the module graph.
- SourceMgr.setModuleBuildStack(
- getSourceManager().getModuleBuildStack());
+ SourceMgr.setModuleBuildStack(getSourceManager().getModuleBuildStack());
SourceMgr.pushModuleBuildStack(ModuleName,
- FullSourceLoc(ImportLoc, getSourceManager()));
+ FullSourceLoc(ImportLoc, getSourceManager()));
// Make a copy for the new instance.
Instance.FailedModules = FailedModules;
@@ -2221,8 +2219,8 @@ void
CompilerInstance::createModuleFromSource(SourceLocation ImportLoc,
std::string NullTerminatedSource(Source.str());
- auto Other = cloneForModuleCompileImpl(
- ImportLoc, ModuleName, Input, StringRef(), ModuleFileName);
+ auto Other = cloneForModuleCompileImpl(ImportLoc, ModuleName, Input,
+ StringRef(), ModuleFileName);
// Create a virtual file containing our desired source.
// FIXME: We shouldn't need to do this.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
