Lex: add support for `{,u}i128` Microsoft extension

[compnerd]

Summary

The Windows SDK has occurrences of the i128 and ui128 suffix (at
least in the 10.0.22000.0 SDK, see intsafe.h). This adds support for
the extension to properly lex the literal.

[rnk]

Seems reasonable, this could use far more testing.

clang/test/Lexer/ms-extensions.c

27

This seems like it could use more testing. Check for other __int128 tests, and try adding a few of these literals to those tests with some basic arithmetic expressions.

In a similar vein, I believe there are already bugs that lurk behind __int128 non-type template parameters. It's worth setting up a few of those test cases. One of the main usages of the expr printer is to produce debug info for non-type template parameters. Do i128 suffixes come through there correctly now?

[aaron.ballman]

The Windows SDK has occurrences of the i128 and ui128 suffix (at least in the 10.0.22000.0 SDK, see intsafe.h). This adds support for the extension to properly lex the literal.

Something fishy is still going on here. I definitely see the use in intsafe.h, but it seems that i128 is not supported by MSVC nor is __int128 as a type: https://godbolt.org/z/K36baejq6 in neither C nor C++ mode, at least not for any architecture I can find. Were you running into compile errors with clang-cl when compiling intsafe.h, or was this more speculative that since it's in the header it must be supported? I have the distinct impression this is a bug in the Windows SDK: https://godbolt.org/z/sTx8W4a51.

[compnerd]

This was something that I was hitting an issue with. In particular, it was a module build for a module which pulled in intsafe.h. Now, given that it is a preprocessor macro, it would stand to reason that it will normally be dropped and thus won't matter if the frontend doesn't actually support a 128-bit type. However, with a module, we would attempt to process the expression. I do admit it is on a slightly more tenuous ground as Microsoft may somehow do something different.

[aaron.ballman]

In D121497#3383910, @compnerd wrote:

This was something that I was hitting an issue with.

Does cl hit the same issue when building a module that includes intsafe.h?

In particular, it was a module build for a module which pulled in intsafe.h. Now, given that it is a preprocessor macro, it would stand to reason that it will normally be dropped and thus won't matter if the frontend doesn't actually support a 128-bit type. However, with a module, we would attempt to process the expression. I do admit it is on a slightly more tenuous ground as Microsoft may somehow do something different.

I am by no means a modules expert (so I'm adding folks to the review who are for their input), but I guess I am naively surprised that a module build would evaluate the macro's expansion list when no code in the module actually expands the macro. So I can't tell whether the issue is that Clang is emitting an error when it shouldn't, or whether cl.exe is failing to emit an error when it should, or something else. But it seems wrong to me to add a Microsoft extension that cl.exe doesn't actually support; that's likely to lead to a fair amount of user confusion. But it could also lead to ABI issues later if Microsoft did decide to support this type but using slightly different calling conventions than Clang uses.

[iains]

could we see an example of the modular code that fails (and what flavour of modules compilation is being used)?
or is there already a PR for this?

[compnerd]

I don't have an example module sadly. It was something that I ran into with Swift code import the WinSDK module defined in https://github.com/apple/swift/blob/main/stdlib/public/Platform/winsdk.modulemap.

[iains]

hmm that makes it quite hard to analyse what's going on ...
.. however, it seems that this is a clang-modules style build (I am more familiar with the C++20 modules stuff, so probably @vsapsai will be able to throw more light on this).
however, I concur with @aaron.ballman that the original fail seems pretty fishy - at least for C++20 modules we do not do anything special in the pre-processor (with the exception of Header Units, but that code is not yet landed).

[aaron.ballman]

In D121497#3388024, @compnerd wrote:

I don't have an example module sadly. It was something that I ran into with Swift code import the WinSDK module defined in https://github.com/apple/swift/blob/main/stdlib/public/Platform/winsdk.modulemap.

Could this be the reproducer? https://godbolt.org/z/YbbMse9a4

[vsapsai]

I don't know how "intsafe.h" looks like and how it is involved with the modules (it's not in winsdk.modulemap as far as I can tell). My immediate guess would be that "intsafe.h" doesn't include all the headers that are supposed to provide necessary macros. And if "intsafe.h" is in a separate module, for the code

