[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 2 inline comments as done.
aaron.ballman added inline comments.


================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:260
   DefineFmt(Prefix + Twine(TypeWidth), Ty, TI, Builder);
+  DefineTypeWidth(Prefix + Twine(TypeWidth) + "_WIDTH__", Ty, TI, Builder);
 }
----------------
erichkeane wrote:
> Minor-est of nits:  It would seem that TypeSize and TypeWidth should go together, just because of how similar they are.  Is there a reason to not put this line above the Fmt?
No reason not to move it, so move it I shall.


================
Comment at: clang/test/Headers/limits.cpp:33
 
-const bool char_is_signed = (char)-1 < (char)0;
-_Static_assert(CHAR_MIN == (char_is_signed ? -CHAR_MAX-1 : 0), "");
-_Static_assert(CHAR_MAX == (char_is_signed ? -(CHAR_MIN+1) : (char)~0ULL), "");
+_Static_assert(CHAR_MIN == (((char)-1 < (char)0) ? -CHAR_MAX-1 : 0), "");
+_Static_assert(CHAR_MAX == (((char)-1 < (char)0) ? -(CHAR_MIN+1) : (char)~0ULL), "");
----------------
erichkeane wrote:
> Is this change related?  Not sure I get what the change here is.
The original code would only compile in C++ mode; once I added a C RUN line, it failed because it wasn't a valid constant expression in C. This change makes it a valid constant expression in both C and C++ mode.


================
Comment at: clang/test/Headers/limits.cpp:62
+/* None of these are defined. */
+int BOOL_WIDTH, CHAR_WIDTH, SCHAR_WIDTH, UCHAR_WIDTH, USHRT_WIDTH, SHRT_WIDTH,
+    UINT_WIDTH, INT_WIDTH, ULONG_WIDTH, LONG_WIDTH, ULLONG_WIDTH, LLONG_WIDTH;
----------------
erichkeane wrote:
> Hmm... this is somewhat clever... perhaps overly so?  Can you improve the comment on 61?
FWIW, this follows the same pattern as what's on line 41. Do you have a suggested improvement (I can make it in both spots)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115253