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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:900-903
+  Builder.defineMacro("__USHRT_WIDTH__", Twine(TI.getShortWidth()));
+  Builder.defineMacro("__UINT_WIDTH__", Twine(TI.getIntWidth()));
+  Builder.defineMacro("__ULONG_WIDTH__", Twine(TI.getLongWidth()));
+  Builder.defineMacro("__ULLONG_WIDTH__", Twine(TI.getLongLongWidth()));
----------------
jyknight wrote:
> This seems odd? We define `__LONG_LONG_MAX__` but not `__LONG_LONG_WIDTH__`, for signed long long, and we define `__ULLONG_WIDTH__` but not `__ULLONG_MAX__`, for unsigned long long?
> This seems odd?

Yeah, this stuff is a bit twisty.

> We define __LONG_LONG_MAX__ but not __LONG_LONG_WIDTH__, for signed long long,

Correct.

> and we define __ULLONG_WIDTH__ but not __ULLONG_MAX__, for unsigned long long?

Correct.

The basic approach I took was: for types with signed/unsigned pairs, define the `unsigned` variants with an underscore name and use them to define the `signed` variants in the header file because the width of signed and unsigned has to be the same per spec ("The macro blah_WIDTH represents the width of the type blah and shall expand to the same value as Ublah_WIDTH.") So that's why you generally only see the "unsigned" width variants added here with a double underscore name. I could expose the signed versions as well (like we do for many of the MAX macros), but that makes the preprocessor slower for no real benefit as we want the users to use the non-underscore versions of these macros whenever possible.


================
Comment at: clang/lib/Headers/stdint.h:728
+   in C2x mode; switch to the correct values once they've been published. */
+#if __STDC_VERSION__ >= 202000L
+/* NB: The C standard requires that these be the same value, but the compiler
----------------
jyknight wrote:
> Why are these conditioned on `__STDC_VERSION__` but the ones above, e.g. UINT64_WIDTH, are not?
Good catch, that's a think-o on my part! And it's even a think-o in the test files!
```
#if defined(INT64_MAX)
_Static_assert(INT64_WIDTH == 64, "");
_Static_assert(UINT64_WIDTH == INT64_WIDTH, "");
_Static_assert(INT64_WIDTH / __CHAR_BIT__ == sizeof(int64_t), "");
_Static_assert(UINT64_WIDTH / __CHAR_BIT__ == sizeof(uint64_t), "");
#else
int INT64_WIDTH, UINT64_WIDTH; /* None of these are defined. */
#endif
```
That predicate is wrong; it should be testing the standard version *and* the presence of the type, not just the presence of the type alone. (We always have the 8, 16, 32, and 64 types, so the test for "none of these are defined" is never run.)


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

https://reviews.llvm.org/D115253