[llvm-readobj] Consistent use of ScopedPrinter

[sbc100]

Summary

There were a few places where outs() was being used
directly rather than the ScopedPrinter object.

[sbc100]

• one more cleanup

[zturner]

tools/llvm-readobj/StackMapPrinter.h

32–42

I think you want to remove all these \n characters right? The previous code was basically printing one string on each line, but in the new version each string is terminated with \n, and then another line is started immediately after by calling W.startLine();

[sbc100]

tools/llvm-readobj/StackMapPrinter.h

32–42

IIUC startLine() only adds indentention, its doesn't add newlines, so any line that is started with startLine() still needs to end in a "\n".

I think the output is unchanged, otherwise test/Object/stackmap-dump.test would be broken.

[sbc100]

friendly ping..

[zturner]

Sorry about that! lgtm

include/llvm/Support/ScopedPrinter.h

public:
 
   void printString(StringRef Value) { startLine() << Value << "\n"; }
 
   void printString(StringRef Label, StringRef Value) {
     startLine() << Label << ": " << Value << "\n";
   }
 
   void printString(StringRef Label, const std::string &Value) {
-    startLine() << Label << ": " << Value << "\n";
+    printString(Label, StringRef(Value));
+  }
+
+  void printString(StringRef Label, const char* Value) {
+    printString(Label, StringRef(Value));
   }
 
   template <typename T>
   void printNumber(StringRef Label, StringRef Str, T Value) {
     startLine() << Label << ": " << Str << " (" << Value << ")\n";
   }
 
   void printBinary(StringRef Label, StringRef Str, ArrayRef<uint8_t> Value) {

tools/llvm-readobj/COFFDumper.cpp

void COFFDumper::printStackMap() const {
   StringRef StackMapContents;
   StackMapSection.getContents(StackMapContents);
   ArrayRef<uint8_t> StackMapContentsArray(
       reinterpret_cast<const uint8_t*>(StackMapContents.data()),
       StackMapContents.size());
 
   if (Obj->isLittleEndian())
     prettyPrintStackMap(
-                      llvm::outs(),
-                      StackMapV2Parser<support::little>(StackMapContentsArray));
+        W, StackMapV2Parser<support::little>(StackMapContentsArray));
   else
-    prettyPrintStackMap(llvm::outs(),
+    prettyPrintStackMap(W,
                         StackMapV2Parser<support::big>(StackMapContentsArray));
 }
 
 void llvm::dumpCodeViewMergedTypes(
     ScopedPrinter &Writer, llvm::codeview::MergingTypeTableBuilder &IDTable,
     llvm::codeview::MergingTypeTableBuilder &CVTypes) {
   // Flatten it first, then run our dumper on it.
   SmallString<0> TypeBuf;

tools/llvm-readobj/COFFImportDumper.cpp

 /// \file
 /// \brief This file implements the COFF import library dumper for llvm-readobj.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/COFFImportFile.h"
+#include "llvm/Support/ScopedPrinter.h"
 
 using namespace llvm::object;
 
 namespace llvm {
 
-void dumpCOFFImportFile(const COFFImportFile *File) {
-  outs() << '\n';
-  outs() << "File: " << File->getFileName() << "\n";
-  outs() << "Format: COFF-import-file\n";
+void dumpCOFFImportFile(const COFFImportFile *File, ScopedPrinter &Writer) {
+  Writer.startLine() << '\n';
+  Writer.printString("File", File->getFileName());
+  Writer.printString("Format", "COFF-import-file");
 
   const coff_import_header *H = File->getCOFFImportHeader();
   switch (H->getType()) {
-  case COFF::IMPORT_CODE:  outs() << "Type: code\n"; break;
-  case COFF::IMPORT_DATA:  outs() << "Type: data\n"; break;
-  case COFF::IMPORT_CONST: outs() << "Type: const\n"; break;
+  case COFF::IMPORT_CODE:  Writer.printString("Type", "code"); break;
+  case COFF::IMPORT_DATA:  Writer.printString("Type", "data"); break;
+  case COFF::IMPORT_CONST: Writer.printString("Type", "const"); break;
   }
 
   switch (H->getNameType()) {
-  case COFF::IMPORT_ORDINAL: outs() << "Name type: ordinal\n"; break;
-  case COFF::IMPORT_NAME: outs() << "Name type: name\n"; break;
-  case COFF::IMPORT_NAME_NOPREFIX: outs() << "Name type: noprefix\n"; break;
-  case COFF::IMPORT_NAME_UNDECORATE: outs() << "Name type: undecorate\n"; break;
+  case COFF::IMPORT_ORDINAL:
+    Writer.printString("Name type", "ordinal");
+    break;
+  case COFF::IMPORT_NAME:
+    Writer.printString("Name type", "name");
+    break;
+  case COFF::IMPORT_NAME_NOPREFIX:
+    Writer.printString("Name type", "noprefix");
+    break;
+  case COFF::IMPORT_NAME_UNDECORATE:
+    Writer.printString("Name type", "undecorate");
+    break;
   }
 
   for (const object::BasicSymbolRef &Sym : File->symbols()) {
-    outs() << "Symbol: ";
-    Sym.printName(outs());
-    outs() << "\n";
+    raw_ostream &OS = Writer.startLine();
+    OS << "Symbol: ";
+    Sym.printName(OS);
+    OS << "\n";
   }
 }
 
 } // namespace llvm

tools/llvm-readobj/ELFDumper.cpp

void ELFDumper<ELFT>::printNeededLibraries() {
   LibsTy Libs;
 
   for (const auto &Entry : dynamic_table())
     if (Entry.d_tag == ELF::DT_NEEDED)
       Libs.push_back(getDynamicString(Entry.d_un.d_val));
 
   std::stable_sort(Libs.begin(), Libs.end());
 
-  for (const auto &L : Libs) {
-    outs() << "  " << L << "\n";
-  }
+  for (const auto &L : Libs)
+     W.startLine() << L << "\n";
 }
 
 
 template <typename ELFT>
 void ELFDumper<ELFT>::printHashTable() {
   DictScope D(W, "HashTable");
   if (!HashTable)
     return;
void ELFDumper<ELFT>::printGnuHashTable() {
   Elf_Sym_Range Syms = dynamic_symbols();
   unsigned NumSyms = std::distance(Syms.begin(), Syms.end());
   if (!NumSyms)
     reportError("No dynamic symbol section");
   W.printHexList("Values", GnuHashTable->values(NumSyms));
 }
 
 template <typename ELFT> void ELFDumper<ELFT>::printLoadName() {
-  outs() << "LoadName: " << SOName << '\n';
+  W.printString("LoadName", SOName);
 }
 
 template <class ELFT>
 void ELFDumper<ELFT>::printAttributes() {
   W.startLine() << "Attributes not implemented.\n";
 }
 
 namespace {
template <class ELFT> void ELFDumper<ELFT>::printStackMap() const {
   }
 
   if (!StackMapSection)
     return;
 
   ArrayRef<uint8_t> StackMapContentsArray =
       unwrapOrError(Obj->getSectionContents(StackMapSection));
 
-  prettyPrintStackMap(outs(), StackMapV2Parser<ELFT::TargetEndianness>(
-                                  StackMapContentsArray));
+  prettyPrintStackMap(
+      W, StackMapV2Parser<ELFT::TargetEndianness>(StackMapContentsArray));
 }
 
 template <class ELFT> void ELFDumper<ELFT>::printGroupSections() {
   ELFDumperStyle->printGroupSections(Obj);
 }
 
 static inline void printFields(formatted_raw_ostream &OS, StringRef Str1,
                                StringRef Str2) {

tools/llvm-readobj/MachODumper.cpp

void MachODumper::printStackMap() const {
 
   StringRef StackMapContents;
   StackMapSection.getContents(StackMapContents);
   ArrayRef<uint8_t> StackMapContentsArray(
       reinterpret_cast<const uint8_t*>(StackMapContents.data()),
       StackMapContents.size());
 
   if (Obj->isLittleEndian())
-     prettyPrintStackMap(
-                      llvm::outs(),
-                      StackMapV2Parser<support::little>(StackMapContentsArray));
+        W, StackMapV2Parser<support::little>(StackMapContentsArray));
   else
-     prettyPrintStackMap(llvm::outs(),
-                         StackMapV2Parser<support::big>(StackMapContentsArray));
+    prettyPrintStackMap(W,
 }
 
 void MachODumper::printMachODataInCode() {
   for (const auto &Load : Obj->load_commands()) {
     if (Load.C.cmd  == MachO::LC_DATA_IN_CODE) {
       MachO::linkedit_data_command LLC = Obj->getLinkeditDataLoadCommand(Load);
       DictScope Group(W, "DataInCode");
       W.printNumber("Data offset", LLC.dataoff);

tools/llvm-readobj/ObjDumper.h

 std::error_code createMachODumper(const object::ObjectFile *Obj,
                                   ScopedPrinter &Writer,
                                   std::unique_ptr<ObjDumper> &Result);
 
 std::error_code createWasmDumper(const object::ObjectFile *Obj,
                                  ScopedPrinter &Writer,
                                  std::unique_ptr<ObjDumper> &Result);
 
-void dumpCOFFImportFile(const object::COFFImportFile *File);
+void dumpCOFFImportFile(const object::COFFImportFile *File,
+                        ScopedPrinter &Writer);
 
 void dumpCodeViewMergedTypes(
     ScopedPrinter &Writer, llvm::codeview::MergingTypeTableBuilder &IDTable,
     llvm::codeview::MergingTypeTableBuilder &TypeTable);
 
 } // namespace llvm
 
 #endif

tools/llvm-readobj/StackMapPrinter.h

 //===-------- StackMapPrinter.h - Pretty-print stackmaps --------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TOOLS_LLVM_READOBJ_STACKMAPPRINTER_H
 #define LLVM_TOOLS_LLVM_READOBJ_STACKMAPPRINTER_H
 
 #include "llvm/Object/StackMapParser.h"
+#include "llvm/Support/ScopedPrinter.h"
 
 namespace llvm {
 
 // Pretty print a stackmap to the given ostream.
-template <typename OStreamT, typename StackMapParserT>
-void prettyPrintStackMap(OStreamT &OS, const StackMapParserT &SMP) {
+template <typename StackMapParserT>
+void prettyPrintStackMap(ScopedPrinter &W, const StackMapParserT &SMP) {
 
-  OS << "LLVM StackMap Version: " << SMP.getVersion()
-     << "\nNum Functions: " << SMP.getNumFunctions();
+  W.printNumber("LLVM StackMap Version",  SMP.getVersion());
+  W.printNumber("Num Functions", SMP.getNumFunctions());
 
   // Functions:
   for (const auto &F : SMP.functions())
-    OS << "\n  Function address: " << F.getFunctionAddress()
+    W.startLine() << "  Function address: " << F.getFunctionAddress()
        << ", stack size: " << F.getStackSize()
-       << ", callsite record count: " << F.getRecordCount();
+       << ", callsite record count: " << F.getRecordCount() << "\n";
 
   // Constants:
-  OS << "\nNum Constants: " << SMP.getNumConstants();
+  W.printNumber("Num Constants", SMP.getNumConstants());
   unsigned ConstantIndex = 0;
   for (const auto &C : SMP.constants())
-    OS << "\n  #" << ++ConstantIndex << ": " << C.getValue();
+    W.startLine() << "  #" << ++ConstantIndex << ": " << C.getValue() << "\n";
 
   // Records:
-  OS << "\nNum Records: " << SMP.getNumRecords();
+  W.printNumber("Num Records", SMP.getNumRecords());
   for (const auto &R : SMP.records()) {
-    OS << "\n  Record ID: " << R.getID()
+    W.startLine() << "  Record ID: " << R.getID()
                   << ", instruction offset: " << R.getInstructionOffset()
-       << "\n    " << R.getNumLocations() << " locations:";
+                  << "\n";
+    W.startLine() << "    " << R.getNumLocations() << " locations:\n";
 
     unsigned LocationIndex = 0;
     for (const auto &Loc : R.locations()) {
-      OS << "\n      #" << ++LocationIndex << ": ";
+      raw_ostream &OS = W.startLine();
+      OS << "      #" << ++LocationIndex << ": ";
       switch (Loc.getKind()) {
       case StackMapParserT::LocationKind::Register:
-        OS << "Register R#" << Loc.getDwarfRegNum();
+        OS << "Register R#" << Loc.getDwarfRegNum() << "\n";
         break;
       case StackMapParserT::LocationKind::Direct:
-        OS << "Direct R#" << Loc.getDwarfRegNum() << " + "
-           << Loc.getOffset();
+        OS << "Direct R#" << Loc.getDwarfRegNum() << " + " << Loc.getOffset()
+           << "\n";
         break;
       case StackMapParserT::LocationKind::Indirect:
-        OS << "Indirect [R#" << Loc.getDwarfRegNum() << " + "
-           << Loc.getOffset() << "]";
+        OS << "Indirect [R#" << Loc.getDwarfRegNum() << " + " << Loc.getOffset()
+           << "]\n";
         break;
       case StackMapParserT::LocationKind::Constant:
-        OS << "Constant " << Loc.getSmallConstant();
+        OS << "Constant " << Loc.getSmallConstant() << "\n";
         break;
       case StackMapParserT::LocationKind::ConstantIndex:
         OS << "ConstantIndex #" << Loc.getConstantIndex() << " ("
-           << SMP.getConstant(Loc.getConstantIndex()).getValue() << ")";
+           << SMP.getConstant(Loc.getConstantIndex()).getValue() << ")\n";
         break;
       }
     }
 
-    OS << "\n    " << R.getNumLiveOuts() << " live-outs: [ ";
+    raw_ostream &OS = W.startLine();
+    OS << "    " << R.getNumLiveOuts() << " live-outs: [ ";
     for (const auto &LO : R.liveouts())
       OS << "R#" << LO.getDwarfRegNum() << " ("
          << LO.getSizeInBytes() << "-bytes) ";
     OS << "]\n";
   }
-
- OS << "\n";
-
 }
 
 }
 
 #endif

tools/llvm-readobj/llvm-readobj.cpp

 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/WindowsResource.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetRegistry.h"
 
 using namespace llvm;
if (Obj->isMachO())
     return createMachODumper(Obj, Writer, Result);
   if (Obj->isWasm())
     return createWasmDumper(Obj, Writer, Result);
 
   return readobj_error::unsupported_obj_file_format;
 }
 
 /// @brief Dumps the specified object file.
-static void dumpObject(const ObjectFile *Obj) {
-  ScopedPrinter Writer(outs());
+static void dumpObject(const ObjectFile *Obj, ScopedPrinter &Writer) {
   std::unique_ptr<ObjDumper> Dumper;
   if (std::error_code EC = createDumper(Obj, Writer, Dumper))
     reportError(Obj->getFileName(), EC);
 
   if (opts::Output == opts::LLVM) {
-    outs() << '\n';
-    outs() << "File: " << Obj->getFileName() << "\n";
-    outs() << "Format: " << Obj->getFileFormatName() << "\n";
-    outs() << "Arch: " << Triple::getArchTypeName(
-                              (llvm::Triple::ArchType)Obj->getArch()) << "\n";
-    outs() << "AddressSize: " << (8 * Obj->getBytesInAddress()) << "bit\n";
+    Writer.startLine() << "\n";
+    Writer.printString("File", Obj->getFileName());
+    Writer.printString("Format", Obj->getFileFormatName());
+    Writer.printString("Arch", Triple::getArchTypeName(
+                                   (llvm::Triple::ArchType)Obj->getArch()));
+    Writer.printString("AddressSize",
+                       formatv("{0}bit", 8 * Obj->getBytesInAddress()));
     Dumper->printLoadName();
   }
 
   if (opts::FileHeaders)
     Dumper->printFileHeaders();
   if (opts::Sections)
     Dumper->printSections();
   if (opts::Relocations)
if (Obj->isMachO()) {
     if (opts::MachODysymtab)
       Dumper->printMachODysymtab();
   }
   if (opts::PrintStackMap)
     Dumper->printStackMap();
 }
 
 /// @brief Dumps each object file in \a Arc;
-static void dumpArchive(const Archive *Arc) {
+static void dumpArchive(const Archive *Arc, ScopedPrinter &Writer) {
   Error Err = Error::success();
   for (auto &Child : Arc->children(Err)) {
     Expected<std::unique_ptr<Binary>> ChildOrErr = Child.getAsBinary();
     if (!ChildOrErr) {
       if (auto E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError())) {
         reportError(Arc->getFileName(), ChildOrErr.takeError());
       }
       continue;
     }
     if (ObjectFile *Obj = dyn_cast<ObjectFile>(&*ChildOrErr.get()))
-      dumpObject(Obj);
+      dumpObject(Obj, Writer);
     else if (COFFImportFile *Imp = dyn_cast<COFFImportFile>(&*ChildOrErr.get()))
-      dumpCOFFImportFile(Imp);
+      dumpCOFFImportFile(Imp, Writer);
     else
       reportError(Arc->getFileName(), readobj_error::unrecognized_file_format);
   }
   if (Err)
     reportError(Arc->getFileName(), std::move(Err));
 }
 
 /// @brief Dumps each object file in \a MachO Universal Binary;
-static void dumpMachOUniversalBinary(const MachOUniversalBinary *UBinary) {
+static void dumpMachOUniversalBinary(const MachOUniversalBinary *UBinary,
+                                     ScopedPrinter &Writer) {
   for (const MachOUniversalBinary::ObjectForArch &Obj : UBinary->objects()) {
     Expected<std::unique_ptr<MachOObjectFile>> ObjOrErr = Obj.getAsObjectFile();
     if (ObjOrErr)
-      dumpObject(&*ObjOrErr.get());
+      dumpObject(&*ObjOrErr.get(), Writer);
     else if (auto E = isNotObjectErrorInvalidFileType(ObjOrErr.takeError())) {
       reportError(UBinary->getFileName(), ObjOrErr.takeError());
     }
     else if (Expected<std::unique_ptr<Archive>> AOrErr = Obj.getAsArchive())
-      dumpArchive(&*AOrErr.get());
+      dumpArchive(&*AOrErr.get(), Writer);
   }
 }
 
 /// @brief Dumps \a WinRes, Windows Resource (.res) file;
 static void dumpWindowsResourceFile(WindowsResource *WinRes) {
   ScopedPrinter Printer{outs()};
   WindowsRes::Dumper Dumper(WinRes, Printer);
   if (auto Err = Dumper.printData())
     reportError(WinRes->getFileName(), std::move(Err));
 }
 
 
 /// @brief Opens \a File and dumps it.
 static void dumpInput(StringRef File) {
+  ScopedPrinter Writer(outs());
 
   // Attempt to open the binary.
   Expected<OwningBinary<Binary>> BinaryOrErr = createBinary(File);
   if (!BinaryOrErr)
     reportError(File, BinaryOrErr.takeError());
   Binary &Binary = *BinaryOrErr.get().getBinary();
 
   if (Archive *Arc = dyn_cast<Archive>(&Binary))
-    dumpArchive(Arc);
+    dumpArchive(Arc, Writer);
   else if (MachOUniversalBinary *UBinary =
                dyn_cast<MachOUniversalBinary>(&Binary))
-    dumpMachOUniversalBinary(UBinary);
+    dumpMachOUniversalBinary(UBinary, Writer);
   else if (ObjectFile *Obj = dyn_cast<ObjectFile>(&Binary))
-    dumpObject(Obj);
+    dumpObject(Obj, Writer);
   else if (COFFImportFile *Import = dyn_cast<COFFImportFile>(&Binary))
-    dumpCOFFImportFile(Import);
+    dumpCOFFImportFile(Import, Writer);
   else if (WindowsResource *WinRes = dyn_cast<WindowsResource>(&Binary))
     dumpWindowsResourceFile(WinRes);
   else
     reportError(File, readobj_error::unrecognized_file_format);
 }
 
 int main(int argc, const char *argv[]) {
   StringRef ToolName = argv[0];