rsmith added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:190
InGroup<CXX98CompatPedantic>, DefaultIgnore;
+def ext_cxx2b_size_t_suffix : Extension<
+ "'size_t' suffix for literals is a C++2b extension">,
----------------
Should this be an `ExtWarn`? I think we should warn on this by default.
================
Comment at: clang/lib/Lex/LiteralSupport.cpp:594-595
// Loop over all of the characters of the suffix. If we see something bad,
// we break out of the loop.
for (; s != ThisTokEnd; ++s) {
----------------
General comment: I think the checks here have become too complex and
error-prone. I suggest we add another flag, `hasSize`, that's set when we parse
any size modifier (`F`, `L`, `LL`, `H`, `F128`, `I64`, and now `Z`), and when
parsing any size modifier, replace the existing ad-hoc set of checks with a
check for `hasSize`.
================
Comment at: clang/lib/Lex/PPExpressions.cpp:325
+ // 'z/uz' literals are a C++2b feature.
+ if (!PP.getLangOpts().CPlusPlus2b && Literal.isSizeT)
+ PP.Diag(PeekTok, PP.getLangOpts().CPlusPlus
----------------
We should issue a compat warning when these literals are used in C++2b or
later. (That is, always issue a diagnostic for a `size_t` literal: in C, it's
an error, in C++20 and before, it's an extension warning, and in C++2b and
later, it's a backwards-compatibility warning.) There are a bunch of examples
of this elsewhere in the codebase -- look for the emission of
`warn_cxx.*_compat`.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:3871
+ // 'z/uz' literals are a C++2b feature.
+ if (!getLangOpts().CPlusPlus2b && Literal.isSizeT)
+ Diag(Tok.getLocation(), getLangOpts().CPlusPlus
----------------
Also need the C++20-and-before compat warning here for C++2b-onwards mode.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:3928
+
if (Ty.isNull() && !Literal.isLong && !Literal.isLongLong) {
// Are int/unsigned possibilities?
----------------
This should exclude the `isSizeT` case in addition to `isLong` and
`isLongLong`. (It probably doesn't matter, because `int` is not larger than
`size_t` on any platform we support, but nonetheless we should check.)
================
Comment at: clang/lib/Sema/SemaExpr.cpp:3944
// Are long/unsigned long possibilities?
if (Ty.isNull() && !Literal.isLongLong) {
unsigned LongSize = Context.getTargetInfo().getLongWidth();
----------------
Likewise here.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:3975
// Check long long if needed.
if (Ty.isNull()) {
unsigned LongLongSize = Context.getTargetInfo().getLongLongWidth();
----------------
Likewise here -- and this one actually does matter, because we have targets
with a 32-bit `size_t` but a 64-bit `long long`.
================
Comment at: clang/test/Lexer/size_t-literal.cpp:6-8
+#if 1uz != 1
+#error "size_t suffix must be recognized by preprocessor"
+#endif
----------------
Please also check `-1z < 0` and `-1zu < 0` to ensure that the preprocessor gets
the signedness correct.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99456/new/
https://reviews.llvm.org/D99456
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
