[clang] [clang] Move state out of `PreprocessorOptions` (2/n) (PR #87099)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/87099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move state out of `PreprocessorOptions` (2/n) (PR #87099)
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
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> 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 FailedModules;
+
/// The set of top-level modules that has already been built on the
/// fly as part of this overall compilation action.
std::map> 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();
+ }
+
+ std::shared_ptr getFailedModulesSetPtr() const {
+return FailedModules;
+ }
+
+ void setFailedModulesSet(std::shared_ptr 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 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();
- 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 fail
[clang] [clang] Move state out of `PreprocessorOptions` (2/n) (PR #87099)
https://github.com/benlangmuir approved this pull request. https://github.com/llvm/llvm-project/pull/87099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move state out of `PreprocessorOptions` (2/n) (PR #87099)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 407a2f231a81862e20d80059870c48d818b61ec2
aa4f75ab41f5bc018c8c5a6352e8b9688249fb21 --
clang/include/clang/Frontend/CompilerInstance.h
clang/include/clang/Lex/PreprocessorOptions.h
clang/lib/Frontend/CompilerInstance.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/include/clang/Frontend/CompilerInstance.h
b/clang/include/clang/Frontend/CompilerInstance.h
index 4684c527de..3464654284 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.
``
https://github.com/llvm/llvm-project/pull/87099
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move state out of `PreprocessorOptions` (2/n) (PR #87099)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Jan Svoboda (jansvoboda11)
Changes
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/87099.diff
3 Files Affected:
- (modified) clang/include/clang/Frontend/CompilerInstance.h (+40)
- (modified) clang/include/clang/Lex/PreprocessorOptions.h (-22)
- (modified) clang/lib/Frontend/CompilerInstance.cpp (+11-16)
``diff
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> 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 FailedModules;
+
/// The set of top-level modules that has already been built on the
/// fly as part of this overall compilation action.
std::map> 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();
+ }
+
+ std::shared_ptr getFailedModulesSetPtr() const {
+return FailedModules;
+ }
+
+ void setFailedModulesSet(std::shared_ptr 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 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();
- 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 (!ImportingInst
[clang] [clang] Move state out of `PreprocessorOptions` (2/n) (PR #87099)
https://github.com/jansvoboda11 created
https://github.com/llvm/llvm-project/pull/87099
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.
>From aa4f75ab41f5bc018c8c5a6352e8b9688249fb21 Mon Sep 17 00:00:00 2001
From: Jan Svoboda
Date: Fri, 22 Mar 2024 15:08:07 -0700
Subject: [PATCH] [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> 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 FailedModules;
+
/// The set of top-level modules that has already been built on the
/// fly as part of this overall compilation action.
std::map> 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();
+ }
+
+ std::shared_ptr getFailedModulesSetPtr() const {
+return FailedModules;
+ }
+
+ void setFailedModulesSet(std::shared_ptr 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 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();
- PPOpts.FailedModules = ImportingPPOpts.FailedModules;
-
// If there is a module map file, build the module using the module map.
// Set up the inputs/o
