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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
