[PATCH] Initial pass at API design for DebugInfo/PDB

[David Blaikie]

================
Comment at: include/llvm/DebugInfo/PDB/IPDBEnumChildren.h:24
@@ +23,3 @@
+
+  virtual uint32_t getChildCount() const = 0;
+  virtual ChildTypePtr getChildAtIndex(uint32_t Index) const = 0;
----------------
Name's OK, but "count" still seems like it might be a bit ambiguous with index, or number remaining. Another option (which seems more idiomatic in LLVM) might be "getNumChildren" (while I would like "size()" it's annoying that the count is attached to the iterator and not to some separate structure that produces iterators and I don't know how to properly communicate the separation of these concepts, unfortunately)

================
Comment at: include/llvm/DebugInfo/PDB/IPDBRawSymbol.h:193
@@ +192,3 @@
+  virtual bool isVirtualInheritance() const = 0;
+  virtual bool isVolatileType() const = 0;
+};
----------------
How're these functions going to communicate failure?

================
Comment at: include/llvm/DebugInfo/PDB/PDBSymbol.h:33
@@ +32,3 @@
+
+  IPDBRawSymbol *getRawSymbol();
+  const IPDBRawSymbol *getRawSymbol() const;
----------------
return by reference rather than pointer here, perhaps?

================
Comment at: include/llvm/DebugInfo/PDB/PDBSymbol.h:46
@@ +45,3 @@
+protected:
+  std::unique_ptr<IPDBRawSymbol> RawSymbol;
+};
----------------
could probably make this private and the derived classes can just call "getRawSymbol" - the derived classes don't need the ability to mutate (null, repoint, etc) this variable. (optional, not a big deal either way, just a little more stringent encapsulation)

The member could actually be const, too/instead.

================
Comment at: lib/DebugInfo/PDB/PDB.cpp:20
@@ +19,3 @@
+  // Create the correct concrete instance type based on the value of Type.
+  return std::unique_ptr<IPDBSession>();
+}
----------------
"return nullptr" will work here

http://reviews.llvm.org/D7356

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/