Including <limits.h> in gmp-impl.h
marc.glisse at inria.fr
Thu Jan 2 19:10:32 UTC 2014
On Thu, 2 Jan 2014, Niels Möller wrote:
> Marc Glisse <marc.glisse at inria.fr> writes:
>> I'll also push a patch handling the fact that the standard SHRT_MAX
>> and others don't have type short but int.
> Can you explain what the problem is? I noticed this change,
> --- a/gmp-h.in Thu Jan 02 12:28:21 2014 +0100
> +++ b/gmp-h.in Thu Jan 02 13:25:31 2014 +0100
> @@ -437,10 +437,10 @@
> #define __GMP_MAX(h,i) ((h) > (i) ? (h) : (i))
> /* __GMP_USHRT_MAX is not "~ (unsigned short) 0" because short is promoted
> - to int by "~". */
> + to int by "~". It still needs to have the promoted type. */
> #define __GMP_UINT_MAX (~ (unsigned) 0)
> #define __GMP_ULONG_MAX (~ (unsigned long) 0)
> -#define __GMP_USHRT_MAX ((unsigned short) ~0)
> +#define __GMP_USHRT_MAX (0 + (unsigned short) ~0)
> 1. Why should it be of type int? Which problems are caused by having it
> as unsigned short?
> 2. If int is desired, I think it's a bit clearer with an explicit cast,
> #define __GMP_USHRT_MAX ((int) (unsigned short) ~0)
> But, unconditionally defining it as int also seems unportable. A C
> implementation may define both short and int to be 32 bits, right? Then
> the maximum unsigned short doesn't fit in a signed int. While it would
> always fit in an *unsigned* int.
> Is that what the "0 + " promotion thing is intended to do; you know the
> spec a better than me, but you'd get unsigned int if short and int are
> of the same size, and signed int if it's of larger size than short?
> I miss a comment spelling out which type really is intended, and why.
You have pretty much said everything already ;-)
C99 (it's the only version I have here) says about SHRT_MAX and others:
"the following shall be replaced by expressions that have the same type as
would an expression that is an object of the corresponding type converted
according to the integer promotions."
integer promotion for a short means int, and for an unsigned short it
means either int or unsigned int depending on their relative size.
"0 + " is indeed a random way to force type promotion (note the sentence I
added in the comment above __GMP_USHRT_MAX).
I made the change so that the type of USHRT_MAX is the same whether it
comes from limits.h or from gmp.
The type is mostly irrelevant, except that we have a few printf calls in
the testsuite where we were using "%hX".
For gmp-5.3 (not this one, the one after that), I think we should remove
__GMP_USHRT_MAX altogether and forcibly include limits.h in gmp.h (it
isn't comparable to stdio.h). We would also remove all the related code in
gmp-impl.h, and half of my patch would become irrelevant. (I didn't want
to do it so close to the release, but it would only take a few minutes if
you want me to do it for 5.2)
So, do you agree with the change? (care to add a suitable comment?)
Or would you prefer a different change?
More information about the gmp-devel