aaron.ballman added inline comments.
================ Comment at: clang/lib/Headers/limits.h:55 #define UCHAR_MAX (__SCHAR_MAX__*2 +1) -#define USHRT_MAX (__SHRT_MAX__ *2 +1) +#define USHRT_MAX (__SHRT_MAX__ * 2U + 1U) #define UINT_MAX (__INT_MAX__ *2U +1U) ---------------- mysterymath wrote: > aaron.ballman wrote: > > It's worth double-checking that this still gives the correct type for the > > macro: > > > > C2x 5.2.4.2.1p2: For all unsigned integer types for which <limits.h> or > > <stdint.h> define a macro with suffix _WIDTH holding its width N, there is > > a macro with suffix _MAX holding the maximal value 2N − 1 that is > > representable by the type and that has the same type as would an expression > > that is an object of the corresponding type converted according to the > > integer promotions. ... > Ah, thanks; it hadn't occurred to me that the type of the expression would be > specified in the standard. It could be either `unsigned int` or `int`, > depending on the target. > > The most straightforward approach I could think of to produce the right type > is: > 1) Perform the arithmetic in `unsigned int` to produce the right value > 2) Cast to `unsigned short` to produce the right type > 3) Directly perform integer promotion using unary plus > > The use of unary plus is a bit odd here, but it seems like the most direct > way to express the logic, and the overall expression seems less fragile than > the `#if` alternative. I've added a comment to explain this as well. Now the trouble is with the cast operation, because that runs afoul of 5.2.4.2.1p1: The values given below shall be replaced by constant expressions suitable for use in conditional expression inclusion preprocessing directives. ... https://godbolt.org/z/K9cs66sdK I'm almost wondering if the most direct solution is for `__SHRT_MAX__` to be generated with or without the `U` suffix depending on target. We should probably use this as an opportunity to add more exhaustive testing for all the _MIN and _MAX macros to ensure the type properties hold. I was playing around with something like this: https://godbolt.org/z/o7KjY3asW Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144218/new/ https://reviews.llvm.org/D144218 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