#define SOME_MACRO 1
#include "intsafe.h"

"intsafe.h" won't see SOME_MACRO. That's how it can end up with different macro expansions. But that is just a guess (that can be waay wrong) and you could have checked this scenario already.

[iains]

In D121497#3388870, @aaron.ballman wrote:

In D121497#3388024, @compnerd wrote:

I don't have an example module sadly. It was something that I ran into with Swift code import the WinSDK module defined in https://github.com/apple/swift/blob/main/stdlib/public/Platform/winsdk.modulemap.

Could this be the reproducer? https://godbolt.org/z/YbbMse9a4

For C++20 modules, the basic rejection of this code is correct; we told the compiler it was a module but the source is not a valid module file (the first non-comment token needs to be a module/expoert decl).
The diagnostic is not especially friendly - but it does not seem to me that the example involves any attempt to expand the macro - it is perhaps poor recovery from the initial error.

So, I m not sure if we've explained the original problem yet - or whether other modules "flavours" would permit a non-module directive to precede any valid one (I suspect that modules-ts has a more flexible allowance, where there is an implicit global module fragment)

[STREGA]

I just hit this issue in my project.

Here is intsafe.h from Windows 10 SDK 20348 (latest)

intsafe.h156 KBDownload

There are 3 macros with 128 bit suffixes; INT128_MAX, UINT128_MAX and INT128_MIN.

I couldn't find any references to any of these macros in the entire Windows SDK.

How can I help get this fix merged?

clang/include/clang/Lex/LiteralSupport.h

public:
   bool isLong : 1;          // This is *not* set for long long.
   bool isLongLong : 1;
   bool isSizeT : 1;         // 1z, 1uz (C++2b)
   bool isHalf : 1;          // 1.0h
   bool isFloat : 1;         // 1.0f
   bool isImaginary : 1;     // 1.0i
   bool isFloat16 : 1;       // 1.0f16
   bool isFloat128 : 1;      // 1.0q
