[PATCH] D21046: Use MappedBlockStream for parsing the directory

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 6 17:56:13 PDT 2016


ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM

Introducing a new abstraction make the code slightly more hard to understand, but since this reduces the amount of code by removing multiple pieces of code that do the same thing, I think it is overall win.


================
Comment at: include/llvm/DebugInfo/PDB/Raw/IPDBStreamData.h:1-2
@@ +1,3 @@
+//===- IPDBStreamData.h - Base interface for PDB Stream Data -----*- C++
+//-*-===//
+//
----------------
Format.

================
Comment at: include/llvm/DebugInfo/PDB/Raw/IndexedStreamData.h:19-28
@@ +18,12 @@
+
+/// IPDBStream abstracts the notion of PDB stream data.  Although we already
+/// have another stream abstraction (namely in the form of StreamInterface
+/// and MappedBlockStream), they assume that the stream data is referenced
+/// the same way.  Namely, by looking in the directory to get the list of
+/// stream blocks, and by looking in the array of stream lengths to get the
+/// length.  This breaks down for the directory itself, however, since its
+/// length and list of blocks are stored elsewhere.  By abstracting the
+/// notion of stream data further, we can use a MappedBlockStream to read
+/// from the directory itself, or from an indexed stream which references
+/// the directory.
+class IndexedStreamData : public IPDBStreamData {
----------------
Remove duplicate comment.


http://reviews.llvm.org/D21046





More information about the llvm-commits mailing list