[clang] [clang] Move state out of `PreprocessorOptions` (2/n) (PR #87099)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 29 12:04:58 PDT 2024
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/87099
>From aa4f75ab41f5bc018c8c5a6352e8b9688249fb21 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 22 Mar 2024 15:08:07 -0700
Subject: [PATCH 1/2] [clang] Move state out of `PreprocessorOptions` (2/n)
An instance of `PreprocessorOptions` is part of `CompilerInvocation` which is supposed to be a value type. The `FailedModules` member is problematic, since it's essentially a shared state used by multiple `CompilerInstance` objects, and not really a preprocessor option. Let's move it into `CompilerInstance` instead.
---
.../include/clang/Frontend/CompilerInstance.h | 40 +++++++++++++++++++
clang/include/clang/Lex/PreprocessorOptions.h | 22 ----------
clang/lib/Frontend/CompilerInstance.cpp | 27 +++++--------
3 files changed, 51 insertions(+), 38 deletions(-)
diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index cce91862ae3d03..4684c527de13a4 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -133,6 +133,28 @@ 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
+ /// 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;
+
/// The set of top-level modules that has already been built on the
/// fly as part of this overall compilation action.
std::map<std::string, std::string, std::less<>> BuiltModules;
@@ -619,6 +641,24 @@ class CompilerInstance : public ModuleLoader {
}
/// @}
+ /// @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/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h
index 24e0ca61d41432..50b5fba0ff773e 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -186,28 +186,6 @@ class PreprocessorOptions {
/// with support for lifetime-qualified pointers.
ObjCXXARCStandardLibraryKind ObjCXXARCStandardLibrary = ARCXX_nolib;
- /// 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
- /// 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;
-
/// Set up preprocessor for RunAnalysis action.
bool SetUpStaticAnalyzer = false;
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 79ebb0ae0ee98f..6e3baf83864415 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1206,16 +1206,6 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
// Note the name of the module we're building.
Invocation->getLangOpts().CurrentModule = std::string(ModuleName);
- // Make sure that the failed-module structure has been allocated in
- // the importing instance, and propagate the pointer to the newly-created
- // instance.
- PreprocessorOptions &ImportingPPOpts
- = ImportingInstance.getInvocation().getPreprocessorOpts();
- if (!ImportingPPOpts.FailedModules)
- ImportingPPOpts.FailedModules =
- std::make_shared<PreprocessorOptions::FailedModulesSet>();
- PPOpts.FailedModules = ImportingPPOpts.FailedModules;
-
// If there is a module map file, build the module using the module map.
// Set up the inputs/outputs so that we build the module from its umbrella
// header.
@@ -1269,6 +1259,13 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
SourceMgr.pushModuleBuildStack(ModuleName,
FullSourceLoc(ImportLoc, ImportingInstance.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 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.
@@ -1992,10 +1989,8 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
return nullptr;
}
- // Check whether we have already attempted to build this module (but
- // failed).
- if (getPreprocessorOpts().FailedModules &&
- getPreprocessorOpts().FailedModules->hasAlreadyFailed(ModuleName)) {
+ // Check whether we have already attempted to build this module (but failed).
+ if (FailedModules && FailedModules->hasAlreadyFailed(ModuleName)) {
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
<< ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
return nullptr;
@@ -2006,8 +2001,8 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
ModuleFilename)) {
assert(getDiagnostics().hasErrorOccurred() &&
"undiagnosed error in compileModuleAndReadAST");
- if (getPreprocessorOpts().FailedModules)
- getPreprocessorOpts().FailedModules->addFailed(ModuleName);
+ if (FailedModules)
+ FailedModules->addFailed(ModuleName);
return nullptr;
}
>From c85dfdebb7e6134cbeb727208cd9fb7e395b6c1d Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 29 Mar 2024 12:04:17 -0700
Subject: [PATCH 2/2] Formatting
---
clang/include/clang/Frontend/CompilerInstance.h | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index 4684c527de13a4..3464654284f199 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -138,13 +138,9 @@ class CompilerInstance : public ModuleLoader {
llvm::StringSet<> Failed;
public:
- bool hasAlreadyFailed(StringRef module) {
- return Failed.count(module) > 0;
- }
+ bool hasAlreadyFailed(StringRef module) { return Failed.count(module) > 0; }
- void addFailed(StringRef module) {
- Failed.insert(module);
- }
+ void addFailed(StringRef module) { Failed.insert(module); }
};
/// The set of modules that failed to build.
More information about the cfe-commits
mailing list