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

Tue Dec 7 08:43:21 PST 2021

erichkeane added inline comments.

================
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), "");
----------------
aaron.ballman wrote:
> 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.
Ah, thanks for the clarification!

================
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;
----------------
aaron.ballman wrote:
> 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)?
Hrmph, so prior art here :(  It annoys me a touch that it ONLY works to ensure that they are not defined to something that would be an illegal identifier (so accidentally only defining INT_WIDTH and not UINT_WIDTH wouldn't be caught).

BUT I don't have a better idea other than some silliness of 
'#ifdef BOOL_WIDTH
 #error
#endif`

And I'm not sure my relatively minor concerns are worth the effort.

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

https://reviews.llvm.org/D115253