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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
