[clang] ee3a302 - [clang] Move state out of `PreprocessorOptions` (2/n) (#87099)

via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 29 12:07:51 PDT 2024


Author: Jan Svoboda
Date: 2024-03-29T12:07:47-07:00
New Revision: ee3a302abaa091e550a80f79694e963d1b5d0b7b

URL: https://github.com/llvm/llvm-project/commit/ee3a302abaa091e550a80f79694e963d1b5d0b7b
DIFF: https://github.com/llvm/llvm-project/commit/ee3a302abaa091e550a80f79694e963d1b5d0b7b.diff

LOG: [clang] Move state out of `PreprocessorOptions` (2/n) (#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.

Added: 
    

Modified: 
    clang/include/clang/Frontend/CompilerInstance.h
    clang/include/clang/Lex/PreprocessorOptions.h
    clang/lib/Frontend/CompilerInstance.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index cce91862ae3d03..3464654284f199 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -133,6 +133,24 @@ 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 +637,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;
   }
 


        


More information about the cfe-commits mailing list