[cfe-commits] [Patch] PR11179 - Update StmtPrinter to not complain on some IntegerLiteral types

[Richard Trieu]

On Sun, Nov 6, 2011 at 5:07 PM, Douglas Gregor <dgregor at apple.com> wrote:

>
>
> Sent from my iPhone
>
> On Nov 4, 2011, at 1:02 PM, Richard Trieu <rtrieu at google.com> wrote:
>
> On Thu, Nov 3, 2011 at 3:06 AM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> On Thu, Nov 3, 2011 at 2:43 AM, Abramo Bagnara <abramo.bagnara at gmail.com>wrote:
>>
>>> Il 03/11/2011 00:49, Richard Trieu ha scritto:
>>> > On Wed, Nov 2, 2011 at 4:21 PM, Abramo Bagnara <
>>> abramo.bagnara at gmail.com
>>> > <mailto:abramo.bagnara at gmail.com>> wrote:
>>> >
>>> >     Il 03/11/2011 00:16, Richard Trieu ha scritto:
>>> >     > On Wed, Nov 2, 2011 at 3:51 PM, Abramo Bagnara
>>> >     <abramo.bagnara at gmail.com <mailto:abramo.bagnara at gmail.com>
>>> >     > <mailto:abramo.bagnara at gmail.com
>>> >     <mailto:abramo.bagnara at gmail.com>>> wrote:
>>> >     >
>>> >     >     Il 02/11/2011 03:21, Richard Trieu ha scritto:
>>> >     >     > On Mon, Oct 24, 2011 at 7:30 AM, Douglas Gregor
>>> >     <dgregor at apple.com <mailto:dgregor at apple.com>
>>> >     >     <mailto:dgregor at apple.com <mailto:dgregor at apple.com>>
>>> >     >     > <mailto:dgregor at apple.com <mailto:dgregor at apple.com>
>>> >     <mailto:dgregor at apple.com <mailto:dgregor at apple.com>>>> wrote:
>>> >     >     >
>>> >     >     >
>>> >     >     >     On Oct 20, 2011, at 5:36 PM, Chandler Carruth wrote:
>>> >     >     >
>>> >     >     >>     On Thu, Oct 20, 2011 at 5:12 PM, Richard Trieu
>>> >     >     <rtrieu at google.com <mailto:rtrieu at google.com>
>>> >     <mailto:rtrieu at google.com <mailto:rtrieu at google.com>>
>>> >     >     >>     <mailto:rtrieu at google.com <mailto:rtrieu at google.com>
>>> >     <mailto:rtrieu at google.com <mailto:rtrieu at google.com>>>> wrote:
>>> >     >     >>
>>> >     >     >>         Should Clang be printing suffixes that are
>>> accepted
>>> >     only with
>>> >     >     >>         certain flags?
>>> >     >     >>
>>> >     >     >>
>>> >     >     >>     I think this is an interesting policy decision. I'd
>>> >     love to hear
>>> >     >     >>     Doug's thoughts on it.
>>> >     >     >>
>>> >     >     >>     It seems fine to me for Clang, when running with
>>> >     -fms-extensions,
>>> >     >     >>     to suggest fixes even if only valid for
>>> >     -fms-extensions. Clearly
>>> >     >     >>     if there is a generic suggestion that could be made,
>>> that
>>> >     >     would be
>>> >     >     >>     a preferred alternative. For example, '__asm__'
>>> should be
>>> >     >     >>     suggested before 'asm'.
>>> >     >     >
>>> >     >     >     I think it's fine for Clang to print suffixes that are
>>> only
>>> >     >     accepted
>>> >     >     >     with certain flags. Presumably, you should never get an
>>> >     >     >     IntegerLiteral of type __int128_t unless you're in a
>>> >     dialect that
>>> >     >     >     supports parsing it.
>>> >     >     >
>>> >     >     >     … except that we cheat when we're building template
>>> >     arguments,
>>> >     >     >     because it was convenient. That cheating could be
>>> >     eliminated by
>>> >     >     >     encoding integer literal values directly within
>>> >     >     >     SubstNonTypeTemplateParmExpr.
>>> >     >     >
>>> >     >     >     - Doug
>>> >     >     >
>>> >     >     >
>>> >     >     > New patch.  Changes as follows:
>>> >     >     > Add int128 and uint128 suffixes (i128 and Ui128) to
>>> StmtPrinter.
>>> >     >      short
>>> >     >     > and unsigned short will get to llvm_unreachable
>>> >     >     > Add assert to IntergerLiteral to prevent creation with type
>>> >     short or
>>> >     >     > unsigned short
>>> >     >     > Fix comment in IntegerLiteral to say that int128 and
>>> uint128 are
>>> >     >     > acceptable types
>>> >     >     > Change BuildExpressFromIntegralTemplateArgument to give a
>>> >     proper Expr.
>>> >     >     >  For negative numbers, use UnaryOperator of IntegerLiteral.
>>> >      For short
>>> >     >     > and unsigned short, ImplicitCastExpr from int.
>>> >     >
>>> >     >     Following this path have you considered how to represent an
>>> int
>>> >     >     parameter with value INT_MIN?
>>> >     >
>>> >     >     I think there is no way you can represent it in standard
>>> >     conformant way
>>> >     >     except to use -INT_MAX - 1 (i.e. -2147483647 <tel:2147483647
>>> >
>>> >     <tel:2147483647 <tel:2147483647>> - 1 if
>>> >     >     int has 32 bits).
>>> >     >
>>> >     >     You cannot use -214783648 because no integer literal of type
>>> >     int with
>>> >     >     value 214783648 can exists.
>>> >     >
>>> >     >
>>> >     > Richard Smith brought up a similar point in the code review
>>> >     comments.  I
>>> >     > am putting in an integer type selector which will move up to
>>> larger
>>> >     > types until a large enough type can be found.  For instance, for
>>> >     > int_min, use the negation of a long with a cast back to int.
>>> >
>>> >     What about when you don't have a larger type? (i.e. the minimum
>>> negative
>>> >     number of larger signed integral type)
>>> >
>>> >
>>> > I'm thinking of switching to the unsigned version of that signed type.
>>> >  And if that is still not large enough, assert, because there's nothing
>>> > left Clang can do.
>>>
>>> This kind of limitation (and complexity) make me think whether is a
>>> better idea the proposal from Douglas to avoid an ordinary expression
>>> child for SubstNonTypeTemplateParmExpr and instead replace it with either
>>>
>>> - an APInt encapsulated in SubstNonTypeTemplateParmExpr
>>> - a special purpose Expr node for non syntactic integer literal
>>> - a flag in IntegerLiteral to mark it as non-syntactic and to represent
>>> its ability to have arbitrary integral type and signed values
>>>
>>> What do you think about this alternative?
>>
>>
>> Sorry, I talked extensively with one Richard about this, and apparently
>> the other (hehe) didn't hear me (my bad). I strongly agree with Doug here.
>> I intensely dislike the abuse of IntegerLiteral here. In particular, I
>> think that these (and other similar constructs) really need dedicated AST
>> types in order to correctly capture their semantics and how they are
>> spelled in the code. Consider, with this we could actually tell the user
>> what expression in the source code was used when (initially) deducing a
>> particular value.
>>
>> Richard Smith and I sketched out a pretty rough set of AST nodes that
>> would make a lot of sense here. It would also begin making the AST for
>> constant expressions generally more useful and reasonable by making them
>> clearly set off from your average "expression".
>>
>> Maybe both Richards and Doug can get a game-plan together for such a
>> solution?
>>
>
> Doug,
>
> Would it be alright to submit the first patch with fix me comments that
> will fix the crasher in the StmtPrinter?  And when we finish up work on the
> AST nodes, we can remove it later
>
>
> Yes, that's fine.
>
r143977
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111107/35cc5d4c/attachment.html>