-  uint8_t MicrosoftInteger; // Microsoft suffix extension i8, i16, i32, or i64.
-
+  uint8_t MicrosoftInteger; // Microsoft suffix extension i8, i16, i32, i64, or
+                            // i128.
   bool isFract : 1;         // 1.0hr/r/lr/uhr/ur/ulr
   bool isAccum : 1;         // 1.0hk/k/lk/uhk/uk/ulk
 
   bool isFixedPointLiteral() const {
     return (saw_period || saw_exponent) && saw_fixed_point_suffix;
   }
 
   bool isIntegerLiteral() const {

clang/lib/AST/StmtPrinter.cpp

   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:
-    break; // no suffix.
+    if (Context && Context->getLangOpts().MicrosoftExt)
+      OS << "i128";
+    break;
   case BuiltinType::UInt128:
-    break; // no suffix.
+    if (Context && Context->getLangOpts().MicrosoftExt)
+      OS << "ui128";
+    break;
   }
 }
 
 void StmtPrinter::VisitFixedPointLiteral(FixedPointLiteral *Node) {
   if (Policy.ConstantsAsWritten && printExprAsWritten(OS, Node, Context))
     return;
   OS << Node->getValueAsString(/*Radix=*/10);
 

clang/lib/Lex/LiteralSupport.cpp

case 'Z':
       if (HasSize)
         break;
       HasSize = true;
       isSizeT = true;
       continue;
     case 'i':
     case 'I':
       if (LangOpts.MicrosoftExt && !isFPConstant) {
-        // Allow i8, i16, i32, and i64. First, look ahead and check if
+        // Allow i8, i16, i32, i64, and i128. First, look ahead and check if
         // suffixes are Microsoft integers and not the imaginary unit.
         uint8_t Bits = 0;
         size_t ToSkip = 0;
         switch (s[1]) {
         case '8': // i8 suffix
           Bits = 8;
           ToSkip = 2;
           break;
         case '1':
-          if (s[2] == '6') { // i16 suffix
+          if (s[2] == '2' && s[3] == '8') {
+            Bits = 128;
+            ToSkip = 4;
+          } else if (s[2] == '6') { // i16 suffix
             Bits = 16;
             ToSkip = 3;
           }
           break;
         case '3':
           if (s[2] == '2') { // i32 suffix
             Bits = 32;
             ToSkip = 3;

clang/lib/Sema/SemaExpr.cpp

       Diag(Tok.getLocation(), getLangOpts().CPlusPlus
                                   ? getLangOpts().CPlusPlus2b
                                         ? diag::warn_cxx20_compat_size_t_suffix
                                         : diag::ext_cxx2b_size_t_suffix
                                   : diag::err_cxx2b_size_t_suffix);
 
     // Get the value in the widest-possible width.
     unsigned MaxWidth = Context.getTargetInfo().getIntMaxTWidth();
+    // Microsoft extensions allow a maximal integer greater than `intmax_t` via
+    // the `i128` suffix, creating a 128-bit integeral value with a 64-bit
+    // maximum integer type.
+    if (Literal.MicrosoftInteger)
+      MaxWidth = Literal.MicrosoftInteger;
     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_literal_too_large)
           << /* Unsigned */ 1;
       Ty = Context.UnsignedLongLongTy;
       assert(Context.getTypeSize(Ty) == ResultVal.getBitWidth() &&

clang/test/Lexer/ms-extensions.c

 __complex float c3 = 1.0if;
 
 // radar 7562363
 #define ULLONG_MAX 0xffffffffffffffffui64
 #define UINT 0xffffffffui32
 #define USHORT 0xffffui16
 #define UCHAR 0xffui8
 
+#define INT128_MAX  170141183460469231731687303715884105727i128
+#define UINT128_MAX 0xffffffffffffffffffffffffffffffffui128
+
 void a(void) {
   unsigned long long m = ULLONG_MAX;
   unsigned int n = UINT;
         unsigned short s = USHORT;
         unsigned char c = UCHAR;
 }
 
 void pr_7968(void)

clang/unittests/AST/StmtPrinterTest.cpp

 }
 
 static void PrintStmt(raw_ostream &Out, const ASTContext *Context,
                       const Stmt *S, PrintingPolicyAdjuster PolicyAdjuster) {
   assert(S != nullptr && "Expected non-null Stmt");
   PrintingPolicy Policy = Context->getPrintingPolicy();
   if (PolicyAdjuster)
     PolicyAdjuster(Policy);
-  S->printPretty(Out, /*Helper*/ nullptr, Policy);
+  S->printPretty(Out, /*Helper*/ nullptr, Policy, 0, "\n", Context);
 }
 
 template <typename Matcher>
 ::testing::AssertionResult
 PrintedStmtMatches(StringRef Code, const std::vector<std::string> &Args,
                    const Matcher &NodeMatch, StringRef ExpectedPrinted,
                    PrintingPolicyAdjuster PolicyAdjuster = nullptr) {
   return PrintedNodeMatches<Stmt>(Code, Args, NodeMatch, ExpectedPrinted, "",
ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX98,
     FunctionBodyMatcher("A"),
     "1 , -1 , 1U , 1U , "
     "1L , 1L , -1L , 1UL , 1UL , "
     "1LL , -1LL , 1ULL"));
     // Should be: with semicolon
 }
 
 TEST(StmtPrinter, TestMSIntegerLiteral) {
   ASSERT_TRUE(PrintedStmtMSMatches(
     "void A() {"
     "  1i8, -1i8, 1ui8, "
     "  1i16, -1i16, 1ui16, "
     "  1i32, -1i32, 1ui32, "
-    "  1i64, -1i64, 1ui64;"
+    "  1i64, -1i64, 1ui64, "
+    "  1i128, -1i128, 1ui128;"
     "}",
     FunctionBodyMatcher("A"),
     "1i8 , -1i8 , 1Ui8 , "
     "1i16 , -1i16 , 1Ui16 , "
     "1 , -1 , 1U , "
-    "1LL , -1LL , 1ULL"));
+    "1LL , -1LL , 1ULL , "
+    "1i128 , -1i128 , 1ui128"));
     // Should be: with semicolon
 }
 
 TEST(StmtPrinter, TestFloatingPointLiteral) {
   ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX98,
     "void A() { 1.0f, -1.0f, 1.0, -1.0, 1.0l, -1.0l; }",
     FunctionBodyMatcher("A"),
     "1.F , -1.F , 1. , -1. , 1.L , -1.L"));