[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: bignum branch

From: Eli Zaretskii
Subject: Re: bignum branch
Date: Fri, 03 Aug 2018 09:23:14 +0300

> From: Andy Moreton <address@hidden>
> Date: Fri, 03 Aug 2018 01:43:17 +0100
> After a lot more testing, I have a somewhat scruffy patch that works in
> the following emacs builds on unpatched master, and on patched bignum branch:
>  - cygwin 64bit (LP64 model)
>  - mingw64 msys2 32bit
>  - mingw64 msys2 64bit (LLP64 model)

I believe your changes also cover the 32-bit MinGW build with wide ints.

> The patch contains changes for:
>  - fix CCL to ensure it uses 28biut codewords properly in bignum build
>  - ensure make_number creates fixnums in LLP64 builds
>  - fix bugnumcompare for LLP64 builds
>  - fix arith_driver to handle markers correctly
>  - fix arith_driver operations for LLP64 builds (more testing needed)
>  - fix float_arith_driver to allow bignums
>  - fix ash_lsh_impl to produce correct results in bignum build
>  - fix NUMBERP to remove redundant BIGNUMP test (ensured by INTEGERP)

Can you tell what is the following hunk about?

> @@ -3414,7 +3473,7 @@ Markers are converted to integers.  */)
>    else
>      {
>        eassume (FIXNUMP (number));
> -      if (XINT (number) > MOST_POSITIVE_FIXNUM)
> +      if (XINT (number) > MOST_NEGATIVE_FIXNUM)
>       XSETINT (number, XINT (number) - 1);
>        else
>       {

Also, this:

> +(defun ccl-fixnum (code)
> +  "Convert a CCL code word to a fixnum value."
> +  (- (logxor (logand code #x0fffffff) #x08000000) #x08000000))

should have a comment explaining why this function is needed.

Btw, isn't it confusing that INTEGERP allows fixnums and bignums, but
XINT, XFASTINT, and XSETINT work only with fixnums?  I envision quite
a few of potential bugs due to this semantic discrepancy, like the one
you just fixed:

>  - fix NUMBERP to remove redundant BIGNUMP test (ensured by INTEGERP)

Should we have a different name for what is now INTEGERP?  Like
WHOLENUMP, for example?

Thanks again for working on this.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]