<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 22, 2016 at 11:45 AM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> writes:<br>
> Rafael Espíndola via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> writes:<br>
>>> I will upload a new patch. Thanks for the suggestion.<br>
>><br>
>> Attached.<br>
><br>
> This is awesome. LGTM.<br>
<br>
</span>Spoke too soon, minor nits *then* this LGTM.<br>
<span class=""><br>
> diff --git a/include/llvm/Support/EndianStream.h b/include/llvm/Support/EndianStream.h<br>
> index d44a9b3..81908e9 100644<br>
> --- a/include/llvm/Support/EndianStream.h<br>
> +++ b/include/llvm/Support/EndianStream.h<br>
> @@ -15,6 +15,7 @@<br>
>  #ifndef LLVM_SUPPORT_ENDIANSTREAM_H<br>
>  #define LLVM_SUPPORT_ENDIANSTREAM_H<br>
><br>
> +#include "llvm/ADT/ArrayRef.h"<br>
>  #include "llvm/Support/Endian.h"<br>
>  #include "llvm/Support/raw_ostream.h"<br>
><br>
> @@ -26,6 +27,12 @@ namespace endian {<br>
>  template <endianness endian> struct Writer {<br>
>    raw_ostream &OS;<br>
>    Writer(raw_ostream &OS) : OS(OS) {}<br>
> +  template <typename value_type> void write(ArrayRef<value_type> Vals) {<br>
> +    for (value_type V : Vals) {<br>
> +      value_type Swaped = byte_swap<value_type, endian>(V);<br>
> +      OS.write((const char *)&Swaped, sizeof(value_type));<br>
> +    }<br>
> +  }<br>
<br>
</span>typo, this should say "Swapped". But isn't the below better anyways?<br>
<span class=""><br>
  template <typename value_type> void write(ArrayRef<value_type> Vals) {<br>
</span>    for (value_type V : Vals)<br>
      write(V);<br></blockquote><div><br>+1 at least (I was trying to suggest this in my first email:  "<span style="font-family:monospace">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? ")<br><br>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.<br><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5">  }<br>
<br>
>    template <typename value_type> void write(value_type Val) {<br>
>      Val = byte_swap<value_type, endian>(Val);<br>
>      OS.write((const char *)&Val, sizeof(value_type));<br>
> diff --git a/unittests/Support/EndianStreamTest.cpp b/unittests/Support/EndianStreamTest.cpp<br>
> index 6a69be5..8221372 100644<br>
> --- a/unittests/Support/EndianStreamTest.cpp<br>
> +++ b/unittests/Support/EndianStreamTest.cpp<br>
> @@ -153,5 +153,35 @@ TEST(EndianStream, WriteDoubleBE) {<br>
>    EXPECT_EQ(static_cast<uint8_t>(data[7]), 0x20);<br>
>  }<br>
><br>
> +TEST(EndianStream, WriteArrayLE) {<br>
> +  SmallString<16> Data;<br>
> +<br>
> +  {<br>
> +    raw_svector_ostream OS(Data);<br>
> +    endian::Writer<little> LE(OS);<br>
> +    LE.write<uint16_t>({0x1234, 0x5678});<br>
> +  }<br>
> +<br>
> +  EXPECT_EQ(static_cast<uint8_t>(Data[0]), 0x34);<br>
> +  EXPECT_EQ(static_cast<uint8_t>(Data[1]), 0x12);<br>
> +  EXPECT_EQ(static_cast<uint8_t>(Data[2]), 0x78);<br>
> +  EXPECT_EQ(static_cast<uint8_t>(Data[3]), 0x56);<br>
> +}<br>
> +<br>
> +TEST(EndianStream, WriteVectorLE) {<br>
> +  SmallString<16> Data;<br>
> +<br>
> +  {<br>
> +    raw_svector_ostream OS(Data);<br>
> +    endian::Writer<little> LE(OS);<br>
> +    std::vector<uint16_t> Vec{0x1234, 0x5678};<br>
> +    LE.write<uint16_t>(Vec);<br>
> +  }<br>
> +<br>
> +  EXPECT_EQ(static_cast<uint8_t>(Data[0]), 0x34);<br>
> +  EXPECT_EQ(static_cast<uint8_t>(Data[1]), 0x12);<br>
> +  EXPECT_EQ(static_cast<uint8_t>(Data[2]), 0x78);<br>
> +  EXPECT_EQ(static_cast<uint8_t>(Data[3]), 0x56);<br>
> +}<br>
><br>
>  } // end anon namespace<br>
</div></div></blockquote></div><br></div></div>