Author: Jan Svoboda Date: 2026-01-06T13:38:34-08:00 New Revision: e667c39e495d5c8ba47dba6107ca3dc2e9c91048
URL: https://github.com/llvm/llvm-project/commit/e667c39e495d5c8ba47dba6107ca3dc2e9c91048 DIFF: https://github.com/llvm/llvm-project/commit/e667c39e495d5c8ba47dba6107ca3dc2e9c91048.diff LOG: [clang] Reference-count `ModuleCache` non-intrusively (#164889) The `ModuleCache` class is currently reference-counted intrusively. As explained in https://github.com/llvm/llvm-project/pull/139584, this is problematic. This PR uses `std::shared_ptr` to reference-count `ModuleCache` instead, which clarifies what happens to its lifetime when constructing `CompilerInstance`, for example. This also makes the reference in `ModuleManager` non-owning, simplifying the ownership relationship further. The `ASTUnit::transferASTDataFromCompilerInstance()` function now accounts for that by taking care to keep it alive. Added: Modified: clang-tools-extra/clangd/ModulesBuilder.cpp clang/include/clang/DependencyScanning/InProcessModuleCache.h clang/include/clang/Frontend/ASTUnit.h clang/include/clang/Frontend/CompilerInstance.h clang/include/clang/Serialization/ModuleCache.h clang/include/clang/Serialization/ModuleManager.h clang/lib/DependencyScanning/DependencyScannerImpl.cpp clang/lib/DependencyScanning/InProcessModuleCache.cpp clang/lib/Frontend/ASTUnit.cpp clang/lib/Frontend/CompilerInstance.cpp clang/lib/Frontend/Rewrite/FrontendActions.cpp clang/lib/Serialization/ModuleCache.cpp clang/lib/Serialization/ModuleManager.cpp clang/unittests/Serialization/ModuleCacheTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 0177a1751bd60..524ec620c4076 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -265,7 +265,7 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath, Preprocessor PP(PPOpts, *Diags, LangOpts, SourceMgr, HeaderInfo, ModuleLoader); - IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache(); + std::shared_ptr<ModuleCache> ModCache = createCrossProcessModuleCache(); PCHContainerOperations PCHOperations; CodeGenOptions CodeGenOpts; ASTReader Reader(PP, *ModCache, /*ASTContext=*/nullptr, diff --git a/clang/include/clang/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/DependencyScanning/InProcessModuleCache.h index 0585348fa7d1d..4c95171a2e21e 100644 --- a/clang/include/clang/DependencyScanning/InProcessModuleCache.h +++ b/clang/include/clang/DependencyScanning/InProcessModuleCache.h @@ -12,6 +12,7 @@ #include "clang/Serialization/ModuleCache.h" #include "llvm/ADT/StringMap.h" +#include <atomic> #include <mutex> #include <shared_mutex> @@ -28,7 +29,7 @@ struct ModuleCacheEntries { llvm::StringMap<std::unique_ptr<ModuleCacheEntry>> Map; }; -IntrusiveRefCntPtr<ModuleCache> +std::shared_ptr<ModuleCache> makeInProcessModuleCache(ModuleCacheEntries &Entries); } // namespace dependencies diff --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h index 0e3c52ae59320..db6bd11dae1c1 100644 --- a/clang/include/clang/Frontend/ASTUnit.h +++ b/clang/include/clang/Frontend/ASTUnit.h @@ -101,7 +101,7 @@ class ASTUnit { IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics; IntrusiveRefCntPtr<FileManager> FileMgr; IntrusiveRefCntPtr<SourceManager> SourceMgr; - IntrusiveRefCntPtr<ModuleCache> ModCache; + std::shared_ptr<ModuleCache> ModCache; std::unique_ptr<HeaderSearch> HeaderInfo; IntrusiveRefCntPtr<TargetInfo> Target; std::shared_ptr<Preprocessor> PP; diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index ded5f55d180aa..d0b41e9f18a1b 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -109,7 +109,7 @@ class CompilerInstance : public ModuleLoader { IntrusiveRefCntPtr<SourceManager> SourceMgr; /// The cache of PCM files. - IntrusiveRefCntPtr<ModuleCache> ModCache; + std::shared_ptr<ModuleCache> ModCache; /// Functor for getting the dependency preprocessor directives of a file. std::unique_ptr<DependencyDirectivesGetter> GetDependencyDirectives; @@ -205,7 +205,7 @@ class CompilerInstance : public ModuleLoader { std::make_shared<CompilerInvocation>(), std::shared_ptr<PCHContainerOperations> PCHContainerOps = std::make_shared<PCHContainerOperations>(), - ModuleCache *ModCache = nullptr); + std::shared_ptr<ModuleCache> ModCache = nullptr); ~CompilerInstance() override; /// @name High-Level Operations @@ -967,6 +967,7 @@ class CompilerInstance : public ModuleLoader { void setExternalSemaSource(IntrusiveRefCntPtr<ExternalSemaSource> ESS); ModuleCache &getModuleCache() const { return *ModCache; } + std::shared_ptr<ModuleCache> getModuleCachePtr() const { return ModCache; } }; } // end namespace clang diff --git a/clang/include/clang/Serialization/ModuleCache.h b/clang/include/clang/Serialization/ModuleCache.h index ec052c5c18e0a..c6795c5dc358a 100644 --- a/clang/include/clang/Serialization/ModuleCache.h +++ b/clang/include/clang/Serialization/ModuleCache.h @@ -10,7 +10,6 @@ #define LLVM_CLANG_SERIALIZATION_MODULECACHE_H #include "clang/Basic/LLVM.h" -#include "llvm/ADT/IntrusiveRefCntPtr.h" #include <ctime> @@ -23,7 +22,7 @@ class InMemoryModuleCache; /// The module cache used for compiling modules implicitly. This centralizes the /// operations the compiler might want to perform on the cache. -class ModuleCache : public RefCountedBase<ModuleCache> { +class ModuleCache { public: /// May perform any work that only needs to be performed once for multiple /// calls \c getLock() with the same module filename. @@ -62,7 +61,7 @@ class ModuleCache : public RefCountedBase<ModuleCache> { /// operated on by multiple processes. This instance must be used across all /// \c CompilerInstance instances participating in building modules for single /// translation unit in order to share the same \c InMemoryModuleCache. -IntrusiveRefCntPtr<ModuleCache> createCrossProcessModuleCache(); +std::shared_ptr<ModuleCache> createCrossProcessModuleCache(); /// Shared implementation of `ModuleCache::maybePrune()`. void maybePruneImpl(StringRef Path, time_t PruneInterval, time_t PruneAfter); diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h index 1eb74aee9787c..8ab70b6630f47 100644 --- a/clang/include/clang/Serialization/ModuleManager.h +++ b/clang/include/clang/Serialization/ModuleManager.h @@ -18,7 +18,6 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Serialization/ModuleFile.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" @@ -65,7 +64,7 @@ class ModuleManager { FileManager &FileMgr; /// Cache of PCM files. - IntrusiveRefCntPtr<ModuleCache> ModCache; + ModuleCache &ModCache; /// Knows how to unwrap module containers. const PCHContainerReader &PCHContainerRdr; @@ -306,7 +305,7 @@ class ModuleManager { /// View the graphviz representation of the module graph. void viewGraph(); - ModuleCache &getModuleCache() const { return *ModCache; } + ModuleCache &getModuleCache() const { return ModCache; } }; } // namespace serialization diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index 0813f8c00f5c6..7df2b5a18ea1d 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -576,7 +576,7 @@ bool DependencyScanningAction::runInvocation( createScanCompilerInvocation(*OriginalInvocation, Service); auto ModCache = makeInProcessModuleCache(Service.getModuleCacheEntries()); ScanInstanceStorage.emplace(std::move(ScanInvocation), - std::move(PCHContainerOps), ModCache.get()); + std::move(PCHContainerOps), std::move(ModCache)); CompilerInstance &ScanInstance = *ScanInstanceStorage; assert(!DiagConsumerFinished && "attempt to reuse finished consumer"); @@ -650,11 +650,11 @@ bool CompilerInstanceWithContext::initialize( canonicalizeDefines(OriginalInvocation->getPreprocessorOpts()); // Create the CompilerInstance. - IntrusiveRefCntPtr<ModuleCache> ModCache = + std::shared_ptr<ModuleCache> ModCache = makeInProcessModuleCache(Worker.Service.getModuleCacheEntries()); CIPtr = std::make_unique<CompilerInstance>( createScanCompilerInvocation(*OriginalInvocation, Worker.Service), - Worker.PCHContainerOps, ModCache.get()); + Worker.PCHContainerOps, std::move(ModCache)); auto &CI = *CIPtr; initializeScanCompilerInstance( diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp index 1dd2d34032a96..4d5dd0c43112c 100644 --- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp +++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp @@ -113,7 +113,7 @@ class InProcessModuleCache : public ModuleCache { }; } // namespace -IntrusiveRefCntPtr<ModuleCache> +std::shared_ptr<ModuleCache> dependencies::makeInProcessModuleCache(ModuleCacheEntries &Entries) { - return llvm::makeIntrusiveRefCnt<InProcessModuleCache>(Entries); + return std::make_shared<InProcessModuleCache>(Entries); } diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index ab0e403415250..0570b8ece24f8 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1431,6 +1431,7 @@ void ASTUnit::transferASTDataFromCompilerInstance(CompilerInstance &CI) { if (CI.hasTarget()) Target = CI.getTargetPtr(); Reader = CI.getASTReader(); + ModCache = CI.getModuleCachePtr(); HadModuleLoaderFatalFailure = CI.hadModuleLoaderFatalFailure(); if (Invocation != CI.getInvocationPtr()) { // This happens when Parse creates a copy of \c Invocation to modify. @@ -2218,7 +2219,7 @@ bool ASTUnit::serialize(raw_ostream &OS) { SmallString<128> Buffer; llvm::BitstreamWriter Stream(Buffer); - IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache(); + std::shared_ptr<ModuleCache> ModCache = createCrossProcessModuleCache(); ASTWriter Writer(Stream, Buffer, *ModCache, *CodeGenOpts, {}); return serializeUnit(Writer, Buffer, getSema(), OS); } diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index d85b32bad2d73..088538c7449d2 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -72,10 +72,11 @@ using namespace clang; CompilerInstance::CompilerInstance( std::shared_ptr<CompilerInvocation> Invocation, std::shared_ptr<PCHContainerOperations> PCHContainerOps, - ModuleCache *ModCache) - : ModuleLoader(/*BuildingModule=*/ModCache), + std::shared_ptr<ModuleCache> ModCache) + : ModuleLoader(/*BuildingModule=*/ModCache != nullptr), Invocation(std::move(Invocation)), - ModCache(ModCache ? ModCache : createCrossProcessModuleCache()), + ModCache(ModCache ? std::move(ModCache) + : createCrossProcessModuleCache()), ThePCHContainerOperations(std::move(PCHContainerOps)) { assert(this->Invocation && "Invocation must not be null"); } @@ -1169,7 +1170,7 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl( // CompilerInstance::CompilerInstance is responsible for finalizing the // buffers to prevent use-after-frees. auto InstancePtr = std::make_unique<CompilerInstance>( - std::move(Invocation), getPCHContainerOperations(), &getModuleCache()); + std::move(Invocation), getPCHContainerOperations(), ModCache); auto &Instance = *InstancePtr; auto &Inv = Instance.getInvocation(); diff --git a/clang/lib/Frontend/Rewrite/FrontendActions.cpp b/clang/lib/Frontend/Rewrite/FrontendActions.cpp index f5656b3b190e9..ef6f9ccf87848 100644 --- a/clang/lib/Frontend/Rewrite/FrontendActions.cpp +++ b/clang/lib/Frontend/Rewrite/FrontendActions.cpp @@ -244,7 +244,7 @@ class RewriteIncludesAction::RewriteImportsListener : public ASTReaderListener { // Rewrite the contents of the module in a separate compiler instance. CompilerInstance Instance( std::make_shared<CompilerInvocation>(CI.getInvocation()), - CI.getPCHContainerOperations(), &CI.getModuleCache()); + CI.getPCHContainerOperations(), CI.getModuleCachePtr()); Instance.setVirtualFileSystem(CI.getVirtualFileSystemPtr()); Instance.createDiagnostics( new ForwardingDiagnosticConsumer(CI.getDiagnosticClient()), diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp index e6e8cbf171bfa..658da6e3b7145 100644 --- a/clang/lib/Serialization/ModuleCache.cpp +++ b/clang/lib/Serialization/ModuleCache.cpp @@ -164,6 +164,6 @@ class CrossProcessModuleCache : public ModuleCache { }; } // namespace -IntrusiveRefCntPtr<ModuleCache> clang::createCrossProcessModuleCache() { - return llvm::makeIntrusiveRefCnt<CrossProcessModuleCache>(); +std::shared_ptr<ModuleCache> clang::createCrossProcessModuleCache() { + return std::make_shared<CrossProcessModuleCache>(); } diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 482c17649ed55..236fe20fdad7c 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -176,7 +176,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, if (NewModule->Kind == MK_ImplicitModule) NewModule->InputFilesValidationTimestamp = - ModCache->getModuleTimestamp(NewModule->FileName); + ModCache.getModuleTimestamp(NewModule->FileName); // Load the contents of the module if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) { @@ -330,7 +330,7 @@ void ModuleManager::moduleFileAccepted(ModuleFile *MF) { ModuleManager::ModuleManager(FileManager &FileMgr, ModuleCache &ModCache, const PCHContainerReader &PCHContainerRdr, const HeaderSearch &HeaderSearchInfo) - : FileMgr(FileMgr), ModCache(&ModCache), PCHContainerRdr(PCHContainerRdr), + : FileMgr(FileMgr), ModCache(ModCache), PCHContainerRdr(PCHContainerRdr), HeaderSearchInfo(HeaderSearchInfo) {} void ModuleManager::visit(llvm::function_ref<bool(ModuleFile &M)> Visitor, diff --git a/clang/unittests/Serialization/ModuleCacheTest.cpp b/clang/unittests/Serialization/ModuleCacheTest.cpp index df26e54588b9e..141e9292137a9 100644 --- a/clang/unittests/Serialization/ModuleCacheTest.cpp +++ b/clang/unittests/Serialization/ModuleCacheTest.cpp @@ -146,7 +146,7 @@ TEST_F(ModuleCacheTest, CachedModuleNewPath) { ASSERT_TRUE(Invocation2); CompilerInstance Instance2(std::move(Invocation2), Instance.getPCHContainerOperations(), - &Instance.getModuleCache()); + Instance.getModuleCachePtr()); Instance2.setVirtualFileSystem(CIOpts.VFS); Instance2.setDiagnostics(Diags); SyntaxOnlyAction Action2; @@ -192,7 +192,7 @@ TEST_F(ModuleCacheTest, CachedModuleNewPathAllowErrors) { ASSERT_TRUE(Invocation2); CompilerInstance Instance2(std::move(Invocation2), Instance.getPCHContainerOperations(), - &Instance.getModuleCache()); + Instance.getModuleCachePtr()); Instance2.setVirtualFileSystem(CIOpts.VFS); Instance2.setDiagnostics(Diags); SyntaxOnlyAction Action2; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
