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

[Abramo Bagnara]

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?