rsmith added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:198-200
+def err_size_t_literal_too_large: Error<
+  "%select{signed |}0'size_t' literal is out of range of possible "
+  "%select{signed |}0'size_t' values">;
----------------
I wonder if it'd be better to say `'ssize_t'` instead of `signed 'size_t'` 
here? The latter sounds self-contradictory since `size_t` is an unsigned type.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:3895
       // be an unsigned int.
       bool AllowUnsigned = Literal.isUnsigned || Literal.getRadix() != 10;
 
----------------
AntonBikineev wrote:
> I now begin to think that we should probably also prohibit things like 
> 0x1234z to be implicitly interpreted as unsigned. Wdyt?
Table 8 says that a `z`-suffixed literal has type `ssize_t` (if the value fits) 
or `size_t` (otherwise), so I think taking `AllowUnsigned` into account is 
correct (although perhaps surprising).


================
Comment at: clang/lib/Sema/SemaExpr.cpp:3997
       // does not fit in a signed long long, but has no U suffix.
       if (Ty.isNull()) {
         Diag(Tok.getLocation(), 
diag::ext_integer_literal_too_large_for_signed);
----------------
AntonBikineev wrote:
> I think this branch should also be covered, since we probably don't want 
> promotion from size_t to ULL but instead have a separate diagnostic that 
> size_t is out-of-range. I've added another diagnostic and branch here.
Thanks, that makes sense to me. I agree that a `z`-suffixed literal should 
never have any type other than `size_t` or `ssize_t`; we don't want to fall 
back to `unsigned long long` as an extension.


================
Comment at: clang/test/SemaCXX/size_t-literal.cpp:78
+
+  (void)0x80000000z; //cxx2b-32-error {{signed 'size_t' literal is out of 
range of possible signed 'size_t' values}}
+  (void)0x80000000uz;
----------------
This case looks valid to me.


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

Reply via email to