[clang] 0797f70 - [clang] Enable making `CompilerInstance` diagnostics thread-safe (#136601)

[via cfe-commits]

Author: Jan Svoboda
Date: 2025-04-22T15:35:48-07:00
New Revision: 0797f708f52e5ab38845f18263de1b6d7c77c653

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

LOG: [clang] Enable making `CompilerInstance` diagnostics thread-safe (#136601)

The `DiagnosticConsumer` interface is not thread-safe. To enable
thread-safety of `CompilerInstance` objects cloned from the same parent,
this PR allows passing an explicit `DiagnosticConsumer` to
`cloneForModuleCompile()`. This will be used from the dependency
scanner.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index d70f5c45b3d38..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<llvm::vfs::FileSystem> VFS;
+    DiagnosticConsumer &DiagConsumer;
+
+  public:
+    ThreadSafeCloneConfig(IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+                          DiagnosticConsumer &DiagConsumer)
+        : VFS(std::move(VFS)), DiagConsumer(DiagConsumer) {
+      assert(this->VFS && "Clone config requires non-null VFS");
+    }
+
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> 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,13 +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.
   std::unique_ptr<CompilerInstance> cloneForModuleCompileImpl(
       SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input,
       StringRef OriginalModuleMapFile, StringRef ModuleFileName,
-      IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
+      std::optional<ThreadSafeCloneConfig> ThreadSafeConfig = std::nullopt);
 
 public:
   /// Creates a new \c CompilerInstance for compiling a module.
@@ -859,11 +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.
+  /// The \c ThreadSafeConfig takes precedence over the \c DiagnosticConsumer
+  /// and \c FileSystem of this instance (and disables \c FileManager sharing).
   std::unique_ptr<CompilerInstance> cloneForModuleCompile(
       SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName,
-      IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
+      std::optional<ThreadSafeCloneConfig> 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 bc663acb034e7..de633f0ec8734 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1153,7 +1153,7 @@ static Language getLanguageFromOptions(const LangOptions &LangOpts) {
 std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
     SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input,
     StringRef OriginalModuleMapFile, StringRef ModuleFileName,
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+    std::optional<ThreadSafeCloneConfig> ThreadSafeConfig) {
   // Construct a compiler invocation for creating this module.
   auto Invocation = std::make_shared<CompilerInvocation>(getInvocation());
 
@@ -1213,18 +1213,24 @@ std::unique_ptr<CompilerInstance> 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());
   }
 
-  Instance.createDiagnostics(
-      Instance.getVirtualFileSystem(),
-      new ForwardingDiagnosticConsumer(getDiagnosticClient()),
-      /*ShouldOwnClient=*/true);
+  if (ThreadSafeConfig) {
+    Instance.createDiagnostics(Instance.getVirtualFileSystem(),
+                               &ThreadSafeConfig->getDiagConsumer(),
+                               /*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,7 @@ static OptionalFileEntryRef getPublicModuleMap(FileEntryRef File,
 
 std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompile(
     SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName,
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+    std::optional<ThreadSafeCloneConfig> ThreadSafeConfig) {
   StringRef ModuleName = Module->getTopLevelModuleName();
 
   InputKind IK(getLanguageFromOptions(getLangOpts()), InputKind::ModuleMap);
@@ -1368,7 +1374,7 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompile(
         ImportLoc, ModuleName,
         FrontendInputFile(ModuleMapFilePath, IK, IsSystem),
         ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName,
-        std::move(VFS));
+        std::move(ThreadSafeConfig));
   }
 
   // FIXME: We only need to fake up an input file here as a way of
@@ -1386,7 +1392,7 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompile(
       ImportLoc, ModuleName,
       FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem),
       ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName,
-      std::move(VFS));
+      std::move(ThreadSafeConfig));
 
   std::unique_ptr<llvm::MemoryBuffer> ModuleMapBuffer =
       llvm::MemoryBuffer::getMemBufferCopy(InferredModuleMapContent);