Allow specification of what kinds of class members to dump

[zturner]

Summary

Now instead of just no-class-definitions, we can get a little more granular. This is now an enum, with 3 possible values: 1) None - Don't display any class definitions. This is equivalent to specifying -no-class-definitions before. 2) Layout - Displays only class members which affect class layout -- e.g. data members. All functions, typedefs, nested classes, etc will be suppressed. 3) Full - Displays everything. This is equivalent to the default behavior before this patch, and is still the default behavior after this patch.

[zturner]

Slightly better logic in layout mode. It was dumping constants and static members before which don't occupy any space in a record.

[amccarth]

LGTM.

Clang-format did some awkward comment wrapping in some of the new code.

When the Native reader catches up, we'll have to update the test config to run the tests on all platforms, which may require the -output-color=false options, but that'll probably be my problem later.

llvm/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp

80

Can you fix the wrapping on these comments?

llvm/test/DebugInfo/PDB/DIA/pdbdump-symbol-format.test

 ; TYPES_FORMAT-DAG: enum TestEnum
 ; TYPES_FORMAT-DAG: enum TestEnumClass
 ; TYPES_FORMAT: Typedefs
 ; TYPES_FORMAT-DAG: typedef int IntType
 ; TYPES_FORMAT-DAG: typedef class A ClassAType
 
 ; TYPES_1: Classes
 ; TYPES_1: struct A {
-; TYPES_1: public:
 ; TYPES_1: virtual void PureFunc() = 0
 ; TYPES_1: virtual void VirtualFunc()
 ; TYPES_1: void RegularFunc()
 ; TYPES_1: }
 
 ; TYPES_2: Classes
 ; TYPES_2: struct MemberTest {
 ; TYPES_2: data +0x00 MemberTest::NestedEnum m_nested_enum

llvm/test/tools/llvm-pdbdump/class-layout.test

 ; GLOBALS_TEST-DAG: int GlobalsTest::IntVar
 ; GLOBALS_TEST-DAG: double GlobalsTest::DoubleVar
 ; GLOBALS_TEST-DAG: GlobalsTest::Enum GlobalsTest::EnumVar
 
 ; MEMBERS_TEST: ---TYPES---
 ; MEMBERS_TEST: class MembersTest::A {
 ; MEMBERS_TEST-DAG: typedef int NestedTypedef
 ; MEMBERS_TEST-DAG: enum NestedEnum
-; MEMBERS_TEST: public:
-; MEMBERS_TEST-NEXT: void MemberFunc()
-; MEMBERS_TEST-NEXT: private:
+; MEMBERS_TEST: void MemberFunc()
 ; MEMBERS_TEST-DAG: int IntMemberVar
 ; MEMBERS_TEST-DAG: double DoubleMemberVar
 ; MEMBERS_TEST: }
 
 ; BASE_CLASS_A: ---TYPES---
 ; BASE_CLASS_A: class BaseClassTest::A {}
 
 ; BASE_CLASS_B: ---TYPES---

 
 ; UDT_KIND_TEST: ---TYPES---
 ; UDT_KIND_TEST-DAG: union UdtKindTest::C {}
 ; UDT_KIND_TEST-DAG: class UdtKindTest::B {}
 ; UDT_KIND_TEST-DAG: struct UdtKindTest::A {}
 
 ; BITFIELD_TEST: ---TYPES---
 ; BITFIELD_TEST: struct BitFieldTest::A {
-; BITFIELD_TEST-NEXT: public:
 ; BITFIELD_TEST-NEXT: +0x00 int Bits1 : 1
 ; BITFIELD_TEST-NEXT: +0x00 int Bits2 : 2
 ; BITFIELD_TEST-NEXT: +0x00 int Bits3 : 3
 ; BITFIELD_TEST-NEXT: +0x00 int Bits4 : 4
 ; BITFIELD_TEST-NEXT: +0x00 int Bits22 : 22
 ; BITFIELD_TEST-NEXT: +0x04 int Offset0x04

llvm/test/tools/llvm-pdbdump/regex-filter.test

 ; RUN: llvm-pdbdump pretty -symbols -globals -types %p/Inputs/FilterTest.pdb \
 ; RUN:    | FileCheck --check-prefix=NO_FILTER %s
+
 ; RUN: llvm-pdbdump pretty -types -exclude-types="GlobalTypedef|NestedTypedef" \
 ; RUN:    %p/Inputs/FilterTest.pdb | FileCheck --check-prefix=EXCLUDE_TYPEDEFS %s
+; RUN: llvm-pdbdump pretty -classes -enums %p/Inputs/FilterTest.pdb \
+; RUN:    | FileCheck --check-prefix=EXCLUDE_TYPEDEFS %s
+
 ; RUN: llvm-pdbdump pretty -types -exclude-types="GlobalEnum|NestedEnum" \
 ; RUN:    %p/Inputs/FilterTest.pdb | FileCheck --check-prefix=EXCLUDE_ENUMS %s
+; RUN: llvm-pdbdump pretty -classes -typedefs %p/Inputs/FilterTest.pdb \
+; RUN:    | FileCheck --check-prefix=EXCLUDE_ENUMS %s
+
 ; RUN: llvm-pdbdump pretty -types -symbols -globals -exclude-symbols="MemberVar|GlobalVar" \
 ; RUN:    %p/Inputs/FilterTest.pdb | FileCheck --check-prefix=EXCLUDE_VARS %s
 ; RUN: llvm-pdbdump pretty -types -exclude-types="FilterTestClass" \
 ; RUN:    %p/Inputs/FilterTest.pdb | FileCheck  --check-prefix=EXCLUDE_WHOLE_CLASS %s
 ; RUN: llvm-pdbdump pretty -symbols -globals -exclude-compilands="FilterTest.obj"  \
 ; RUN:    %p/Inputs/FilterTest.pdb | FileCheck  --check-prefix=EXCLUDE_COMPILAND %s
 ; RUN: llvm-pdbdump pretty -types -include-types="FilterTestClass" \
 ; RUN:    %p/Inputs/FilterTest.pdb | FileCheck --check-prefix=INCLUDE_ONLY_TYPES %s

 ; NO_FILTER: Inputs\FilterTest.obj
 ; NO_FILTER: int __cdecl main(int argc, char** argv)
 ; NO_FILTER: ---GLOBALS---
 ; NO_FILTER-DAG: double DoubleGlobalVar
 ; NO_FILTER-DAG: int IntGlobalVar
 ; NO_FILTER-DAG: GlobalEnum GlobalEnumVar
 
 ; EXCLUDE_TYPEDEFS: ---TYPES---
-; EXCLUDE_TYPEDEFS: Enums:
-; EXCLUDE_TYPEDEFS: GlobalEnum
-; EXCLUDE_TYPEDEFS: Typedefs
 ; EXCLUDE_TYPEDEFS-NOT: GlobalTypedef
-; EXCLUDE_TYPEDEFS: Classes
-; EXCLUDE_TYPEDEFS: class FilterTestClass
 ; EXCLUDE_TYPEDEFS-NOT: NestedTypedef
-; EXCLUDE_TYPEDEFS: private:
+; EXCLUDE_TYPEDEFS-DAG: GlobalEnum
+; EXCLUDE_TYPEDEFS-DAG: NestedEnum
+; EXCLUDE_TYPEDEFS: class FilterTestClass
 
 ; EXCLUDE_ENUMS: ---TYPES---
-; EXCLUDE_ENUMS: Enums:
 ; EXCLUDE_ENUMS-NOT: GlobalEnum
-; EXCLUDE_ENUMS: Typedefs
+; EXCLUDE_ENUMS-NOT: NestedEnum
 ; EXCLUDE_ENUMS: GlobalTypedef
-; EXCLUDE_ENUMS: Classes
 ; EXCLUDE_ENUMS: class FilterTestClass
-; EXCLUDE_ENUMS-NOT: NestedEnum
-; EXCLUDE_ENUMS: private:
 
 ; EXCLUDE_VARS: ---TYPES---
-; EXCLUDE_VARS: Classes:
-; EXCLUDE_VARS: class FilterTestClass
-; EXCLUDE_VARS: private:
 ; EXCLUDE_VARS-NOT: IntMemberVar
 ; EXCLUDE_VARS-NOT: DoubleMemberVar
+; EXCLUDE_VARS-DAG: GlobalEnum
+; EXCLUDE_VARS-DAG: NestedEnum
+; EXCLUDE_VARS: GlobalTypedef
+; EXCLUDE_VARS: class FilterTestClass
 ; EXCLUDE_VARS: ---GLOBALS---
 ; EXCLUDE_VARS-NOT: DoubleGlobalVar
 ; EXCLUDE_VARS-NOT: IntGlobalVar
 
 ; EXCLUDE_WHOLE_CLASS: ---TYPES---
 ; EXCLUDE_WHOLE_CLASS-NOT: class FilterTestClass
 ; EXCLUDE_WHOLE_CLASS-NOT: typedef int NestedTypedef
 ; EXCLUDE_WHOLE_CLASS-NOT: enum NestedEnum

llvm/tools/llvm-pdbdump/PrettyClassDefinitionDumper.h

public:
   void dump(const PDBSymbolTypeEnum &Symbol) override;
   void dump(const PDBSymbolFunc &Symbol) override;
   void dump(const PDBSymbolTypeTypedef &Symbol) override;
   void dump(const PDBSymbolTypeUDT &Symbol) override;
   void dump(const PDBSymbolTypeVTable &Symbol) override;
 
 private:
   LinePrinter &Printer;
-
-  struct SymbolGroup {
-    SymbolGroup() {}
-    SymbolGroup(SymbolGroup &&Other) {
-      Functions = std::move(Other.Functions);
-      Data = std::move(Other.Data);
-      Unknown = std::move(Other.Unknown);
-    }
-
-    std::list<std::unique_ptr<PDBSymbolFunc>> Functions;
-    std::list<std::unique_ptr<PDBSymbolData>> Data;
-    std::list<std::unique_ptr<PDBSymbol>> Unknown;
-    SymbolGroup(const SymbolGroup &other) = delete;
-    SymbolGroup &operator=(const SymbolGroup &other) = delete;
-  };
-  typedef std::unordered_map<int, SymbolGroup> SymbolGroupByAccess;
-
-  int dumpAccessGroup(PDB_MemberAccess Access, const SymbolGroup &Group);
 };
 }
 }
 #endif

