[cfe-commits] [PATCH] Improved handling of 128-bit integer literals

[Stephen Canon]

> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp	(revision 168447)
> +++ lib/Sema/SemaExpr.cpp	(working copy)
> [...]
> +      // Microsoft literals simply *are* the specified type, regardless of
> +      // whether the value fits into it or not, or whether they are hex or
> +      // decimal or octal.
> +      if (Literal.isLongLong) {
> +        Width = Context.getTargetInfo().getLongLongWidth();
> +        Ty = Literal.isUnsigned ? Context.UnsignedLongLongTy :
> +          Context.LongLongTy;
> +      } else if (Literal.isLong) {
> +        Width = Context.getTargetInfo().getLongWidth();
> +        Ty = Literal.isUnsigned ? Context.UnsignedLongTy : Context.LongTy;
> +      } else {
> +        Width = Context.getTargetInfo().getIntWidth();
> +        Ty = Literal.isUnsigned ? Context.UnsignedIntTy : Context.IntTy;
>        }
> 
> Hmm, it looks incorrect to use isLong here instead of checking microsoftWidth. In particular, if I use -fms-extensions on Linux, this static_assert fails:
> 
> static_assert(sizeof(0i32) == 4, "");
> 
> ... because 'long' is a 64-bit type on my target. (We also incorrectly use 'int' for i16 and i8.) We should presumably use the smallest integer type which is at least N bits wide. I'm OK with this being dealt with in a separate patch, since it's not a regression.

I'll take a look at these in another patch.

> +      if (ResultVal.getActiveBits() > (Literal.isUnsigned ?
> +                                       Literal.microsoftWidth :
> +                                       Literal.microsoftWidth - 1)) {
> +        Diag(Tok.getLocation(), diag::warn_integer_too_large);
> +        // APInt asserts on NOP-truncations, so we need to be a little bit
> +        // carefull; otherwise we would crash when the literal is signed and
> +        // microsoftWidth == Width.
> +        if (Literal.microsoftWidth < Width)
> +          ResultVal = ResultVal.trunc(Literal.microsoftWidth);
> 
> This can be simplified somewhat by passing Literal.microsoftWidth instead of 64 when constructing ResultVal (you'd still need to check for the high bit being set for a signed literal).

Sounds good.

>        }
> +      
> +      // Unlike in the non-MS-literal case, we may have already truncated the
> +      // APInt to be smaller than the final literal, so we may need to adjust
> +      // the size in either direciton here; use sextOrTrunc instead of trunc.
> +      if (ResultVal.getBitWidth() != Width)
> +        ResultVal = ResultVal.sextOrTrunc(Width);
> 
> You should zext if the literal is unsigned (maybe make ResultVal an APSInt, and use extOrTrunc). For instance, 0x18000Ui16 will currently first be truncated to 0x8000 (with a warning), then sign extended to 0x80000000, rather than producing the desired value 0x00008000.

Yep, I'll fix this.

> +      // Checking for 64-bit pointers is perhaps not the correct way to detect
> +      // i128 support, but it seems to be what is done elsewhere.
> +      if (PP.getTargetInfo().getPointerWidth(0) >= 64) {
> 
> TargetInfo now has a hasInt128Type().

Great, that's very helpful.

- Steve