[PATCH] D39517: [DebugInfo/PDB] Adding getUndecoratedNameEx and IPDB interfaces for IDiaEnumTables and IDiaTable.

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

Looks good, mostly just nitpicks.  Can you also add a test for this?  We've checked in an several actual PDBs into `test/tools/llvm-pdbutil/Inputs`, but you'd need to add a command line option to the `pretty` mode of `llvm-pdbutil` so that it can dump it.  Perhaps making it a hidden option.  Hopefully the PDBs that are checked in already will cause DIA to return something interesting for the tables.



================
Comment at: include/llvm/DebugInfo/PDB/DIA/DIARawSymbol.h:99
   std::string getUndecoratedName() const override;
+  std::string getUndecoratedNameEx(uint32_t Options) const override;
   uint32_t getUnmodifiedTypeId() const override;
----------------
Can we make this an enum, similar to how we have `PDB_NameSearchFlags` in this same file.


================
Comment at: include/llvm/DebugInfo/PDB/IPDBRawSymbol.h:111
   virtual std::string getUndecoratedName() const = 0;
+  virtual std::string getUndecoratedNameEx(uint32_t Options) const = 0;
   virtual uint32_t getUnmodifiedTypeId() const = 0;
----------------
enum if possible


================
Comment at: include/llvm/DebugInfo/PDB/PDBTypes.h:77-86
+  TableInvalid,
   Symbols,
   SourceFiles,
   LineNumbers,
   SectionContribs,
   Segments,
   InjectedSources,
----------------
None of these values are well documented.  I'm assuming you understand the format of the content in these tables, so if so would you mind adding a comment before the enumeration?  Or if it depends too heavily on the enum value, a comment next to each value.

For example, I can hypothesize that the `Symbols` contains a chunk of CodeView records, but what does that correspond to in the underlying PDB?  Is it literally direct access to the globals stream?  or the publics stream?  Or something else?  Same for line numbers.  It would be nice, while you're here, if you could expand a little bit on what people can expect to get out of these tables, because I honestly don't know.  There's some vague hints about it [here](https://docs.microsoft.com/en-us/visualstudio/debugger/debug-interface-access/idiatable), but this page does not mention `InputAssemblyFiles` or `Dbg` at all, so it's unclear what we can expect to find in these tables.


================
Comment at: include/llvm/DebugInfo/PDB/PDBTypes.h:250
+// https://msdn.microsoft.com/en-us/library/kszfk0fs.aspx
+#define UND_COMPLETE                0x0
+#define UND_NO_LEADING_UNDERSCORES  0x1
----------------
Can this be a flags enum like `PDB_NameSearchFlags` instead of some `#defines`?  We also don't need to adhere firmly to the Microsoft definition names, so we can camel-case them in traditional LLVM style.


================
Comment at: lib/DebugInfo/PDB/DIA/DIATable.cpp:40
+  CComBSTR Name16;
+  if (S_OK == Table->get_name(&Name16)) {
+    if (Name16 == DiaTable_Symbols)
----------------
Can you do an early return here if it fails?  Same with the rest of the cases, we can remove all the `else`s because each if will just return early.


https://reviews.llvm.org/D39517