llvm/tools/llvm-pdbdump/PrettyClassDefinitionDumper.cpp

 
 using namespace llvm;
 using namespace llvm::pdb;
 
 ClassDefinitionDumper::ClassDefinitionDumper(LinePrinter &P)
     : PDBSymDumper(true), Printer(P) {}
 
 void ClassDefinitionDumper::start(const PDBSymbolTypeUDT &Class) {
+  assert(opts::pretty::ClassFormat !=
+         opts::pretty::ClassDefinitionFormat::None);
+
   std::string Name = Class.getName();
   WithColor(Printer, PDB_ColorItem::Keyword).get() << Class.getUdtKind() << " ";
   WithColor(Printer, PDB_ColorItem::Type).get() << Class.getName();
 
   auto Bases = Class.findAllChildren<PDBSymbolTypeBaseClass>();
   if (Bases->getChildCount() > 0) {
     Printer.Indent();
     Printer.NewLine();
while (auto Base = Bases->getNext()) {
         Printer << ",";
       }
     }
     Printer.Unindent();
   }
 
   Printer << " {";
   auto Children = Class.findAllChildren();
-  if (Children->getChildCount() == 0) {
-    Printer << "}";
-    return;
-  }
-
-  // Try to dump symbols organized by member access level.  Public members
-  // first, then protected, then private.  This might be slow, so it's worth
-  // reconsidering the value of this if performance of large PDBs is a problem.
-  // NOTE: Access level of nested types is not recorded in the PDB, so we have
-  // a special case for them.
-  SymbolGroupByAccess Groups;
-  Groups.insert(std::make_pair(0, SymbolGroup()));
-  Groups.insert(std::make_pair((int)PDB_MemberAccess::Private, SymbolGroup()));
-  Groups.insert(
-      std::make_pair((int)PDB_MemberAccess::Protected, SymbolGroup()));
-  Groups.insert(std::make_pair((int)PDB_MemberAccess::Public, SymbolGroup()));
-
+  Printer.Indent();
+  int DumpedCount = 0;
   while (auto Child = Children->getNext()) {
-    PDB_MemberAccess Access = Child->getRawSymbol().getAccess();
-    if (isa<PDBSymbolTypeBaseClass>(*Child))
+    if (!isa<PDBSymbolData>(Child.get())) {
+      // Only data symbols affect record layout, so skip any non-data symbols if
+      // we're in record layout mode.
+      if (opts::pretty::ClassFormat ==
+          opts::pretty::ClassDefinitionFormat::LayoutOnly)
         continue;
-
-    auto &AccessGroup = Groups.find((int)Access)->second;
+    }
 
     if (auto Func = dyn_cast<PDBSymbolFunc>(Child.get())) {
       if (Func->isCompilerGenerated() && opts::pretty::ExcludeCompilerGenerated)
         continue;
+
       if (Func->getLength() == 0 && !Func->isPureVirtual() &&
           !Func->isIntroVirtualFunction())
         continue;
-      Child.release();
-      AccessGroup.Functions.push_back(std::unique_ptr<PDBSymbolFunc>(Func));
-    } else if (auto Data = dyn_cast<PDBSymbolData>(Child.get())) {
-      Child.release();
-      AccessGroup.Data.push_back(std::unique_ptr<PDBSymbolData>(Data));
-    } else {
-      AccessGroup.Unknown.push_back(std::move(Child));
-    }
     }
 
