[PATCH] D124804: [Object][DX] Parse DXContainer Parts

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

In D124804#3526729 <https://reviews.llvm.org/D124804#3526729>, @beanz wrote:

> libObject uses MemoryBufferRefs which play back and forth with StringRef, and while it has been pointed out over and over again that many of StringRef’s methods should probably be on MemoryBufferRef because they are more broadly useful, it is the way it is.
> I think ArrayRef<char> is probably the least common pattern for representing arbitrary data buffers in LLVM, and given the needs of libObject to operate with MemoryBufferRef I think this code is best to operate on StringRefs.

I'm much more used to `ArrayRef<uint8_t>`. For example, you can see it used in `BinaryStreamReader::readLongestContiguousChunk` and there's also a helper conversion function `ArrayRef<uint8_t> arrayRefFromStringRef(StringRef Input)` in `StringExtras.h`. In total, I counted ~1500 uses of `ArrayRef<uint8_t>` in the monorepo, so I don't consider it an uncommon pattern.

That being said, I think sticking with `StringRef` is also a practical option and I don't oppose it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124804/new/

https://reviews.llvm.org/D124804