bug#32463: 27.0.50; (logior -1) => 4611686018427387903

[Pip Cet]

On Fri, Aug 17, 2018 at 9:34 AM Andy Moreton <andrewjmoreton@gmail.com> wrote:
>
> On Fri 17 Aug 2018, Pip Cet wrote:
>
> > Can you try the attached patch? It fixes a few things that I reported
> > to Tom yesterday:
> >
> > ---
> >
> >
> > None of these are necessarily bugs, but:
> >
> > * I find the behavior of `lsh', `logand', `logior', and `logxor', when
> > given negative arguments, surprising. I think it would make most sense
> > to treat negative numbers as the infinite bitstream consisting of all
> > ones to the left of the specified value:
> >
> > (logand -1 -1) would be interpreted as ...1111111 & ...1111111 =
> > ...1111111, so it would be -1 (rather than 2 * most-positive-fixnum +
> > 1).
> > (lsh (- (lsh -1 64) 1) -1) would be ...1110111...1111 shifted to the
> > right by one digit, an odd number, rather than the even number
> > currently produced. (I believe lsh and ash should behave identically.)
>
> > @@ -3383,8 +3383,6 @@ ash_lsh_impl (Lisp_Object value, Lisp_Object count, 
> > bool lsh)
> >        mpz_init (result);
> >        if (XFIXNUM (count) >= 0)
> >       mpz_mul_2exp (result, XBIGNUM (value)->value, XFIXNUM (count));
> > -      else if (lsh)
> > -     mpz_tdiv_q_2exp (result, XBIGNUM (value)->value, - XFIXNUM (count));
> >        else
> >       mpz_fdiv_q_2exp (result, XBIGNUM (value)->value, - XFIXNUM (count));
> >        val = make_number (result);
> > @@ -3401,14 +3399,7 @@ ash_lsh_impl (Lisp_Object value, Lisp_Object count, 
> > bool lsh)
>
> Please add more test cases to test/src/data-tests.el to ensure that the
> logical operations have the expected behaviour for positive and negative
> arguments, and for both bignum and fixnum arguments.

Paul committed a patch in the meantime (independently, I think?) which
does add tests. I'll try to write some more.

> The tdiv/fdiv were
> added to give expected results. Pay particular attention to values around
> most-positive-fixnum and most-negative-fixnum.

I don't think they do give the expected results. We should discuss
that in more detail, but first, can we agree that lsh and ash behave
the same for bignums? If so, clearly one branch of the code you quoted
is incorrect, and I think it's the tdiv one.

I ran:

(require 'cl)

(let ((i 0))
  (while (< i 128)
    (message "%d %x" i (lsh (- (lsh -1 i) 1) -1))
    (incf i)))

and got this output:

[...]
57 -100000000000001
58 -200000000000001
59 -400000000000001
60 -800000000000001
61 -1000000000000000
62 -2000000000000000
63 -4000000000000000
64 -8000000000000000
[...]

Something is wrong there. The expression certainly shouldn't switch
from being odd to being even just because we're leaving the fixnum
range. If I do the calculation by hand on a piece of paper, I get the
results that correspond to the fixnum case, not the bignum case.