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_svob...@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_svob...@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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits