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

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev marked 9 inline comments as done.
AntonBikineev added a comment.

Thanks Aaron for taking a look! Addressed the comments. I also hope it's okay to test preprocessor in the same test (test/Lexer/size_t-literal.cpp).



================
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(
----------------
aaron.ballman wrote:
> The assert message doesn't seem to match the predicate -- why does a null qualtype imply it's a microsoft literal?
Hm, agree, that's probably misleading. I've tried to check that the previous branch couldn't be taken if this one has been. I think "assert(!Literal.MicrosoftInteger && ...);" would make more sense.


================
Comment at: clang/test/SemaCXX/size_t-literal.cpp:14-16
+#if __cplusplus >= 202101L
+//  expected-no-diagnostics
+#endif
----------------
aaron.ballman wrote:
> 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 {{}}`.
Good idea, thanks! That has made the test much cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456