[PATCH] D26405: Add the ability to dump hex bytes with optional offsets and ASCII bytes.

[Mehdi AMINI via llvm-commits]

mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.

LGTM now. Wait a little to see if @zturner has any other comment.



================
Comment at: include/llvm/Support/Format.h:223
+
+inline FormattedHexBytes format_hex_bytes(const void *P, size_t N,
+                                          uint64_t Offset = -1ULL,
----------------
zturner wrote:
> mehdi_amini wrote:
> > I'm not against the "type erasure" with `void *`, but what about also keeping an overload with an ArrayRef of uint8_t? It could avoid to have to unpack at call sites.
> I kind of am.  Is there a reason why we need to specify `void* P, size_t N` when we can just specify `ArrayRef<uint8_t>`?
I see a reason: it avoids casting at call site when you don't have uint8_t (for instance if you have a ArrayRef<int8_t>).

(However is it dangerous because N here is the number of bytes and if you have an `ArrayRef<int> Arr` it is too easy to pass `Arr.size()` for N)


https://reviews.llvm.org/D26405