Add VerboseOutputStream to CompilerInstance

[scott.linder]

Summary

Remove one instance of a hardcoded output stream in CompilerInstance::ExecuteAction. There are still other cases of output being hard-coded to standard streams in ExecuteCompilerInvocation, but this patch covers the case when no flags like -version or -help are passed, namely the "X warnings and Y errors generated." diagnostic.

[scott.linder]

Ping

[scott.linder]

Ping

[scott.linder]

Ping

[scott.linder]

Rebase and ping

[scott.linder]

Actually rebase this time

[dblaikie]

include/clang/Frontend/CompilerInstance.h

158–159

Rather than a bool, this could be a unique_ptr, perhaps? (null when non-owning)

362–363

Two functions - one that takes a raw_ostream& and another that takes a unique_ptr<raw_ostream>?

[scott.linder]

include/clang/Frontend/CompilerInstance.h

362–363

Do both overloads transfer ownership? Are the "standard" streams like llvm::outs() OK to free?

[scott.linder]

After reading again I think I understand the ask now. Is this closer to what you had in mind?

[scott.linder]

Same patch, this time with a working test.

[scott.linder]

Ping

[scott.linder]

Ping

[arsenm]

LGTM

include/clang/Frontend/CompilerInstance.h

class CompilerInstance : public ModuleLoader {
   bool BuildGlobalModuleIndex = false;
 
   /// We have a full global module index, with all modules.
   bool HaveFullGlobalModuleIndex = false;
 
   /// One or more modules failed to build.
   bool ModuleBuildFailed = false;
 
+  /// The stream for verbose output if owned, otherwise nullptr.
+  std::unique_ptr<raw_ostream> OwnedVerboseOutputStream;
+
+  /// The stream for verbose output.
+  raw_ostream *VerboseOutputStream = &llvm::errs();
+
   /// Holds information about the output file.
   ///
   /// If TempFilename is not empty we must rename it to Filename at the end.
   /// TempFilename may be empty and Filename non-empty if creating the temporary
   /// failed.
   struct OutputFile {
     std::string Filename;
     std::string TempFilename;
public:
   ///  - Clients should eventually call llvm_shutdown() upon the completion of
   ///    this routine to ensure that any managed objects are properly destroyed.
   ///
   /// Note that this routine may write output to 'stderr'.
   ///
   /// \param Act - The action to execute.
   /// \return - True on success.
   //
-  // FIXME: This function should take the stream to write any debugging /
-  // verbose output to as an argument.
-  //
   // FIXME: Eliminate the llvm_shutdown requirement, that should either be part
   // of the context or else not CompilerInstance specific.
   bool ExecuteAction(FrontendAction &Act);
 
   /// }
   /// @name Compiler Invocation and Options
   /// {
 
public:
 
   DiagnosticConsumer &getDiagnosticClient() const {
     assert(Diagnostics && Diagnostics->getClient() &&
            "Compiler instance has no diagnostic client!");
     return *Diagnostics->getClient();
   }
 
   /// }
+  /// @name VerboseOutputStream
+  /// }
+
+  /// Replace the current stream for verbose output.
+  void setVerboseOutputStream(raw_ostream &Value);
+
+  /// Replace the current stream for verbose output.
+  void setVerboseOutputStream(std::unique_ptr<raw_ostream> Value);
+
+  /// Get the current stream for verbose output.
+  raw_ostream &getVerboseOutputStream() {
+    return *VerboseOutputStream;
+  }
+
+  /// }
   /// @name Target Info
   /// {
 
   bool hasTarget() const { return Target != nullptr; }
 
   TargetInfo &getTarget() const {
     assert(Target && "Compiler instance has no target!");
     return *Target;

lib/Frontend/CompilerInstance.cpp

return (BuildGlobalModuleIndex ||
            getFrontendOpts().GenerateGlobalModuleIndex)) &&
          !ModuleBuildFailed;
 }
 
 void CompilerInstance::setDiagnostics(DiagnosticsEngine *Value) {
   Diagnostics = Value;
 }
 
+void CompilerInstance::setVerboseOutputStream(raw_ostream &Value) {
+  OwnedVerboseOutputStream.release();
+  VerboseOutputStream = &Value;
+}
+
+void CompilerInstance::setVerboseOutputStream(std::unique_ptr<raw_ostream> Value) {
+  OwnedVerboseOutputStream.swap(Value);
+  VerboseOutputStream = OwnedVerboseOutputStream.get();
+}
+
 void CompilerInstance::setTarget(TargetInfo *Value) { Target = Value; }
 void CompilerInstance::setAuxTarget(TargetInfo *Value) { AuxTarget = Value; }
 
