[Patch] Add array support to EndianStream.

[David Blaikie via llvm-commits]

On Fri, Jan 22, 2016 at 11:45 AM, Justin Bogner <mail at justinbogner.com>
wrote:

> Justin Bogner <mail at justinbogner.com> writes:
> > Rafael Espíndola via llvm-commits <llvm-commits at lists.llvm.org> writes:
> >>> I will upload a new patch. Thanks for the suggestion.
> >>
> >> Attached.
> >
> > This is awesome. LGTM.
>
> Spoke too soon, minor nits *then* this LGTM.
>
> > diff --git a/include/llvm/Support/EndianStream.h
> b/include/llvm/Support/EndianStream.h
> > index d44a9b3..81908e9 100644
> > --- a/include/llvm/Support/EndianStream.h
> > +++ b/include/llvm/Support/EndianStream.h
> > @@ -15,6 +15,7 @@
> >  #ifndef LLVM_SUPPORT_ENDIANSTREAM_H
> >  #define LLVM_SUPPORT_ENDIANSTREAM_H
> >
> > +#include "llvm/ADT/ArrayRef.h"
> >  #include "llvm/Support/Endian.h"
> >  #include "llvm/Support/raw_ostream.h"
> >
> > @@ -26,6 +27,12 @@ namespace endian {
> >  template <endianness endian> struct Writer {
> >    raw_ostream &OS;
> >    Writer(raw_ostream &OS) : OS(OS) {}
> > +  template <typename value_type> void write(ArrayRef<value_type> Vals) {
> > +    for (value_type V : Vals) {
> > +      value_type Swaped = byte_swap<value_type, endian>(V);
> > +      OS.write((const char *)&Swaped, sizeof(value_type));
> > +    }
> > +  }
>
> typo, this should say "Swapped". But isn't the below better anyways?
>
>   template <typename value_type> void write(ArrayRef<value_type> Vals) {
>     for (value_type V : Vals)
>       write(V);
>

+1 at least (I was trying to suggest this in my first email:  "Also, would
it make sense to implement the array version in terms of the non-array
version, rather than doing the byte swapping explicitly here? ")

Looking at it further, this should allow the various write specializations
(for float and double) to work for the array version - a test or two could
be nice to demonstrate this.

  }
>
> >    template <typename value_type> void write(value_type Val) {
> >      Val = byte_swap<value_type, endian>(Val);
> >      OS.write((const char *)&Val, sizeof(value_type));
> > diff --git a/unittests/Support/EndianStreamTest.cpp
> b/unittests/Support/EndianStreamTest.cpp
> > index 6a69be5..8221372 100644
> > --- a/unittests/Support/EndianStreamTest.cpp
> > +++ b/unittests/Support/EndianStreamTest.cpp
> > @@ -153,5 +153,35 @@ TEST(EndianStream, WriteDoubleBE) {
> >    EXPECT_EQ(static_cast<uint8_t>(data[7]), 0x20);
> >  }
> >
> > +TEST(EndianStream, WriteArrayLE) {
> > +  SmallString<16> Data;
> > +
> > +  {
> > +    raw_svector_ostream OS(Data);
> > +    endian::Writer<little> LE(OS);
> > +    LE.write<uint16_t>({0x1234, 0x5678});
> > +  }
> > +
> > +  EXPECT_EQ(static_cast<uint8_t>(Data[0]), 0x34);
> > +  EXPECT_EQ(static_cast<uint8_t>(Data[1]), 0x12);
> > +  EXPECT_EQ(static_cast<uint8_t>(Data[2]), 0x78);
> > +  EXPECT_EQ(static_cast<uint8_t>(Data[3]), 0x56);
> > +}
> > +
> > +TEST(EndianStream, WriteVectorLE) {
> > +  SmallString<16> Data;
> > +
> > +  {
> > +    raw_svector_ostream OS(Data);
> > +    endian::Writer<little> LE(OS);
> > +    std::vector<uint16_t> Vec{0x1234, 0x5678};
> > +    LE.write<uint16_t>(Vec);
> > +  }
> > +
> > +  EXPECT_EQ(static_cast<uint8_t>(Data[0]), 0x34);
> > +  EXPECT_EQ(static_cast<uint8_t>(Data[1]), 0x12);
> > +  EXPECT_EQ(static_cast<uint8_t>(Data[2]), 0x78);
> > +  EXPECT_EQ(static_cast<uint8_t>(Data[3]), 0x56);
> > +}
> >
> >  } // end anon namespace
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160122/f010ff23/attachment.html>