-  int Count = 0;
-  Count += dumpAccessGroup((PDB_MemberAccess)0, Groups[0]);
-  Count += dumpAccessGroup(PDB_MemberAccess::Public,
-                           Groups[(int)PDB_MemberAccess::Public]);
-  Count += dumpAccessGroup(PDB_MemberAccess::Protected,
-                           Groups[(int)PDB_MemberAccess::Protected]);
-  Count += dumpAccessGroup(PDB_MemberAccess::Private,
-                           Groups[(int)PDB_MemberAccess::Private]);
-  if (Count > 0)
-    Printer.NewLine();
-  Printer << "}";
+    ++DumpedCount;
+    Child->dump(*this);
   }
 
-int ClassDefinitionDumper::dumpAccessGroup(PDB_MemberAccess Access,
-                                           const SymbolGroup &Group) {
-  if (Group.Functions.empty() && Group.Data.empty() && Group.Unknown.empty())
-    return 0;
-
-  int Count = 0;
-  if (Access == PDB_MemberAccess::Private) {
-    Printer.NewLine();
-    WithColor(Printer, PDB_ColorItem::Keyword).get() << "private";
-    Printer << ":";
-  } else if (Access == PDB_MemberAccess::Protected) {
-    Printer.NewLine();
-    WithColor(Printer, PDB_ColorItem::Keyword).get() << "protected";
-    Printer << ":";
-  } else if (Access == PDB_MemberAccess::Public) {
-    Printer.NewLine();
-    WithColor(Printer, PDB_ColorItem::Keyword).get() << "public";
-    Printer << ":";
-  }
-  Printer.Indent();
-  for (auto iter = Group.Functions.begin(), end = Group.Functions.end();
-       iter != end; ++iter) {
-    ++Count;
-    (*iter)->dump(*this);
-  }
-  for (auto iter = Group.Data.begin(), end = Group.Data.end(); iter != end;
-       ++iter) {
-    ++Count;
-    (*iter)->dump(*this);
-  }
-  for (auto iter = Group.Unknown.begin(), end = Group.Unknown.end();
-       iter != end; ++iter) {
-    ++Count;
-    (*iter)->dump(*this);
-  }
   Printer.Unindent();
-  return Count;
+  if (DumpedCount > 0)
+    Printer.NewLine();
+  Printer << "}";
 }
 
 void ClassDefinitionDumper::dump(const PDBSymbolTypeBaseClass &Symbol) {}
 
 void ClassDefinitionDumper::dump(const PDBSymbolData &Symbol) {
   VariableDumper Dumper(Printer);
   Dumper.start(Symbol);
 }

