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

Enea Zaffanella zaffanella at cs.unipr.it
Wed Nov 2 00:36:09 PDT 2011


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


The case of explictly signed/unsigned char template arguments should be
considered too. Currently clang produces a character literal having a
signed/unsigned char type in those cases, but such a character literal
should typically have plain char type. Would it be possible to improve
the current patch so that an integer literal is used instead (with the
appropriate cast)?


Regarding the kind of cast: I am fully aware that here it is mainly a
matter of taste ... but wouldn't it be slightly better to use explicit
casts, so that the required conversions are more clearly readable from
the pretty printed statement?

For instance, consider the following:

  template <short v> struct S {
    static const long a = v;
  };

  S<0> s;

Here, when initializing the static member `a', the short value `v' is
implicitly converted to long. If we go for implicit casts, we will
obtain a template instance where the 0 integer literal is first
implicitly converted to short and then implicitly converted to long,
which is somehow strange looking. An explicit cast from the integer
literal to short may serve as a better hint for the reader regarding the
fact that such a 0 constant value was originally having type short.

Cheers,
Enea.


> Basically, protect IntegerLiteral being shorts, fix the only case of
> short IntegerLiterals, and add support for printing int128
> IntegerLiterals.  
> 
> PR:
> http://llvm.org/bugs/show_bug.cgi?id=11179
> 
> Patch also located at:
> http://codereview.appspot.com/5309045/
> 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list