Okay, I fixed the concerns you raised.  Is this better?<br><br><div class="gmail_quote">On Thu, Apr 28, 2011 at 2:32 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Hi Richard,<br>
<div class="im"><br>
On Tue, April 26, 2011 22:49, Richard Trieu wrote:<br>
> In this case, the integer in APInt is 64 bits (returned from<br>
> CAT->getSize()), while the type given is sized as 32 bits (Context.IntTy).<br>
>  A truncate instead of an extend is required.  Is getSize() returning<br>
> with a larger bit width than necessary or can the array index exceed the<br>
> size of an int? </div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im">
<br>
</div>The array bound can exceed the size of an int. However, it cannot exceed<br>
the size of a ptrdiff_t: see [support.types]p5.<br></blockquote><div><br></div><div>Range-based for-loops now use ptrdiff_t for the type.  This passes the new bit length check in the integer literal constructor.  Extending the integer wasn't necessary, so I didn't add code for it.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im"> </div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im">
> Also, if we do proceed this way, the new integer literal<br>
> creation method I wrote would not be used.  Will it still be useful or<br>
> should I remove it?<br>
<br>
</div>Best to avoid dead code. Your patch will still be in the archives if we<br>
want that code for anything.<br></blockquote><div><br></div><div>Removed my unused code. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
Best regards,<br>
<font color="#888888">Richard<br>
</font><div><div></div><div class="h5"><br>
> On Tue, Apr 26, 2011 at 5:50 AM, Richard Smith<br>
> <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>>wrote:<br>
>> Hi Richard,<br>
>><br>
>> Thanks for catching this, and my apologies for the delay in getting<br>
>> back to you.<br>
>><br>
>> On Fri, 22 Apr 2011 21:26, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>
>><br>
>>> The code for range-based for-loops makes a bad integer literal<br>
>>> expression which has a mis-match between the size of the literal and<br>
>>> the integer type. This causes an assert to be thrown when<br>
>>> IsIntegerConstant() is<br>
>>> called on it. For this patch,<br>
>>><br>
>>> 1) the assert that checks bit width is copied from<br>
>>> IsIntegerConstant()<br>
>>> into the integer literal constructor.<br>
>><br>
>> This looks fine.<br>
>><br>
>><br>
>>> 2) a new static function was written so that it automatically selects<br>
>>><br>
>> the > correct size integer and returns a proper integer literal<br>
>>><br>
>>> 3) the range-based for-loops now use the function in #2 to get the<br>
>>> right integer literal<br>
>><br>
>> I'd prefer for us to always use ptrdiff_t[*] for this (which is<br>
>> guaranteed to be big enough), and to extend the integer to fit, rather<br>
>> than picking a 'big enough' type based on the integer's value.<br>
>><br>
>><br>
>> [*] I think this actually needs to be the appropriate ptrdiff_t for the<br>
>>  address space of the array.<br>
>><br>
>> Thanks!<br>
>> Richard<br>
<br>
<br>
</div></div></blockquote></div><br>