 void CompilerInstance::setFileManager(FileManager *Value) {
   FileMgr = Value;
 }
 
 void CompilerInstance::setSourceManager(SourceManager *Value) {
bool CompilerInstance::ExecuteAction(FrontendAction &Act) {
   assert(!getFrontendOpts().ShowHelp && "Client must handle '-help'!");
   assert(!getFrontendOpts().ShowVersion && "Client must handle '-version'!");
 
   // Mark this point as the bottom of the stack if we don't have somewhere
   // better. We generally expect frontend actions to be invoked with (nearly)
   // DesiredStackSpace available.
   noteBottomOfStack();
 
-  // FIXME: Take this as an argument, once all the APIs we used have moved to
-  // taking it as an input instead of hard-coding llvm::errs.
-  raw_ostream &OS = llvm::errs();
+  raw_ostream &OS = getVerboseOutputStream();
 
   if (!Act.PrepareToExecute(*this))
     return false;
 
   // Create the target instance.
   setTarget(TargetInfo::CreateTargetInfo(getDiagnostics(),
                                          getInvocation().TargetOpts));
   if (!hasTarget())

unittests/Frontend/OutputStreamTest.cpp

 //===- unittests/Frontend/OutputStreamTest.cpp --- FrontendAction 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/Basic/LangStandard.h"
 #include "clang/CodeGen/BackendUtil.h"
 #include "clang/CodeGen/CodeGenAction.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/FrontendTool/Utils.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 using namespace clang;
 using namespace clang::frontend;
 
TEST(FrontendOutputTests, TestOutputStream) {
   Compiler.setInvocation(std::move(Invocation));
   Compiler.createDiagnostics();
 
   bool Success = ExecuteCompilerInvocation(&Compiler);
   EXPECT_TRUE(Success);
   EXPECT_TRUE(!IRBuffer.empty());
   EXPECT_TRUE(StringRef(IRBuffer.data()).startswith("BC"));
 }
+
+TEST(FrontendOutputTests, TestVerboseOutputStreamShared) {
+  auto Invocation = std::make_shared<CompilerInvocation>();
+  Invocation->getPreprocessorOpts().addRemappedFile(
+      "test.cc", MemoryBuffer::getMemBuffer("invalid").release());
+  Invocation->getFrontendOpts().Inputs.push_back(
+      FrontendInputFile("test.cc", Language::CXX));
+  Invocation->getFrontendOpts().ProgramAction = EmitBC;
+  Invocation->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+  CompilerInstance Compiler;
+
+  std::string VerboseBuffer;
+  raw_string_ostream VerboseStream(VerboseBuffer);
+
+  Compiler.setInvocation(std::move(Invocation));
+  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+  Compiler.createDiagnostics(
+      new TextDiagnosticPrinter(llvm::nulls(), &*DiagOpts), true);
+  Compiler.setVerboseOutputStream(VerboseStream);
+
+  bool Success = ExecuteCompilerInvocation(&Compiler);
+  EXPECT_FALSE(Success);
+  EXPECT_TRUE(!VerboseStream.str().empty());
+  EXPECT_TRUE(StringRef(VerboseBuffer.data()).contains("errors generated"));
+}
+
+TEST(FrontendOutputTests, TestVerboseOutputStreamOwned) {
+  std::string VerboseBuffer;
+  bool Success;
+  {
+    auto Invocation = std::make_shared<CompilerInvocation>();
+    Invocation->getPreprocessorOpts().addRemappedFile(
+        "test.cc", MemoryBuffer::getMemBuffer("invalid").release());
+    Invocation->getFrontendOpts().Inputs.push_back(
+        FrontendInputFile("test.cc", Language::CXX));
+    Invocation->getFrontendOpts().ProgramAction = EmitBC;
+    Invocation->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+    CompilerInstance Compiler;
+
+    std::unique_ptr<raw_ostream> VerboseStream =
+        std::make_unique<raw_string_ostream>(VerboseBuffer);
+
+    Compiler.setInvocation(std::move(Invocation));
+    IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+    Compiler.createDiagnostics(
+        new TextDiagnosticPrinter(llvm::nulls(), &*DiagOpts), true);
+    Compiler.setVerboseOutputStream(std::move(VerboseStream));
+
+    Success = ExecuteCompilerInvocation(&Compiler);
+  }
+  EXPECT_FALSE(Success);
+  EXPECT_TRUE(!VerboseBuffer.empty());
+  EXPECT_TRUE(StringRef(VerboseBuffer.data()).contains("errors generated"));
+}
 }