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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/136601.diff


2 Files Affected:

- (modified) clang/include/clang/Frontend/CompilerInstance.h (+11-4) 
- (modified) clang/lib/Frontend/CompilerInstance.cpp (+15-8) 


``````````diff
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<CompilerInstance> cloneForModuleCompileImpl(
       SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input,
       StringRef OriginalModuleMapFile, StringRef ModuleFileName,
-      IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
+      IntrusiveRefCntPtr<llvm::vfs::FileSystem> 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<CompilerInstance> cloneForModuleCompile(
-      SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName,
-      IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
+  /// Explicitly-specified \c DiagConsumer takes precedence over forwarding to
+  /// this instance.
+  std::unique_ptr<CompilerInstance>
+  cloneForModuleCompile(SourceLocation ImportLoc, Module *Module,
+                        StringRef ModuleFileName,
+                        IntrusiveRefCntPtr<llvm::vfs::FileSystem> 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> CompilerInstance::cloneForModuleCompileImpl(
     SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input,
     StringRef OriginalModuleMapFile, StringRef ModuleFileName,
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+    DiagnosticConsumer *DiagConsumer) {
   // Construct a compiler invocation for creating this module.
   auto Invocation = std::make_shared<CompilerInvocation>(getInvocation());
 
@@ -1221,10 +1222,15 @@ std::unique_ptr<CompilerInstance> 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> CompilerInstance::cloneForModuleCompile(
     SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName,
-    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+    DiagnosticConsumer *DiagConsumer) {
   StringRef ModuleName = Module->getTopLevelModuleName();
 
   InputKind IK(getLanguageFromOptions(getLangOpts()), InputKind::ModuleMap);
@@ -1368,7 +1375,7 @@ std::unique_ptr<CompilerInstance> 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> CompilerInstance::cloneForModuleCompile(
       ImportLoc, ModuleName,
       FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem),
       ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName,
-      std::move(VFS));
+      std::move(VFS), DiagConsumer);
 
   std::unique_ptr<llvm::MemoryBuffer> ModuleMapBuffer =
       llvm::MemoryBuffer::getMemBufferCopy(InferredModuleMapContent);

``````````

</details>


https://github.com/llvm/llvm-project/pull/136601