[Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer in the compiler

[arphaman]

Summary

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.

[arphaman]

+ cfe-commits

[Bigcheese]

LGTM

cfe/trunk/lib/Frontend/CompilerInstance.cpp

if (DiagOpts->DiagnosticLogFile != "-") {
     }
   }
 
   // Chain in the diagnostic client which will log the diagnostics.
   auto Logger = llvm::make_unique<LogDiagnosticPrinter>(*OS, DiagOpts,
                                                         std::move(StreamOwner));
   if (CodeGenOpts)
     Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);
-  assert(Diags.ownsClient());
+  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,
                                        DiagnosticsEngine &Diags,
                                        StringRef OutputFile) {
   auto SerializedConsumer =
       clang::serialized_diags::create(OutputFile, DiagOpts);
 

cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp

 //===- unittests/Frontend/CompilerInstanceTest.cpp - CI tests -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 using namespace clang;
 
TEST(CompilerInstance, DefaultVFSOverlayFromInvocation) {
   Instance.setInvocation(CInvok);
   Instance.createFileManager();
 
   // Check if the virtual file exists which means that our VFS is used by the
   // CompilerInstance.
   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