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

[Zachary Turner]

================
Comment at: include/llvm/DebugInfo/PDB/IPDBDataStream.h:25
@@ +24,3 @@
+
+  virtual uint32 getRecordCount() = 0;
+  virtual std::string getName() = 0;
----------------
dblaikie wrote:
> This is the total number of things this stream will visit? (or the number remaining? or the current index?)
The former.  It's the total number of times it's valid to call getNext() if you're at the beginning of the stream (when it's newly constructed, or you've just called reset).

================
Comment at: include/llvm/DebugInfo/PDB/IPDBDataStream.h:26
@@ +25,3 @@
+  virtual uint32 getRecordCount() = 0;
+  virtual std::string getName() = 0;
+  virtual bool getItemAtIndex(RecordType &Record) = 0;
----------------
dblaikie wrote:
> What's this the name of?
The name of the data stream (which in turn identifies what kind of data it has).  The stream named "FPO" for example, contains fpo data.

================
Comment at: include/llvm/DebugInfo/PDB/IPDBDataStream.h:27
@@ +26,3 @@
+  virtual std::string getName() = 0;
+  virtual bool getItemAtIndex(RecordType &Record) = 0;
+  virtual bool getNext(RecordType &Record) = 0;
----------------
dblaikie wrote:
> I take it this returns the current item but doesn't move the cursor? Maybe it should be const?
In theory it probably could be, but the underlying DIA interface doesn't have any const methods.  I could probably use const on almost every single method of IPDBSymbol if we're ok const_casting.  I didn't make anything const though since the underlying DIA interface has no const methods.

Since you poitned out this method though, I also noticed that I left off the index.

================
Comment at: include/llvm/DebugInfo/PDB/IPDBSymbolBase.h:25
@@ +24,3 @@
+
+  virtual IPDBSymbolEnumerator *findChildren(PDB_SymType Type, StringRef Name,
+                                             PDB_NameSearchFlags Flags) = 0;
----------------
dblaikie wrote:
> Wouldn't this return value be passing ownership (via unique_ptr or similar)? and similarly on the next two functions.
Yes, thanks.  I forgot to update these to return unique_ptrs.

================
Comment at: include/llvm/DebugInfo/PDB/PDBTypes.h:27
@@ +26,3 @@
+
+typedef std::unique_ptr<IPDBDataStream> PDBDataStreamPtr;
+typedef std::unique_ptr<IPDBSession> PDBSessionPtr;
----------------
dblaikie wrote:
> Not sure if these typedefs help readability - it feels like they hinder it by hiding a common construct (unique_ptr) in something opaque when reading the code. But I tend to be a bit verbose... 
Does llvm tend to write out unique_ptr<T> everywhere?  It leads to a lot of line wrapping.

http://reviews.llvm.org/D7356

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