[BinaryStream] Simplify the process of using BinaryStreams

[zturner]

Summary

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.

[zturner]

+amccarth since he's used some of this stuff before too.

Also, in case it's not obvious, I included the changes to COFFDumper as an example of how it can simplify code (see especially the part where i no longer have to hold a copy of a BinaryStreamRef inside of a class.

The only real downside to this patch is that all instances of BinaryStreamRef become larger by sizeof(std::shared_ptr<T>), and we frequently pass them by value. But that seems somewhat minor.

[rnk]

This makes things shorter, but it does it by using templates to forward constructor arguments. Do we really want to do that? Is there some way we can simplify this code by removing some of the overlapping stream concepts instead?

llvm/lib/Support/BinaryStreamRef.cpp

19

Do we really need two refs? ArrayRef is already a pretty crappy name for a non-owning array. In general, can we have four word class names, max?

[zturner]

In D33229#756559, @rnk wrote:

This makes things shorter, but it does it by using templates to forward constructor arguments. Do we really want to do that? Is there some way we can simplify this code by removing some of the overlapping stream concepts instead?

I could get rid of the forwarding constructor by just providing a couple of explicit overloads.

[zturner]

Deleted the forwarding constructors and made the private owning implementation names shorter.

[rnk]

lgtm

llvm/include/llvm/Support/BinaryStreamReader.h

 /// \brief Provides read only access to a subclass of `BinaryStream`.  Provides
 /// bounds checking and helpers for writing certain common data types such as
 /// null-terminated strings, integers in various flavors of endianness, etc.
 /// Can be subclassed to provide reading of custom datatypes, although no
 /// are overridable.
 class BinaryStreamReader {
 public:
   BinaryStreamReader() = default;
-  explicit BinaryStreamReader(BinaryStreamRef Stream);
+  explicit BinaryStreamReader(BinaryStreamRef Ref);
+  explicit BinaryStreamReader(BinaryStream &Stream);
+  explicit BinaryStreamReader(ArrayRef<uint8_t> Data,
+                              llvm::support::endianness Endian);
+  explicit BinaryStreamReader(StringRef Data, llvm::support::endianness Endian);
+
+  BinaryStreamReader(const BinaryStreamReader &Other)
+      : Stream(Other.Stream), Offset(Other.Offset) {}
+
+  BinaryStreamReader &operator=(const BinaryStreamReader &Other) {
+    Stream = Other.Stream;
+    Offset = Other.Offset;
+    return *this;
+  }
+
   virtual ~BinaryStreamReader() {}
 
   /// Read as much as possible from the underlying string at the current offset
   /// without invoking a copy, and set \p Buffer to the resulting data slice.
   /// Updates the stream's offset to point after the newly read data.
   ///
   /// \returns a success error code if the data was successfully read, otherwise
   /// returns an appropriate error code.
public:
   /// \returns the next byte in the stream.
   uint8_t peek() const;
 
   std::pair<BinaryStreamReader, BinaryStreamReader>
   split(uint32_t Offset) const;
 
 private:
   BinaryStreamRef Stream;
-  uint32_t Offset;
+  uint32_t Offset = 0;
 };
 } // namespace llvm
 
 #endif // LLVM_SUPPORT_BINARYSTREAMREADER_H

llvm/include/llvm/Support/BinaryStreamRef.h

 #define LLVM_SUPPORT_BINARYSTREAMREF_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/BinaryStream.h"
 #include "llvm/Support/BinaryStreamError.h"
 #include "llvm/Support/Error.h"
 #include <algorithm>
 #include <cstdint>
