Re: macro FIXNUM_OVERFLOW_P in lisp.h is valid ?

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

> I read the code of function `string-to-number', and traced functions or
> macros recursively.

>   traces of string-to-number:

>     data.c: Fstring_to_number
>       lisp.h: make_fixnum_or_float
>         lisp.h: FIXNUM_OVERFLOW_P

>   citation of FIXNUM_OVERFLOW_P (in Emacs 23.1):

>     /* Value is non-zero if C integer I doesn't fit into a Lisp fixnum.  */

>     #define FIXNUM_OVERFLOW_P(i) \
>       ((EMACS_INT)(i) > MOST_POSITIVE_FIXNUM \
>        || (EMACS_INT) (i) < MOST_NEGATIVE_FIXNUM)

> I think FIXNUM_OVERFLOW_P is problematic.

> The reason is that
> in case `i' is 4294967296.0 (2^32), (EMACS_INT)(i) returns 0
> with a executable program by some compiler
> (for example, Microsoft 32-bit C/C++ Compiler).
> As a result, FIXNUM_OVERFLOW_P(i) returns 0.

>   In Microsoft 32-bit C/C++ Compiler:

>     i					(int)(i)
>     ===============================================
>     2147483648.0	(2^31)		-2147483648
>     2147483647.0	(2^31 - 1)	 2147483647

>     4294967296.0	(2^32)		 0
>     4294967295.0	(2^32 - 1)	-1

>     8589934592.0	(2^33)		 0
>     8589934591.0	(2^33 - 1)	 -1

>   In gcc version 3.4.4 (cygming special):

>     i					(int)(i)
>     ===============================================
>     2147483648.0	(2^31)		-2147483648
>     2147483647.0	(2^31 - 1)	 2147483647

>     4294967296.0	(2^32)		-2147483648
>     4294967295.0	(2^32 - 1)	-2147483648

>     8589934592.0	(2^33)		-2147483648
>     8589934591.0	(2^33 - 1)	-2147483648

> In conclusion, it needs casting to double, I think.

>     #define FIXNUM_OVERFLOW_P(i) \
>       ((double)(i) > (double)MOST_POSITIVE_FIXNUM \
>        || (double)(i) < (double)MOST_NEGATIVE_FIXNUM)

> What do you think about it ?

I think the problem is that FIXNUM_OVERFLOW_P is written to handle an
integer argument, not a float argument.  So rather than patch it to use
floats, I'd rather write a new macro that works for floats (and only
casts the MOST_POSITIVE_FIXNUM side).


        Stefan