[cfe-commits] Patch - Fix range-based for-loop so that it forms proper integer literals

[Richard Smith]

Hi Richard,

On Tue, April 26, 2011 22:49, Richard Trieu wrote:
> In this case, the integer in APInt is 64 bits (returned from
> CAT->getSize()), while the type given is sized as 32 bits (Context.IntTy).
>  A truncate instead of an extend is required.  Is getSize() returning
> with a larger bit width than necessary or can the array index exceed the
> size of an int?

The array bound can exceed the size of an int. However, it cannot exceed
the size of a ptrdiff_t: see [support.types]p5.

> Also, if we do proceed this way, the new integer literal
> creation method I wrote would not be used.  Will it still be useful or
> should I remove it?

Best to avoid dead code. Your patch will still be in the archives if we
want that code for anything.

Best regards,
Richard

> On Tue, Apr 26, 2011 at 5:50 AM, Richard Smith
> <richard at metafoo.co.uk>wrote:
>> Hi Richard,
>>
>> Thanks for catching this, and my apologies for the delay in getting
>> back to you.
>>
>> On Fri, 22 Apr 2011 21:26, Richard Trieu <rtrieu at google.com> wrote:
>>
>>> The code for range-based for-loops makes a bad integer literal
>>> expression which has a mis-match between the size of the literal and
>>> the integer type. This causes an assert to be thrown when
>>> IsIntegerConstant() is
>>> called on it. For this patch,
>>>
>>> 1) the assert that checks bit width is copied from
>>> IsIntegerConstant()
>>> into the integer literal constructor.
>>
>> This looks fine.
>>
>>
>>> 2) a new static function was written so that it automatically selects
>>>
>> the > correct size integer and returns a proper integer literal
>>>
>>> 3) the range-based for-loops now use the function in #2 to get the
>>> right integer literal
>>
>> I'd prefer for us to always use ptrdiff_t[*] for this (which is
>> guaranteed to be big enough), and to extend the integer to fit, rather
>> than picking a 'big enough' type based on the integer's value.
>>
>>
>> [*] I think this actually needs to be the appropriate ptrdiff_t for the
>>  address space of the array.
>>
>> Thanks!
>> Richard