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

Richard Smith richard at metafoo.co.uk
Tue Nov 1 22:41:04 PDT 2011


On Wed, November 2, 2011 02:21, Richard Trieu wrote:
> On Mon, Oct 24, 2011 at 7:30 AM, Douglas Gregor <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> 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.
>
> 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/

I've left some comments there.

Best wishes,
Richard




More information about the cfe-commits mailing list