<br><br><div class="gmail_quote">On Wed, Jan 11, 2012 at 9:04 PM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">+</div><div class="im">
+class BitstreamBytes {<br>
+public:<br>
+  BitstreamBytes() { }<br>
<br>
<br>
</div>How is this different than the code in llvm/Support/MemoryObject.h?  It seems that any additional functionality should be merged into memory object instead of making some new bitcode-specific streaming api.<br>
<br>
New classes should have doxygen comments on them.  If MemoryBitstreamBytes needs to continue to exist, it should take a StringRef instead of start/end char positions.  In any case, the abstraction for streaming has nothing to do with bitcode, so it should be in MemoryObject or somewhere else that is not the bitcode reader.<br>
</blockquote><div><br></div><div>After considering this question a bit more, I have a better answer. The fundamental difference between what we need here and what MemoryObject provides is that an object that provides access to potentially-streamed data does not necessarily know in advance how large the data stream is. So the getExtent()/getEndPos() methods would have to wait until the whole stream is loaded in order to be implemented properly, which would defeat the purpose of streaming. That's why the uses of getLastChar() in BitstreamReader are replaced with isEndPos() or canSkipToPos().</div>
<div>So, similar predicates (or maybe we could get by with just one) could be added directly to MemoryObject (thus introducing streaming-specific interfaces into a class that's meant to be more general), or to some new class like StreamingMemoryObject (or more accurately, something silly like PotentiallyStreamingMemoryObject) which would have a streamed and non-streamed implementation.</div>
<div>Do you have a sense for which would be preferable?</div><div><br></div><div>Alternatively we could require that all streamable bitcode have a wrapper (the standard wrapper format already includes the bitcode size). This would definitely simplify the implementation, (and it would be fine for us since we use wrappers anyway) but the cost would be potentially reduced usefulness, and we might not necessarily be able to turn it on by default in a tool like llvm-dis (or we would at least have to condition it on the existence of the wrapper).</div>
</div>