+#include <memory>
 
 namespace llvm {
 
 /// Common stuff for mutable and immutable StreamRefs.
-template <class StreamType, class RefType> class BinaryStreamRefBase {
-public:
-  BinaryStreamRefBase() : Stream(nullptr), ViewOffset(0), Length(0) {}
-  BinaryStreamRefBase(StreamType &Stream, uint32_t Offset, uint32_t Length)
-      : Stream(&Stream), ViewOffset(Offset), Length(Length) {}
+template <class RefType, class StreamType> class BinaryStreamRefBase {
+protected:
+  BinaryStreamRefBase() = default;
+  BinaryStreamRefBase(std::shared_ptr<StreamType> SharedImpl, uint32_t Offset,
+                      uint32_t Length)
+      : SharedImpl(SharedImpl), BorrowedImpl(SharedImpl.get()),
+        ViewOffset(Offset), Length(Length) {}
+  BinaryStreamRefBase(StreamType &BorrowedImpl, uint32_t Offset,
+                      uint32_t Length)
+      : BorrowedImpl(&BorrowedImpl), ViewOffset(Offset), Length(Length) {}
+  BinaryStreamRefBase(const BinaryStreamRefBase &Other) {
+    SharedImpl = Other.SharedImpl;
+    BorrowedImpl = Other.BorrowedImpl;
+    ViewOffset = Other.ViewOffset;
+    Length = Other.Length;
+  }
 
-  llvm::support::endianness getEndian() const { return Stream->getEndian(); }
+public:
+  llvm::support::endianness getEndian() const {
+    return BorrowedImpl->getEndian();
+  }
 
   uint32_t getLength() const { return Length; }
-  const StreamType *getStream() const { return Stream; }
 
   /// Return a new BinaryStreamRef with the first \p N elements removed.
   RefType drop_front(uint32_t N) const {
-    if (!Stream)
+    if (!BorrowedImpl)
       return RefType();
 
     N = std::min(N, Length);
-    return RefType(*Stream, ViewOffset + N, Length - N);
+    RefType Result(static_cast<const RefType &>(*this));
+    Result.ViewOffset += N;
+    Result.Length -= N;
+    return Result;
   }
 
   /// Return a new BinaryStreamRef with only the first \p N elements remaining.
   RefType keep_front(uint32_t N) const {
-    if (!Stream)
+    if (!BorrowedImpl)
       return RefType();
     N = std::min(N, Length);
-    return RefType(*Stream, ViewOffset, N);
+    RefType Result(static_cast<const RefType &>(*this));
+    Result.Length = N;
+    return Result;
   }
 
   /// Return a new BinaryStreamRef with the first \p Offset elements removed,
   /// and retaining exactly \p Len elements.
   RefType slice(uint32_t Offset, uint32_t Len) const {
     return drop_front(Offset).keep_front(Len);
   }
 
+  bool valid() const { return BorrowedImpl != nullptr; }
+
   bool operator==(const RefType &Other) const {
-    if (Stream != Other.Stream)
+    if (BorrowedImpl != Other.BorrowedImpl)
       return false;
     if (ViewOffset != Other.ViewOffset)
       return false;
     if (Length != Other.Length)
       return false;
     return true;
   }
 
 protected:
   Error checkOffset(uint32_t Offset, uint32_t DataSize) const {
     if (Offset > getLength())
       return make_error<BinaryStreamError>(stream_error_code::invalid_offset);
     if (getLength() < DataSize + Offset)
       return make_error<BinaryStreamError>(stream_error_code::stream_too_short);
     return Error::success();
   }
 
-  StreamType *Stream;
-  uint32_t ViewOffset;
-  uint32_t Length;
+  std::shared_ptr<StreamType> SharedImpl;
+  StreamType *BorrowedImpl = nullptr;
+  uint32_t ViewOffset = 0;
+  uint32_t Length = 0;
 };
 
 /// \brief BinaryStreamRef is to BinaryStream what ArrayRef is to an Array.  It
 /// provides copy-semantics and read only access to a "window" of the underlying
 /// BinaryStream. Note that BinaryStreamRef is *not* a BinaryStream.  That is to
 /// say, it does not inherit and override the methods of BinaryStream.  In
 /// general, you should not pass around pointers or references to BinaryStreams
 /// and use inheritance to achieve polymorphism.  Instead, you should pass
 /// around BinaryStreamRefs by value and achieve polymorphism that way.
 class BinaryStreamRef
-    : public BinaryStreamRefBase<BinaryStream, BinaryStreamRef> {
+    : public BinaryStreamRefBase<BinaryStreamRef, BinaryStream> {
+  friend BinaryStreamRefBase<BinaryStreamRef, BinaryStream>;
+  friend class WritableBinaryStreamRef;
+  BinaryStreamRef(std::shared_ptr<BinaryStream> Impl, uint32_t ViewOffset,
+                  uint32_t Length)
+      : BinaryStreamRefBase(Impl, ViewOffset, Length) {}
+
 public:
   BinaryStreamRef() = default;
-  BinaryStreamRef(BinaryStream &Stream)
-      : BinaryStreamRefBase(Stream, 0, Stream.getLength()) {}
-  BinaryStreamRef(BinaryStream &Stream, uint32_t Offset, uint32_t Length)
-      : BinaryStreamRefBase(Stream, Offset, Length) {}
+  BinaryStreamRef(BinaryStream &Stream);
+  BinaryStreamRef(BinaryStream &Stream, uint32_t Offset, uint32_t Length);
+  explicit BinaryStreamRef(ArrayRef<uint8_t> Data,
+                           llvm::support::endianness Endian);
+  explicit BinaryStreamRef(StringRef Data, llvm::support::endianness Endian);
+
+  BinaryStreamRef(const BinaryStreamRef &Other);
 
   // Use BinaryStreamRef.slice() instead.
   BinaryStreamRef(BinaryStreamRef &S, uint32_t Offset,
                   uint32_t Length) = delete;
 
-  /// Check if a Stream is valid.
-  bool valid() const { return Stream != nullptr; }
-
   /// Given an Offset into this StreamRef and a Size, return a reference to a
   /// buffer owned by the stream.
   ///
   /// \returns a success error code if the entire range of data is within the
   /// bounds of this BinaryStreamRef's view and the implementation could read
   /// the data, and an appropriate error code otherwise.
   Error readBytes(uint32_t Offset, uint32_t Size,
-                  ArrayRef<uint8_t> &Buffer) const {
-    if (auto EC = checkOffset(Offset, Size))
-      return EC;
-
-    return Stream->readBytes(ViewOffset + Offset, Size, Buffer);
-  }
+                  ArrayRef<uint8_t> &Buffer) const;
 
   /// Given an Offset into this BinaryStreamRef, return a reference to the
   /// largest buffer the stream could support without necessitating a copy.
   ///
   /// \returns a success error code if implementation could read the data,
   /// and an appropriate error code otherwise.
   Error readLongestContiguousChunk(uint32_t Offset,
-                                   ArrayRef<uint8_t> &Buffer) const {
-    if (auto EC = checkOffset(Offset, 1))
-      return EC;
-
-    if (auto EC =
-            Stream->readLongestContiguousChunk(ViewOffset + Offset, Buffer))
-      return EC;
-    // This StreamRef might refer to a smaller window over a larger stream.  In
-    // that case we will have read out more bytes than we should return, because
-    // we should not read past the end of the current view.
-    uint32_t MaxLength = Length - Offset;
-    if (Buffer.size() > MaxLength)
-      Buffer = Buffer.slice(0, MaxLength);
-    return Error::success();
-  }
+                                   ArrayRef<uint8_t> &Buffer) const;
 };
 
 class WritableBinaryStreamRef
-    : public BinaryStreamRefBase<WritableBinaryStream,
-                                 WritableBinaryStreamRef> {
+    : public BinaryStreamRefBase<WritableBinaryStreamRef,
+                                 WritableBinaryStream> {
+  friend BinaryStreamRefBase<WritableBinaryStreamRef, WritableBinaryStream>;
+  WritableBinaryStreamRef(std::shared_ptr<WritableBinaryStream> Impl,
+                          uint32_t ViewOffset, uint32_t Length)
+      : BinaryStreamRefBase(Impl, ViewOffset, Length) {}
+
 public:
   WritableBinaryStreamRef() = default;
-  WritableBinaryStreamRef(WritableBinaryStream &Stream)
-      : BinaryStreamRefBase(Stream, 0, Stream.getLength()) {}
+  WritableBinaryStreamRef(WritableBinaryStream &Stream);
   WritableBinaryStreamRef(WritableBinaryStream &Stream, uint32_t Offset,
-                          uint32_t Length)
-      : BinaryStreamRefBase(Stream, Offset, Length) {}
+                          uint32_t Length);
+  explicit WritableBinaryStreamRef(MutableArrayRef<uint8_t> Data,
+                                   llvm::support::endianness Endian);
+  WritableBinaryStreamRef(const WritableBinaryStreamRef &Other);
 
   // Use WritableBinaryStreamRef.slice() instead.
   WritableBinaryStreamRef(WritableBinaryStreamRef &S, uint32_t Offset,
                           uint32_t Length) = delete;
 
   /// Given an Offset into this WritableBinaryStreamRef and some input data,
   /// writes the data to the underlying stream.
   ///
   /// \returns a success error code if the data could fit within the underlying
   /// stream at the specified location and the implementation could write the
   /// data, and an appropriate error code otherwise.
-  Error writeBytes(uint32_t Offset, ArrayRef<uint8_t> Data) const {
-    if (auto EC = checkOffset(Offset, Data.size()))
-      return EC;
-
-    return Stream->writeBytes(ViewOffset + Offset, Data);
-  }
+  Error writeBytes(uint32_t Offset, ArrayRef<uint8_t> Data) const;
 
-  operator BinaryStreamRef() { return BinaryStreamRef(*Stream); }
+  /// Conver this WritableBinaryStreamRef to a read-only BinaryStreamRef.
+  operator BinaryStreamRef() const;
 
   /// \brief For buffered streams, commits changes to the backing store.
-  Error commit() { return Stream->commit(); }
+  Error commit();
 };
 
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_BINARYSTREAMREF_H

llvm/include/llvm/Support/BinaryStreamWriter.h

 /// \brief Provides write only access to a subclass of `WritableBinaryStream`.
 /// Provides bounds checking and helpers for writing certain common data types
 /// such as null-terminated strings, integers in various flavors of endianness,
 /// etc.  Can be subclassed to provide reading and writing of custom datatypes,
 /// although no methods are overridable.
 class BinaryStreamWriter {
 public:
   BinaryStreamWriter() = default;
-  explicit BinaryStreamWriter(WritableBinaryStreamRef Stream);
+  explicit BinaryStreamWriter(WritableBinaryStreamRef Ref);
+  explicit BinaryStreamWriter(WritableBinaryStream &Stream);
+  explicit BinaryStreamWriter(MutableArrayRef<uint8_t> Data,
+                              llvm::support::endianness Endian);
+
+  BinaryStreamWriter(const BinaryStreamWriter &Other)
+      : Stream(Other.Stream), Offset(Other.Offset) {}
+
+  BinaryStreamWriter &operator=(const BinaryStreamWriter &Other) {
+    Stream = Other.Stream;
+    Offset = Other.Offset;
+    return *this;
+  }
+
   virtual ~BinaryStreamWriter() {}
 
   /// Write the bytes specified in \p Buffer to the underlying stream.
   /// On success, updates the offset so that subsequent writes will occur
   /// at the next unwritten position.
   ///
   /// \returns a success error code if the data was successfully written,
   /// otherwise returns an appropriate error code.

llvm/lib/Support/BinaryStreamReader.cpp

 //===- BinaryStreamReader.cpp - Reads objects from a binary stream --------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/BinaryStreamReader.h"
 
 #include "llvm/Support/BinaryStreamError.h"
 #include "llvm/Support/BinaryStreamRef.h"
 
 using namespace llvm;
+using endianness = llvm::support::endianness;
 
-BinaryStreamReader::BinaryStreamReader(BinaryStreamRef S)
-    : Stream(S), Offset(0) {}
+BinaryStreamReader::BinaryStreamReader(BinaryStreamRef Ref) : Stream(Ref) {}
+
+BinaryStreamReader::BinaryStreamReader(BinaryStream &Stream) : Stream(Stream) {}
+
+BinaryStreamReader::BinaryStreamReader(ArrayRef<uint8_t> Data,
+                                       endianness Endian)
+    : Stream(Data, Endian) {}
+
+BinaryStreamReader::BinaryStreamReader(StringRef Data, endianness Endian)
+    : Stream(Data, Endian) {}
 
 Error BinaryStreamReader::readLongestContiguousChunk(
     ArrayRef<uint8_t> &Buffer) {
   if (auto EC = Stream.readLongestContiguousChunk(Offset, Buffer))
     return EC;
   Offset += Buffer.size();
   return Error::success();
 }
BinaryStreamReader::split(uint32_t Off) const {
   BinaryStreamRef First = Stream.drop_front(Offset);
 
   BinaryStreamRef Second = First.drop_front(Off);
   First = First.keep_front(Off);
   BinaryStreamReader W1{First};
   BinaryStreamReader W2{Second};
   return std::make_pair(W1, W2);
 }
 No newline at end of file

llvm/lib/Support/BinaryStreamRef.cpp

+//===- BinaryStreamRef.cpp - ----------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/BinaryStreamRef.h"
+#include "llvm/Support/BinaryByteStream.h"
+
+using namespace llvm;
+using namespace llvm::support;
+
+namespace {
+
+class ArrayRefImpl : public BinaryStream {
+public:
+  ArrayRefImpl(ArrayRef<uint8_t> Data, endianness Endian) : BBS(Data, Endian) {}
+
+  llvm::support::endianness getEndian() const override {
+    return BBS.getEndian();
+  }
+  Error readBytes(uint32_t Offset, uint32_t Size,
+                  ArrayRef<uint8_t> &Buffer) override {
+    return BBS.readBytes(Offset, Size, Buffer);
+  }
+  Error readLongestContiguousChunk(uint32_t Offset,
+                                   ArrayRef<uint8_t> &Buffer) override {
+    return BBS.readLongestContiguousChunk(Offset, Buffer);
+  }
+  uint32_t getLength() override { return BBS.getLength(); }
+
+private:
+  BinaryByteStream BBS;
+};
+
+class MutableArrayRefImpl : public WritableBinaryStream {
+public:
+  MutableArrayRefImpl(MutableArrayRef<uint8_t> Data, endianness Endian)
+      : BBS(Data, Endian) {}
+
+  // Inherited via WritableBinaryStream
+  llvm::support::endianness getEndian() const override {
+    return BBS.getEndian();
+  }
+  Error readBytes(uint32_t Offset, uint32_t Size,
+                  ArrayRef<uint8_t> &Buffer) override {
+    return BBS.readBytes(Offset, Size, Buffer);
+  }
+  Error readLongestContiguousChunk(uint32_t Offset,
+                                   ArrayRef<uint8_t> &Buffer) override {
+    return BBS.readLongestContiguousChunk(Offset, Buffer);
+  }
+  uint32_t getLength() override { return BBS.getLength(); }
+
+  Error writeBytes(uint32_t Offset, ArrayRef<uint8_t> Data) override {
+    return BBS.writeBytes(Offset, Data);
+  }
+  Error commit() override { return BBS.commit(); }
+
+private:
+  MutableBinaryByteStream BBS;
+};
+}
+
+BinaryStreamRef::BinaryStreamRef(BinaryStream &Stream)
+    : BinaryStreamRef(Stream, 0, Stream.getLength()) {}
+BinaryStreamRef::BinaryStreamRef(BinaryStream &Stream, uint32_t Offset,
+                                 uint32_t Length)
+    : BinaryStreamRefBase(Stream, Offset, Length) {}
+BinaryStreamRef::BinaryStreamRef(ArrayRef<uint8_t> Data, endianness Endian)
+    : BinaryStreamRefBase(std::make_shared<ArrayRefImpl>(Data, Endian), 0,
+                          Data.size()) {}
+BinaryStreamRef::BinaryStreamRef(StringRef Data, endianness Endian)
+    : BinaryStreamRef(makeArrayRef(Data.bytes_begin(), Data.bytes_end()),
+                      Endian) {}
+
+BinaryStreamRef::BinaryStreamRef(const BinaryStreamRef &Other)
+    : BinaryStreamRefBase(Other) {}
+
+Error BinaryStreamRef::readBytes(uint32_t Offset, uint32_t Size,
+                                 ArrayRef<uint8_t> &Buffer) const {
+  if (auto EC = checkOffset(Offset, Size))
+    return EC;
+  return BorrowedImpl->readBytes(ViewOffset + Offset, Size, Buffer);
+}
+
+Error BinaryStreamRef::readLongestContiguousChunk(
+    uint32_t Offset, ArrayRef<uint8_t> &Buffer) const {
+  if (auto EC = checkOffset(Offset, 1))
+    return EC;
+
+  if (auto EC =
+          BorrowedImpl->readLongestContiguousChunk(ViewOffset + Offset, Buffer))
+    return EC;
+  // This StreamRef might refer to a smaller window over a larger stream.  In
+  // that case we will have read out more bytes than we should return, because
+  // we should not read past the end of the current view.
+  uint32_t MaxLength = Length - Offset;
+  if (Buffer.size() > MaxLength)
+    Buffer = Buffer.slice(0, MaxLength);
+  return Error::success();
+}
+
+WritableBinaryStreamRef::WritableBinaryStreamRef(WritableBinaryStream &Stream)
+    : WritableBinaryStreamRef(Stream, 0, Stream.getLength()) {}
+
+WritableBinaryStreamRef::WritableBinaryStreamRef(WritableBinaryStream &Stream,
+                                                 uint32_t Offset,
+                                                 uint32_t Length)
+    : BinaryStreamRefBase(Stream, Offset, Length) {}
+
+WritableBinaryStreamRef::WritableBinaryStreamRef(MutableArrayRef<uint8_t> Data,
+                                                 endianness Endian)
+    : BinaryStreamRefBase(std::make_shared<MutableArrayRefImpl>(Data, Endian),
+                          0, Data.size()) {}
+
+WritableBinaryStreamRef::WritableBinaryStreamRef(
+    const WritableBinaryStreamRef &Other)
+    : BinaryStreamRefBase(Other) {}
+
+Error WritableBinaryStreamRef::writeBytes(uint32_t Offset,
+                                          ArrayRef<uint8_t> Data) const {
+  if (auto EC = checkOffset(Offset, Data.size()))
+    return EC;
+
+  return BorrowedImpl->writeBytes(ViewOffset + Offset, Data);
+}
+
+WritableBinaryStreamRef::operator BinaryStreamRef() const {
+  return BinaryStreamRef(*BorrowedImpl, ViewOffset, Length);
+}
+
+/// \brief For buffered streams, commits changes to the backing store.
+Error WritableBinaryStreamRef::commit() { return BorrowedImpl->commit(); }

llvm/lib/Support/BinaryStreamWriter.cpp

 #include "llvm/Support/BinaryStreamWriter.h"
 
 #include "llvm/Support/BinaryStreamError.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/BinaryStreamRef.h"
 
 using namespace llvm;
 
-BinaryStreamWriter::BinaryStreamWriter(WritableBinaryStreamRef S)
-    : Stream(S), Offset(0) {}
+BinaryStreamWriter::BinaryStreamWriter(WritableBinaryStreamRef Ref)
+    : Stream(Ref) {}
+
+BinaryStreamWriter::BinaryStreamWriter(WritableBinaryStream &Stream)
+    : Stream(Stream) {}
+
+BinaryStreamWriter::BinaryStreamWriter(MutableArrayRef<uint8_t> Data,
+                                       llvm::support::endianness Endian)
+    : Stream(Data, Endian) {}
 
 Error BinaryStreamWriter::writeBytes(ArrayRef<uint8_t> Buffer) {
   if (auto EC = Stream.writeBytes(Offset, Buffer))
     return EC;
   Offset += Buffer.size();
   return Error::success();
 }
 

llvm/lib/Support/CMakeLists.txt

add_llvm_library(LLVMSupport
   APInt.cpp
   APSInt.cpp
   ARMBuildAttrs.cpp
   ARMAttributeParser.cpp
   ARMWinEH.cpp
   Allocator.cpp
   BinaryStreamError.cpp
   BinaryStreamReader.cpp
+  BinaryStreamRef.cpp
   BinaryStreamWriter.cpp
   BlockFrequency.cpp
   BranchProbability.cpp
   CachePruning.cpp
   circular_raw_ostream.cpp
   Chrono.cpp
   COM.cpp
   CommandLine.cpp

llvm/tools/llvm-readobj/COFFDumper.cpp

 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeDumpVisitor.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeStreamMerger.h"
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/ObjectFile.h"
-#include "llvm/Support/BinaryByteStream.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/COFF.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/DataExtractor.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
getResourceDirectoryTableEntry(const coff_resource_dir_table &Table,
                                  uint32_t Index);
 
   typedef DenseMap<const coff_section*, std::vector<RelocationRef> > RelocMapTy;
 
   const llvm::object::COFFObjectFile *Obj;
   bool RelocCached = false;
   RelocMapTy RelocMap;
 
-  BinaryByteStream ChecksumContents;
   VarStreamArray<FileChecksumEntry> CVFileChecksumTable;
 
-  BinaryByteStream StringTableContents;
   StringTableRef CVStringTable;
 
   ScopedPrinter &Writer;
   TypeDatabase TypeDB;
 };
 
 class COFFObjectDumpDelegate : public SymbolDumpDelegate {
 public:
while (Reader.bytesRemaining() > 0 &&
     error(Reader.readInteger(SubType));
     error(Reader.readInteger(SubSectionSize));
 
     StringRef Contents;
     error(Reader.readFixedString(Contents, SubSectionSize));
 
     switch (ModuleDebugFragmentKind(SubType)) {
     case ModuleDebugFragmentKind::FileChecksums: {
-      ChecksumContents = BinaryByteStream(Contents, support::little);
-      BinaryStreamReader CSR(ChecksumContents);
+      BinaryStreamReader CSR(Contents, support::little);
       error(CSR.readArray(CVFileChecksumTable, CSR.getLength()));
       break;
     }
     case ModuleDebugFragmentKind::StringTable: {
-      StringTableContents = BinaryByteStream(Contents, support::little);
-      error(CVStringTable.initialize(StringTableContents));
+      BinaryStreamRef ST(Contents, support::little);
+      error(CVStringTable.initialize(ST));
     } break;
     default:
       break;
     }
 
     uint32_t PaddedSize = alignTo(SubSectionSize, 4);
     error(Reader.skip(PaddedSize - SubSectionSize));
   }
void COFFDumper::printCodeViewSymbolSection(StringRef SectionName,
   W.printNumber("Section", SectionName, Obj->getSectionID(Section));
 
   uint32_t Magic;
   error(consume(Data, Magic));
   W.printHex("Magic", Magic);
   if (Magic != COFF::DEBUG_SECTION_MAGIC)
     return error(object_error::parse_failed);
 
-  BinaryByteStream FileAndStrings(Data, support::little);
-  BinaryStreamReader FSReader(FileAndStrings);
+  BinaryStreamReader FSReader(Data, support::little);
   initializeFileAndStringTables(FSReader);
 
   // TODO: Convert this over to using ModuleSubstreamVisitor.
   while (!Data.empty()) {
     // The section consists of a number of subsection in the following format:
     // |SubSectionType|SubSectionSize|Contents...|
     uint32_t SubType, SubSectionSize;
     error(consume(Data, SubType));
case ModuleDebugFragmentKind::Lines: {
       }
 
       FunctionLineTables[LinkageName] = Contents;
       FunctionNames.push_back(LinkageName);
       break;
     }
     case ModuleDebugFragmentKind::FrameData: {
       // First four bytes is a relocation against the function.
-      BinaryByteStream S(Contents, llvm::support::little);
-      BinaryStreamReader SR(S);
+      BinaryStreamReader SR(Contents, llvm::support::little);
       const uint32_t *CodePtr;
       error(SR.readObject(CodePtr));
       StringRef LinkageName;
       error(resolveSymbolName(Obj->getCOFFSection(Section), SectionContents,
                               CodePtr, LinkageName));
       W.printString("LinkageName", LinkageName);
 
       // To find the active frame description, search this array for the
void COFFDumper::printCodeViewSymbolSection(StringRef SectionName,
 
   // Dump the line tables now that we've read all the subsections and know all
   // the required information.
   for (unsigned I = 0, E = FunctionNames.size(); I != E; ++I) {
     StringRef Name = FunctionNames[I];
     ListScope S(W, "FunctionLineTable");
     W.printString("LinkageName", Name);
 
-    BinaryByteStream LineTableInfo(FunctionLineTables[Name], support::little);
-    BinaryStreamReader Reader(LineTableInfo);
+    BinaryStreamReader Reader(FunctionLineTables[Name], support::little);
 
     ModuleDebugLineFragmentRef LineInfo;
     error(LineInfo.initialize(Reader));
 
     W.printHex("Flags", LineInfo.header()->Flags);
     W.printHex("CodeSize", LineInfo.header()->CodeSize);
     for (const auto &Entry : LineInfo) {
 
void COFFDumper::printCodeViewSymbolsSubsection(StringRef Subsection,
                                                 StringRef SectionContents) {
   ArrayRef<uint8_t> BinaryData(Subsection.bytes_begin(),
                                Subsection.bytes_end());
   auto CODD = llvm::make_unique<COFFObjectDumpDelegate>(*this, Section, Obj,
                                                         SectionContents);
 
   CVSymbolDumper CVSD(W, TypeDB, std::move(CODD),
                       opts::CodeViewSubsectionBytes);
-  BinaryByteStream Stream(BinaryData, llvm::support::little);
   CVSymbolArray Symbols;
-  BinaryStreamReader Reader(Stream);
+  BinaryStreamReader Reader(BinaryData, llvm::support::little);
   if (auto EC = Reader.readArray(Symbols, Reader.getLength())) {
     consumeError(std::move(EC));
     W.flush();
     error(object_error::parse_failed);
   }
 
   if (auto EC = CVSD.dump(Symbols)) {
     W.flush();
     error(std::move(EC));
   }
   W.flush();
 }
 
 void COFFDumper::printCodeViewFileChecksums(StringRef Subsection) {
-  BinaryByteStream S(Subsection, llvm::support::little);
-  BinaryStreamReader SR(S);
+  BinaryStreamReader SR(Subsection, llvm::support::little);
   ModuleDebugFileChecksumFragmentRef Checksums;
   error(Checksums.initialize(SR));
 
   for (auto &FC : Checksums) {
     DictScope S(W, "FileChecksum");
 
     StringRef Filename = error(CVStringTable.getString(FC.FileNameOffset));
     W.printHex("Filename", Filename, FC.FileNameOffset);
     W.printHex("ChecksumSize", FC.Checksum.size());
     W.printEnum("ChecksumKind", uint8_t(FC.Kind),
                 makeArrayRef(FileChecksumKindNames));
 
     W.printBinary("ChecksumBytes", FC.Checksum);
   }
 }
 
 void COFFDumper::printCodeViewInlineeLines(StringRef Subsection) {
-  BinaryByteStream S(Subsection, llvm::support::little);
-  BinaryStreamReader SR(S);
+  BinaryStreamReader SR(Subsection, llvm::support::little);
   ModuleDebugInlineeLineFragmentRef Lines;
   error(Lines.initialize(SR));
 
   for (auto &Line : Lines) {
     DictScope S(W, "InlineeSourceLine");
     printTypeIndex("Inlinee", Line.Header->Inlinee);
     printFileNameForOffset("FileID", Line.Header->FileID);
     W.printNumber("SourceLineNum", Line.Header->SourceLineNum);
for (const SectionRef &S : Obj->sections()) {
     error(S.getName(SectionName));
     if (SectionName == ".debug$T") {
       StringRef Data;
       error(S.getContents(Data));
       uint32_t Magic;
       error(consume(Data, Magic));
       if (Magic != 4)
         error(object_error::parse_failed);
-      ArrayRef<uint8_t> Bytes(reinterpret_cast<const uint8_t *>(Data.data()),
-                              Data.size());
-      BinaryByteStream Stream(Bytes, llvm::support::little);
+
       CVTypeArray Types;
-      BinaryStreamReader Reader(Stream);
+      BinaryStreamReader Reader(Data, llvm::support::little);
       if (auto EC = Reader.readArray(Types, Reader.getLength())) {
         consumeError(std::move(EC));
         W.flush();
         error(object_error::parse_failed);
       }
 
       if (auto EC = mergeTypeStreams(CVIDs, CVTypes, nullptr, Types))
         return error(std::move(EC));