r363009 - [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Mon Jun 10 16:32:42 2019
New Revision: 363009

URL: http://llvm.org/viewvc/llvm-project?rev=363009&view=rev
Log:
[Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer
in the compiler

The function SetUpDiagnosticLog that was called from createDiagnostics didn't
handle the case where the diagnostics engine didn't own the diagnostics consumer.
This is a potential problem for a clang tool, in particular some of the follow-up
patches for clang-scan-deps will need this fix.

Differential Revision: https://reviews.llvm.org/D63101

Modified:
    cfe/trunk/lib/Frontend/CompilerInstance.cpp
    cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=363009&r1=363008&r2=363009&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 10 16:32:42 2019
@@ -232,9 +232,13 @@ static void SetUpDiagnosticLog(Diagnosti
                                                         std::move(StreamOwner));
   if (CodeGenOpts)
     Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);
-  assert(Diags.ownsClient());
-  Diags.setClient(
-      new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));
+  if (Diags.ownsClient()) {
+    Diags.setClient(
+        new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));
+  } else {
+    Diags.setClient(
+        new ChainedDiagnosticConsumer(Diags.getClient(), std::move(Logger)));
+  }
 }
 
 static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,

Modified: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp?rev=363009&r1=363008&r2=363009&view=diff
==============================================================================
--- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp (original)
+++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp Mon Jun 10 16:32:42 2019
@@ -8,6 +8,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -70,4 +71,21 @@ TEST(CompilerInstance, DefaultVFSOverlay
   ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));
 }
 
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  auto DiagOpts = new DiagnosticOptions();
+  DiagOpts->DiagnosticLogFile = "log.diags";
+
+  // Create the diagnostic engine with unowned consumer.
+  std::string DiagnosticOutput;
+  llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
+  auto DiagPrinter = llvm::make_unique<TextDiagnosticPrinter>(
+      DiagnosticsOS, new DiagnosticOptions());
+  CompilerInstance Instance;
+  IntrusiveRefCntPtr<DiagnosticsEngine> Diags = Instance.createDiagnostics(
+      DiagOpts, DiagPrinter.get(), /*ShouldOwnClient=*/false);
+
+  Diags->Report(diag::err_expected) << "no crash";
+  ASSERT_EQ(DiagnosticsOS.str(), "error: expected no crash\n");
+}
+
 } // anonymous namespace