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

[Richard Trieu]

Okay, I fixed the concerns you raised.  Is this better?

On Thu, Apr 28, 2011 at 2:32 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> 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.
>

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.

>
>
> 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.
>

Removed my unused code.

>
> 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110428/1dd3d423/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: IntegerLiteral2.patch
Type: text/x-patch
Size: 1935 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110428/1dd3d423/attachment.bin>