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