[PATCH] D99456: [C++2b] Support size_t literals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

Thanks for working on this! I think the direction is good in general, but I think we should also add tests for use in the preprocessor (`#if 1z == 1`, etc) as well as tests for the behavior in C.



================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:191
+def ext_cxx2b_size_t_suffix : Extension<
+  "size_t suffix for literals is a C++2b extension">,
+  InGroup<CXX2b>;
----------------



================
Comment at: clang/include/clang/Lex/LiteralSupport.h:66
   bool isLongLong : 1;
+  bool isSizeT : 1;         // C++2b's z/uz size_t literals
   bool isHalf : 1;          // 1.0h
----------------



================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:593-595
+  if (LangOpts.CPlusPlus2b) {
+    Builder.defineMacro("__cpp_size_t_suffix", "202011L");
+  }
----------------



================
Comment at: clang/lib/Lex/LiteralSupport.cpp:682
+        break; // invalid for floats.
+      if (isMicrosoftInteger)
+        break; // invalid for Microsoft integers.
----------------



================
Comment at: clang/lib/Lex/PPExpressions.cpp:325-327
+    if (!PP.getLangOpts().CPlusPlus2b && Literal.isSizeT) {
+      PP.Diag(PeekTok, diag::ext_cxx2b_size_t_suffix);
+    }
----------------



================
Comment at: clang/lib/Lex/PPExpressions.cpp:326
+    if (!PP.getLangOpts().CPlusPlus2b && Literal.isSizeT) {
+      PP.Diag(PeekTok, diag::ext_cxx2b_size_t_suffix);
+    }
----------------
This feature is specific to C++ and needs some sort of diagnostic in C. There is not currently a WG14 proposal for this literal suffix, so I think this should be an error in that case rather than a warning.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:3871-3873
+    if (!getLangOpts().CPlusPlus2b && Literal.isSizeT) {
+      Diag(Tok.getLocation(), diag::ext_cxx2b_size_t_suffix);
+    }
----------------



================
Comment at: clang/lib/Sema/SemaExpr.cpp:3872
+    if (!getLangOpts().CPlusPlus2b && Literal.isSizeT) {
+      Diag(Tok.getLocation(), diag::ext_cxx2b_size_t_suffix);
+    }
----------------
Same diagnostic needs for C here as above.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:3911
+      if (Literal.isSizeT) {
+        assert(Ty.isNull() && "size_t literals can't be Microsoft literals");
+        unsigned SizeTSize = Context.getTargetInfo().getTypeWidth(
----------------
The assert message doesn't seem to match the predicate -- why does a null qualtype imply it's a microsoft literal?


================
Comment at: clang/test/SemaCXX/size_t-literal.cpp:14-16
+#if __cplusplus >= 202101L
+//  expected-no-diagnostics
+#endif
----------------
Rather than check `__cplusplus` like this, I think the RUN lines should specify a verify prefix. e.g., `-verify=cxx2b` and then use `cxx2b-no-diagnostics` and `-verify=cxx20` and then use `cxx20-warning {{}}`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99456/new/

https://reviews.llvm.org/D99456