[PATCH] D33229: [BinaryStream] Simplify the process of using BinaryStreams

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 16 00:14:23 PDT 2017


zturner created this revision.
Herald added a subscriber: mgorny.

The goal here is cutting down on boilerplate required to use these classes.

Often you have an array and you just want to use it.  With the current design, you have to first construct a `BinaryByteStream`, and then create a `BinaryStreamRef` from it.  Worse, the `BinaryStreamRef` holds a pointer to the `BinaryByteStream`, so you can't just create a temporary one to appease the compiler, you have to actually hold onto both the `ArrayRef` as well as the `BinaryByteStream` *AND* the `BinaryStreamReader` on top of that.  This makes for very cumbersome code, often requiring one to store a `BinaryByteStream` in a class just to circumvent this.

At the cost of some added complexity (not exposed to users, but internal to the library), we can do better than this.  This patch allows us to construct `BinaryStreamReaders` and `BinaryStreamWriters` directly from source data (e.g. `StringRef`, `MutableArrayRef<uint8_t>`, etc).  Not only does this reduce the amount of code you have to type and make it more obvious how to use it, but it solves real lifetime issues when it's inconvenient to hold onto a `BinaryByteStream` for a long time.

The additional complexity is in the form of an added layer of indirection.  Whereas before we simply stored a `BinaryStream*` in the ref, we now store both a `BinaryStream*` **and** a `std::shared_ptr<BinaryStream>`.  When the user wants to construct a `BinaryStreamRef` directly from an `ArrayRef` etc, we allocate an internal object that holds ownership over a `BinaryByteStream` and forwards all calls, and store this in the `shared_ptr<>`.  This also maintains the ref semantics, as you can copy it by value and references refer to the same underlying stream -- the one being held in the object stored in the `shared_ptr`.


https://reviews.llvm.org/D33229

Files:
  llvm/include/llvm/Support/BinaryStreamReader.h
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/include/llvm/Support/BinaryStreamWriter.h
  llvm/lib/Support/BinaryStreamReader.cpp
  llvm/lib/Support/BinaryStreamRef.cpp
  llvm/lib/Support/BinaryStreamWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/tools/llvm-readobj/COFFDumper.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33229.99107.patch
Type: text/x-patch
Size: 21878 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170516/550eeca1/attachment.bin>


More information about the llvm-commits mailing list