[cfe-commits] [Patch] PR11179 - Update StmtPrinter to not complain on some IntegerLiteral types
Douglas Gregor
dgregor at apple.com
Sun Nov 6 17:07:43 PST 2011
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111106/6ab05edb/attachment.html>
More information about the cfe-commits
mailing list