llvm/tools/llvm-pdbdump/PrettyTypeDumper.cpp

void TypeDumper::dump(const PDBSymbolTypeUDT &Symbol) {
 
   if (Symbol.getUnmodifiedTypeId() != 0)
     return;
   if (Printer.IsTypeExcluded(Symbol.getName()))
     return;
 
   Printer.NewLine();
 
-  if (opts::pretty::NoClassDefs) {
+  if (opts::pretty::ClassFormat == opts::pretty::ClassDefinitionFormat::None) {
     WithColor(Printer, PDB_ColorItem::Keyword).get() << "class ";
     WithColor(Printer, PDB_ColorItem::Identifier).get() << Symbol.getName();
   } else {
     ClassDefinitionDumper Dumper(Printer);
     Dumper.start(Symbol);
   }
 }

llvm/tools/llvm-pdbdump/llvm-pdbdump.h

 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace opts {
 
 namespace pretty {
+enum class ClassDefinitionFormat { None, LayoutOnly, Full };
+
 extern llvm::cl::opt<bool> Compilands;
 extern llvm::cl::opt<bool> Symbols;
 extern llvm::cl::opt<bool> Globals;
 extern llvm::cl::opt<bool> Classes;
 extern llvm::cl::opt<bool> Enums;
 extern llvm::cl::opt<bool> Typedefs;
 extern llvm::cl::opt<bool> All;
 extern llvm::cl::opt<bool> ExcludeCompilerGenerated;
 
-extern llvm::cl::opt<bool> NoClassDefs;
+extern llvm::cl::opt<ClassDefinitionFormat> ClassFormat;
 extern llvm::cl::opt<bool> NoEnumDefs;
 extern llvm::cl::list<std::string> ExcludeTypes;
 extern llvm::cl::list<std::string> ExcludeSymbols;
 extern llvm::cl::list<std::string> ExcludeCompilands;
 extern llvm::cl::list<std::string> IncludeTypes;
 extern llvm::cl::list<std::string> IncludeSymbols;
 extern llvm::cl::list<std::string> IncludeCompilands;
 }

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

 cl::opt<bool> ExcludeCompilerGenerated(
     "no-compiler-generated",
     cl::desc("Don't show compiler generated types and symbols"),
     cl::cat(FilterCategory), cl::sub(PrettySubcommand));
 cl::opt<bool>
     ExcludeSystemLibraries("no-system-libs",
                            cl::desc("Don't show symbols from system libraries"),
                            cl::cat(FilterCategory), cl::sub(PrettySubcommand));
-cl::opt<bool> NoClassDefs("no-class-definitions",
-                          cl::desc("Don't display full class definitions"),
-                          cl::cat(FilterCategory), cl::sub(PrettySubcommand));
+cl::opt<ClassDefinitionFormat> ClassFormat(
+    "class-definitions", cl::desc("Class definition format"),
+    cl::init(ClassDefinitionFormat::Full),
+    cl::values(clEnumValN(ClassDefinitionFormat::Full, "full",
+                          "Display complete class definition"),
+               clEnumValN(ClassDefinitionFormat::LayoutOnly, "layout",
+                          "Display only members which affect record layout"),
+               clEnumValN(ClassDefinitionFormat::None, "none",
+                          "Don't display class definitions")),
+
 cl::opt<bool> NoEnumDefs("no-enum-definitions",
                          cl::desc("Don't display full enum definitions"),
                          cl::cat(FilterCategory), cl::sub(PrettySubcommand));
 }
 
 namespace diff {
 cl::opt<bool> Pedantic("pedantic",
                        cl::desc("Finds all differences (even structural ones "