Page MenuHomePhabricator

Add byte counting mechanism to LLDB's Stream class.

Authored by teemperor on Aug 1 2018, 2:32 PM.



This patch allows LLDB's Stream class to count the bytes it has written to so far.

There are two major motivations for this patch:

The first one is that this will allow us to get rid of all the handwritten byte counting code
we have in LLDB so far. Examples for this are pretty much all functions in LLDB that
take a Stream to write to and return a size_t, which usually represents the bytes written.

By moving to this centralized byte counting mechanism, we hopefully can avoid some
tricky errors that happen when some code forgets to count the written bytes while
writing something to a stream.

The second motivation is that this is needed for the migration away from LLDB's Stream
and towards LLVM's raw_ostream. My current plan is to start offering a fake raw_ostream
class that just forwards to a LLDB Stream.

However, for this raw_ostream wrapper we need to fulfill the raw_ostream interface with
LLDB's Stream, which currently lacks the ability to count the bytes written so far (which
raw_ostream exposes by it's tell() method). By adding this functionality it is trivial to start
rolling out our raw_ostream wrapper (and then eventually completely move to raw_ostream).

Also, once this fake raw_ostream is available, we can start replacing our own code writing
to LLDB's Stream by LLVM code writing to raw_ostream. The best example for this is the
LEB128 encoding we currently ship, which can be replaced with by LLVM's version which
accepts an raw_ostream.

From the point of view of the pure source changes this test does, we essentially just renamed
the Write implementation in Stream to WriteImpl while the Write method everyone is using
to write its raw bytes is now just forwarding and counting the written bytes.

Diff Detail

Event Timeline

teemperor created this revision.Aug 1 2018, 2:32 PM
teemperor retitled this revision from Add byte counting mechanism to stream. to Add byte counting mechanism to LLDB's Stream class..
teemperor updated this revision to Diff 158642.Aug 1 2018, 2:35 PM
teemperor edited the summary of this revision. (Show Details)
  • Added some more documentation.
labath accepted this revision.Aug 2 2018, 2:12 AM

I think there's a small difference in semantics between this and the tell function on llvm streams. This tells the number of bytes written, while the other one an absolute position within the stream. The two probably only differ if you open a StreamFile in append mode. However, even then, I doubt anyone will care about the distinction, so it shouldn't impede the migration. So, I'm happy to sign off on this. Thanks for doing it.

This revision is now accepted and ready to land.Aug 2 2018, 2:12 AM
JDevlieghere added inline comments.

I believe you need the < for Doxygen to associate the comment with the var, like the line(s) above.

teemperor updated this revision to Diff 158777.Aug 2 2018, 9:37 AM
  • Fixing some of the merge conflicts.
  • Fixed doxygen comment.

Thanks for the reviews!

This revision was automatically updated to reflect the committed changes.