Lex: Use the correct types for MS integer suffixes

[majnemer]

Summary

We didn't properly implement support for the sized integer suffixes.
Suffixes like i16 were essentially ignored instead of mapping them to
the appropriately sized integer type.

This fixes PR20008.

[rsmith]

lib/AST/StmtPrinter.cpp

914 ↗

(On Diff #10382)

This comment is out of date.

lib/Sema/SemaExpr.cpp

3189 ↗

(On Diff #10382)

May as well use MicrosoftInteger == 128 now.

3217 ↗

(On Diff #10382)

Does this do the right thing if long long is 128 bits wide?

test/SemaCXX/ms_integer_suffix.cpp

8 ↗

(On Diff #10382)

I think this will fail if the host does not support __int128 (32 bit targets don't).

[majnemer]

• Address review comments

[rnk]

lgtm

[majnemer]

Closed by commit rL211426 (authored by @majnemer).

cfe/trunk/include/clang/Lex/LiteralSupport.h

NumericLiteralParser(StringRef TokSpelling,
                        SourceLocation TokLoc,
                        Preprocessor &PP);
   bool hadError;
   bool isUnsigned;
   bool isLong;        // This is *not* set for long long.
   bool isLongLong;
   bool isFloat;       // 1.0f
   bool isImaginary;   // 1.0i
-  bool isMicrosoftInteger;  // Microsoft suffix extension i8, i16, i32, or i64.
+  uint8_t MicrosoftInteger;  // Microsoft suffix extension i8, i16, i32, or i64.
 
   bool isIntegerLiteral() const {
     return !saw_period && !saw_exponent;
   }
   bool isFloatingLiteral() const {
     return saw_period || saw_exponent;
   }
 

cfe/trunk/lib/AST/StmtPrinter.cpp

void StmtPrinter::VisitIntegerLiteral(IntegerLiteral *Node) {
   OS << Node->getValue().toString(10, isSigned);
 
   // Emit suffixes.  Integer literals are always a builtin integer type.
   switch (Node->getType()->getAs<BuiltinType>()->getKind()) {
   default: llvm_unreachable("Unexpected type for integer literal!");
   // FIXME: The Short and UShort cases are to handle cases where a short
   // integeral literal is formed during template instantiation.  They should
   // be removed when template instantiation no longer needs integer literals.
-  case BuiltinType::Short:
-  case BuiltinType::UShort:
+  case BuiltinType::SChar:     OS << "i8"; break;
+  case BuiltinType::UChar:     OS << "Ui8"; break;
+  case BuiltinType::Short:     OS << "i16"; break;
+  case BuiltinType::UShort:    OS << "Ui16"; break;
   case BuiltinType::Int:       break; // no suffix.
   case BuiltinType::UInt:      OS << 'U'; break;
   case BuiltinType::Long:      OS << 'L'; break;
   case BuiltinType::ULong:     OS << "UL"; break;
   case BuiltinType::LongLong:  OS << "LL"; break;
   case BuiltinType::ULongLong: OS << "ULL"; break;
   case BuiltinType::Int128:    OS << "i128"; break;
   case BuiltinType::UInt128:   OS << "Ui128"; break;

cfe/trunk/lib/Lex/LiteralSupport.cpp

NumericLiteralParser::NumericLiteralParser(StringRef TokSpelling,
   saw_exponent = false;
   saw_period = false;
   saw_ud_suffix = false;
   isLong = false;
   isUnsigned = false;
   isLongLong = false;
   isFloat = false;
   isImaginary = false;
-  isMicrosoftInteger = false;
+  MicrosoftInteger = 0;
   hadError = false;
 
   if (*s == '0') { // parse radix
     ParseNumberStartingWithZero(TokLoc);
     if (hadError)
       return;
   } else { // the first digit is non-zero
     radix = 10;
case 'L':
         ++s;  // Eat both of them.
       } else {
         isLong = true;
       }
       continue;  // Success.
     case 'i':
     case 'I':
       if (PP.getLangOpts().MicrosoftExt) {
-        if (isLong || isLongLong) break;
+        if (isLong || isLongLong || MicrosoftInteger)
+          break;
 
         // Allow i8, i16, i32, i64, and i128.
         if (s + 1 != ThisTokEnd) {
           switch (s[1]) {
             case '8':
               if (isFPConstant) break;
               s += 2; // i8 suffix
-              isMicrosoftInteger = true;
+              MicrosoftInteger = 8;
               break;
             case '1':
               if (isFPConstant) break;
               if (s + 2 == ThisTokEnd) break;
               if (s[2] == '6') {
                 s += 3; // i16 suffix
-                isMicrosoftInteger = true;
+                MicrosoftInteger = 16;
               }
               else if (s[2] == '2') {
                 if (s + 3 == ThisTokEnd) break;
                 if (s[3] == '8') {
                   s += 4; // i128 suffix
-                  isMicrosoftInteger = true;
+                  MicrosoftInteger = 128;
                 }
               }
               break;
             case '3':
               if (isFPConstant) break;
               if (s + 2 == ThisTokEnd) break;
               if (s[2] == '2') {
                 s += 3; // i32 suffix
-                isLong = true;
-                isMicrosoftInteger = true;
+                MicrosoftInteger = 32;
               }
               break;
             case '6':
               if (isFPConstant) break;
               if (s + 2 == ThisTokEnd) break;
               if (s[2] == '4') {
                 s += 3; // i64 suffix
-                isLongLong = true;
-                isMicrosoftInteger = true;
+                MicrosoftInteger = 64;
               }
               break;
             default:
               break;
           }
-          if (isMicrosoftInteger)
+          if (MicrosoftInteger)
             break;
         }
       }
       // "i", "if", and "il" are user-defined suffixes in C++1y.
       if (PP.getLangOpts().CPlusPlus1y && *s == 'i')
         break;
       // fall through.
     case 'j':
if (s != ThisTokEnd) {
     if (isValidUDSuffix(PP.getLangOpts(), UDSuffixBuf)) {
       // Any suffix pieces we might have parsed are actually part of the
       // ud-suffix.
       isLong = false;
       isUnsigned = false;
       isLongLong = false;
       isFloat = false;
       isImaginary = false;
-      isMicrosoftInteger = false;
+      MicrosoftInteger = 0;
 
       saw_ud_suffix = true;
       return;
     }
 
     // Report an error if there are any.
     PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, SuffixBegin - ThisTokBegin),
             isFPConstant ? diag::err_invalid_suffix_float_constant :

cfe/trunk/lib/Sema/SemaExpr.cpp

if (Literal.isFloatingLiteral()) {
     }
 
     // Get the value in the widest-possible width.
     unsigned MaxWidth = Context.getTargetInfo().getIntMaxTWidth();
     // The microsoft literal suffix extensions support 128-bit literals, which
     // may be wider than [u]intmax_t.
     // FIXME: Actually, they don't. We seem to have accidentally invented the
     //        i128 suffix.
-    if (Literal.isMicrosoftInteger && MaxWidth < 128 &&
+    if (Literal.MicrosoftInteger && MaxWidth < 128 &&
         Context.getTargetInfo().hasInt128Type())
       MaxWidth = 128;
     llvm::APInt ResultVal(MaxWidth, 0);
 
     if (Literal.GetIntegerValue(ResultVal)) {
       // If this value didn't fit into uintmax_t, error and force to ull.
       Diag(Tok.getLocation(), diag::err_integer_too_large);
       Ty = Context.UnsignedLongLongTy;
       assert(Context.getTypeSize(Ty) == ResultVal.getBitWidth() &&
              "long long is not intmax_t?");
     } else {
       // If this value fits into a ULL, try to figure out what else it fits into
       // according to the rules of C99 6.4.4.1p5.
 
       // Octal, Hexadecimal, and integers with a U suffix are allowed to
       // be an unsigned int.
       bool AllowUnsigned = Literal.isUnsigned || Literal.getRadix() != 10;
 
       // Check from smallest to largest, picking the smallest type we can.
       unsigned Width = 0;
-      if (!Literal.isLong && !Literal.isLongLong) {
+
+      // Microsoft specific integer suffixes are explicitly sized.
+      if (Literal.MicrosoftInteger) {
+        Width = Literal.MicrosoftInteger;
+        if (Width < 128)
+          Ty = Context.getIntTypeForBitwidth(Width,
+                                             /*Signed=*/!Literal.isUnsigned);
+        else
+          Ty = Literal.isUnsigned ? Context.UnsignedInt128Ty : Context.Int128Ty;
+      }
+
+      if (Ty.isNull() && !Literal.isLong && !Literal.isLongLong) {
         // Are int/unsigned possibilities?
         unsigned IntSize = Context.getTargetInfo().getIntWidth();
 
         // Does it fit in a unsigned int?
         if (ResultVal.isIntN(IntSize)) {
           // Does it fit in a signed int?
           if (!Literal.isUnsigned && ResultVal[IntSize-1] == 0)
             Ty = Context.IntTy;
if (Literal.GetIntegerValue(ResultVal)) {
           if (!Literal.isUnsigned && (ResultVal[LongLongSize-1] == 0 ||
               (getLangOpts().MicrosoftExt && Literal.isLongLong)))
             Ty = Context.LongLongTy;
           else if (AllowUnsigned)
             Ty = Context.UnsignedLongLongTy;
           Width = LongLongSize;
         }
       }
-        
-      // If it doesn't fit in unsigned long long, and we're using Microsoft
-      // extensions, then its a 128-bit integer literal.
-      if (Ty.isNull() && Literal.isMicrosoftInteger &&
-          Context.getTargetInfo().hasInt128Type()) {
-        if (Literal.isUnsigned)
-          Ty = Context.UnsignedInt128Ty;
-        else
-          Ty = Context.Int128Ty;
-        Width = 128;
-      }
-
       // If we still couldn't decide a type, we probably have something that
       // does not fit in a signed long long, but has no U suffix.
       if (Ty.isNull()) {
         Diag(Tok.getLocation(), diag::ext_integer_too_large_for_signed);
         Ty = Context.UnsignedLongLongTy;
         Width = Context.getTargetInfo().getLongLongWidth();
       }
 

cfe/trunk/test/SemaCXX/ms_integer_suffix.cpp

+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fms-compatibility -verify %s
+// expected-no-diagnostics
+
+static_assert(sizeof(0i8  ) == sizeof(__INT8_TYPE__ ), "");
+static_assert(sizeof(0i16 ) == sizeof(__INT16_TYPE__), "");
+static_assert(sizeof(0i32 ) == sizeof(__INT32_TYPE__), "");
+static_assert(sizeof(0i64 ) == sizeof(__INT64_TYPE__), "");
+static_assert(sizeof(0i128) >  sizeof(__INT64_TYPE__), "");

cfe/trunk/unittests/AST/StmtPrinterTest.cpp

PrintedStmtCXX11Matches(StringRef Code, const StatementMatcher &NodeMatch,
   return PrintedStmtMatches(Code, Args, NodeMatch, ExpectedPrinted);
 }
 
 ::testing::AssertionResult PrintedStmtMSMatches(
                                               StringRef Code,
                                               StringRef ContainingFunction,
                                               StringRef ExpectedPrinted) {
   std::vector<std::string> Args;
+  Args.push_back("-target");
+  Args.push_back("i686-pc-win32");
   Args.push_back("-std=c++98");
   Args.push_back("-fms-extensions");
   Args.push_back("-Wno-unused-value");
   return PrintedStmtMatches(Code,
                             Args,
                             functionDecl(hasName(ContainingFunction),
                                          has(compoundStmt(has(stmt().bind("id"))))),
                             ExpectedPrinted);
TEST(StmtPrinter, TestMSIntegerLiteral) {
   ASSERT_TRUE(PrintedStmtMSMatches(
     "void A() {"
     "  1i8, -1i8, 1ui8, "
     "  1i16, -1i16, 1ui16, "
     "  1i32, -1i32, 1ui32, "
     "  1i64, -1i64, 1ui64;"
     "}",
     "A",
+    "1i8 , -1i8 , 1Ui8 , "
+    "1i16 , -1i16 , 1Ui16 , "
     "1 , -1 , 1U , "
-    "1 , -1 , 1U , "
-    "1L , -1L , 1UL , "
     "1LL , -1LL , 1ULL"));
     // Should be: with semicolon
 }
 
 TEST(StmtPrinter, TestFloatingPointLiteral) {
   ASSERT_TRUE(PrintedStmtCXX98Matches(
     "void A() { 1.0f, -1.0f, 1.0, -1.0, 1.0l, -1.0l; }",
     "A",