[PATCH] D34912: Handle cases where the value is too large to fit into the underlying type.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/AST/TemplateBase.cpp:64
   } else {
     Out << Val;
+    // Handle cases where the value is too large to fit into the underlying type
----------------
If `Val` is `LLONG_MIN`, this will still produce an integer too large for any signed type.


================
Comment at: lib/AST/TemplateBase.cpp:68
+    if (const BuiltinType *BT = T->getAs<BuiltinType>()) {
+      if (Val.isUnsigned() && Val.getBitWidth() == 64 && Val.isNegative())
+        Out << "ull";
----------------
rsmith wrote:
> This is not sufficient to ensure a correct round-trip in all cases; for `template<auto>`, we would need the type of the template argument to exactly match the type of the printed expression. But if you don't want to address that case now, that's fine.
We shouldn't be assuming that `long long` is exactly 64 bits here. Perhaps we could produce an `U`/`L`/`UL`/`LL`/`ULL` suffix based on the particular builtin type, regardless of the value itself?


================
Comment at: lib/AST/TemplateBase.cpp:68-69
+    if (const BuiltinType *BT = T->getAs<BuiltinType>()) {
+      if (Val.isUnsigned() && Val.getBitWidth() == 64 && Val.isNegative())
+        Out << "ull";
+    }
----------------
This is not sufficient to ensure a correct round-trip in all cases; for `template<auto>`, we would need the type of the template argument to exactly match the type of the printed expression. But if you don't want to address that case now, that's fine.


================
Comment at: lib/AST/TemplateBase.cpp:69
+      if (Val.isUnsigned() && Val.getBitWidth() == 64 && Val.isNegative())
+        Out << "ull";
+    }
----------------
This should be uppercase, to match how we pretty-print `IntegerLiteral`s.


https://reviews.llvm.org/D34912