On Thu, Feb 17, 2011 at 3:55 PM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<div class="im"><br>
On Feb 10, 2011, at 1:40 PM, Talin wrote:<br>
<br>
> This patch adds a new "get" method to ConstantStruct, ConstantArray and ConstantVector. The new method accepts a pair of RandomAccessIterators, similar to other LLVM method signatures. This allows constants to be build using a small vector or other sequential container type more easily.<br>


<br>
</div>Hi Talin,<br>
<br>
I don't think that this is the right way to go.  Generic random access iterators are not guaranteed to be contiguous in memory (e.g. std::deque).<br></blockquote><div><br></div><div>OK - I was just following the patterns that I saw used elsewhere in LLVM. </div>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
Doesn't ArrayRef solve this need?<br></blockquote><div><br></div><div>Yes, although you'll have to manually call the ArrayRef constructor in cases like this instead of having it be implicit, but that's not a huge deal for me.</div>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im"><br>
> In addition, I've also added an END_WITH_NULL variant for ConstantStruct::get(). I only added it to ConstantStruct because I figured that it would be an uncommon use case for arrays and vectors, whereas the creation of small constant structs of predetermined length is fairly common.<br>


<br>
</div>This part seems fine to me,<br></blockquote><div><br></div><div>Good cause this is the part I really care about :)</div><div><br></div><div>Should I wait until you've converted ConstantStruct and friends to use ArrayRef before sending you a new patch? (I'd do it myself, except as you've discovered there may be other projects broken by such changes and you're in a better position to detect and correct those.)</div>

<div><br></div></div>-- <br>-- Talin<br>