From 1c8f8988358dd602829a3261cff4e721b4108ab9 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 21 Apr 2025 12:45:23 -0700 Subject: [PATCH 1/2] [clang] Enable making `CompilerInstance` diagnostics thread-safe --- .../include/clang/Frontend/CompilerInstance.h | 15 ++++++++---- clang/lib/Frontend/CompilerInstance.cpp | 23 ++++++++++++------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index d70f5c45b3d38..86e6eb38f5306 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -848,10 +848,13 @@ class CompilerInstance : public ModuleLoader { /// /// Explicitly-specified \c VFS takes precedence over the VFS of this instance /// when creating the clone and also prevents \c FileManager sharing. + /// Explicitly-specified \c DiagConsumer takes precedence over forwarding to + /// this instance. std::unique_ptr cloneForModuleCompileImpl( SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input, StringRef OriginalModuleMapFile, StringRef ModuleFileName, - IntrusiveRefCntPtr VFS = nullptr); + IntrusiveRefCntPtr VFS = nullptr, + DiagnosticConsumer *DiagConsumer = nullptr); public: /// Creates a new \c CompilerInstance for compiling a module. @@ -861,9 +864,13 @@ class CompilerInstance : public ModuleLoader { /// /// Explicitly-specified \c VFS takes precedence over the VFS of this instance /// when creating the clone and also prevents \c FileManager sharing. - std::unique_ptr cloneForModuleCompile( - SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName, - IntrusiveRefCntPtr VFS = nullptr); + /// Explicitly-specified \c DiagConsumer takes precedence over forwarding to + /// this instance. + std::unique_ptr + cloneForModuleCompile(SourceLocation ImportLoc, Module *Module, + StringRef ModuleFileName, + IntrusiveRefCntPtr VFS = nullptr, + DiagnosticConsumer *DiagConsumer = nullptr); /// Compile a module file for the given module, using the options /// provided by the importing compiler instance. Returns true if the module diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index bc663acb034e7..ad502827f98c7 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1153,7 +1153,8 @@ static Language getLanguageFromOptions(const LangOptions &LangOpts) { std::unique_ptr CompilerInstance::cloneForModuleCompileImpl( SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input, StringRef OriginalModuleMapFile, StringRef ModuleFileName, - IntrusiveRefCntPtr VFS) { + IntrusiveRefCntPtr VFS, + DiagnosticConsumer *DiagConsumer) { // Construct a compiler invocation for creating this module. auto Invocation = std::make_shared(getInvocation()); @@ -1221,10 +1222,15 @@ std::unique_ptr CompilerInstance::cloneForModuleCompileImpl( Instance.createFileManager(&getVirtualFileSystem()); } - Instance.createDiagnostics( - Instance.getVirtualFileSystem(), - new ForwardingDiagnosticConsumer(getDiagnosticClient()), - /*ShouldOwnClient=*/true); + if (DiagConsumer) { + Instance.createDiagnostics(Instance.getVirtualFileSystem(), DiagConsumer, + /*ShouldOwnClient=*/false); + } else { + Instance.createDiagnostics( + Instance.getVirtualFileSystem(), + new ForwardingDiagnosticConsumer(getDiagnosticClient()), + /*ShouldOwnClient=*/true); + } if (llvm::is_contained(DiagOpts.SystemHeaderWarningsModules, ModuleName)) Instance.getDiagnostics().setSuppressSystemWarnings(false); @@ -1322,7 +1328,8 @@ static OptionalFileEntryRef getPublicModuleMap(FileEntryRef File, std::unique_ptr CompilerInstance::cloneForModuleCompile( SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName, - IntrusiveRefCntPtr VFS) { + IntrusiveRefCntPtr VFS, + DiagnosticConsumer *DiagConsumer) { StringRef ModuleName = Module->getTopLevelModuleName(); InputKind IK(getLanguageFromOptions(getLangOpts()), InputKind::ModuleMap); @@ -1368,7 +1375,7 @@ std::unique_ptr CompilerInstance::cloneForModuleCompile( ImportLoc, ModuleName, FrontendInputFile(ModuleMapFilePath, IK, IsSystem), ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName, - std::move(VFS)); + std::move(VFS), DiagConsumer); } // FIXME: We only need to fake up an input file here as a way of @@ -1386,7 +1393,7 @@ std::unique_ptr CompilerInstance::cloneForModuleCompile( ImportLoc, ModuleName, FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem), ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName, - std::move(VFS)); + std::move(VFS), DiagConsumer); std::unique_ptr ModuleMapBuffer = llvm::MemoryBuffer::getMemBufferCopy(InferredModuleMapContent); From a5b85b1d7860edb4df080af74825c6f204f8bb96 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 22 Apr 2025 12:31:06 -0700 Subject: [PATCH 2/2] Extract VFS and `DiagnosticConsumer` into `ThreadSafeCloneConfig` --- .../include/clang/Frontend/CompilerInstance.h | 39 +++++++++++-------- clang/lib/Frontend/CompilerInstance.cpp | 19 +++++---- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 86e6eb38f5306..6007d5659c6b4 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -825,6 +825,23 @@ class CompilerInstance : public ModuleLoader { bool loadModuleFile(StringRef FileName, serialization::ModuleFile *&LoadedModuleFile); + /// Configuration object for making the result of \c cloneForModuleCompile() + /// thread-safe. + class ThreadSafeCloneConfig { + IntrusiveRefCntPtr VFS; + DiagnosticConsumer &DiagConsumer; + + public: + ThreadSafeCloneConfig(IntrusiveRefCntPtr VFS, + DiagnosticConsumer &DiagConsumer) + : VFS(std::move(VFS)), DiagConsumer(DiagConsumer) { + assert(this->VFS && "Clone config requires non-null VFS"); + } + + IntrusiveRefCntPtr getVFS() const { return VFS; } + DiagnosticConsumer &getDiagConsumer() const { return DiagConsumer; } + }; + private: /// Find a module, potentially compiling it, before reading its AST. This is /// the guts of loadModule. @@ -845,16 +862,10 @@ class CompilerInstance : public ModuleLoader { /// Creates a \c CompilerInstance for compiling a module. /// /// This expects a properly initialized \c FrontendInputFile. - /// - /// Explicitly-specified \c VFS takes precedence over the VFS of this instance - /// when creating the clone and also prevents \c FileManager sharing. - /// Explicitly-specified \c DiagConsumer takes precedence over forwarding to - /// this instance. std::unique_ptr cloneForModuleCompileImpl( SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input, StringRef OriginalModuleMapFile, StringRef ModuleFileName, - IntrusiveRefCntPtr VFS = nullptr, - DiagnosticConsumer *DiagConsumer = nullptr); + std::optional ThreadSafeConfig = std::nullopt); public: /// Creates a new \c CompilerInstance for compiling a module. @@ -862,15 +873,11 @@ class CompilerInstance : public ModuleLoader { /// This takes care of creating appropriate \c FrontendInputFile for /// public/private frameworks, inferred modules and such. /// - /// Explicitly-specified \c VFS takes precedence over the VFS of this instance - /// when creating the clone and also prevents \c FileManager sharing. - /// Explicitly-specified \c DiagConsumer takes precedence over forwarding to - /// this instance. - std::unique_ptr - cloneForModuleCompile(SourceLocation ImportLoc, Module *Module, - StringRef ModuleFileName, - IntrusiveRefCntPtr VFS = nullptr, - DiagnosticConsumer *DiagConsumer = nullptr); + /// The \c ThreadSafeConfig takes precedence over the \c DiagnosticConsumer + /// and \c FileSystem of this instance (and disables \c FileManager sharing). + std::unique_ptr cloneForModuleCompile( + SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName, + std::optional ThreadSafeConfig = std::nullopt); /// Compile a module file for the given module, using the options /// provided by the importing compiler instance. Returns true if the module diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index ad502827f98c7..de633f0ec8734 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1153,8 +1153,7 @@ static Language getLanguageFromOptions(const LangOptions &LangOpts) { std::unique_ptr CompilerInstance::cloneForModuleCompileImpl( SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input, StringRef OriginalModuleMapFile, StringRef ModuleFileName, - IntrusiveRefCntPtr VFS, - DiagnosticConsumer *DiagConsumer) { + std::optional ThreadSafeConfig) { // Construct a compiler invocation for creating this module. auto Invocation = std::make_shared(getInvocation()); @@ -1214,16 +1213,17 @@ std::unique_ptr CompilerInstance::cloneForModuleCompileImpl( auto &Inv = *Invocation; Instance.setInvocation(std::move(Invocation)); - if (VFS) { - Instance.createFileManager(std::move(VFS)); + if (ThreadSafeConfig) { + Instance.createFileManager(ThreadSafeConfig->getVFS()); } else if (FrontendOpts.ModulesShareFileManager) { Instance.setFileManager(&getFileManager()); } else { Instance.createFileManager(&getVirtualFileSystem()); } - if (DiagConsumer) { - Instance.createDiagnostics(Instance.getVirtualFileSystem(), DiagConsumer, + if (ThreadSafeConfig) { + Instance.createDiagnostics(Instance.getVirtualFileSystem(), + &ThreadSafeConfig->getDiagConsumer(), /*ShouldOwnClient=*/false); } else { Instance.createDiagnostics( @@ -1328,8 +1328,7 @@ static OptionalFileEntryRef getPublicModuleMap(FileEntryRef File, std::unique_ptr CompilerInstance::cloneForModuleCompile( SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName, - IntrusiveRefCntPtr VFS, - DiagnosticConsumer *DiagConsumer) { + std::optional ThreadSafeConfig) { StringRef ModuleName = Module->getTopLevelModuleName(); InputKind IK(getLanguageFromOptions(getLangOpts()), InputKind::ModuleMap); @@ -1375,7 +1374,7 @@ std::unique_ptr CompilerInstance::cloneForModuleCompile( ImportLoc, ModuleName, FrontendInputFile(ModuleMapFilePath, IK, IsSystem), ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName, - std::move(VFS), DiagConsumer); + std::move(ThreadSafeConfig)); } // FIXME: We only need to fake up an input file here as a way of @@ -1393,7 +1392,7 @@ std::unique_ptr CompilerInstance::cloneForModuleCompile( ImportLoc, ModuleName, FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem), ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName, - std::move(VFS), DiagConsumer); + std::move(ThreadSafeConfig)); std::unique_ptr ModuleMapBuffer = llvm::MemoryBuffer::getMemBufferCopy(InferredModuleMapContent);