Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incompatible with python 3.11 #114

Closed
mkoeppe opened this issue May 18, 2022 · 2 comments · Fixed by #120
Closed

Incompatible with python 3.11 #114

mkoeppe opened this issue May 18, 2022 · 2 comments · Fixed by #120

Comments

@mkoeppe
Copy link

mkoeppe commented May 18, 2022

  gcc -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -g -O2 -g -O2 -I./cypari2 -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-py311/local/var/lib/sage/venv-python3.11.0b1/lib/python3.11/site-packages/cysignals -I/usr/local/include -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-py311/local/var/lib/sage/venv-python3.11.0b1/include/python3.11 -c cypari2/convert.c -o build/temp.macosx-12.4-x86_64-3.11/cypari2/convert.o
  cypari2/convert.c:3347:21: error: cannot take the address of an rvalue of type 'Py_ssize_t' (aka 'long')
    __pyx_v_sizeptr = &Py_SIZE(((PyObject *)__pyx_v_x));
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 error generated.

https://trac.sagemath.org/ticket/33842

@kliem
Copy link
Collaborator

kliem commented May 19, 2022

The offending code is the following:

x = _PyLong_New(sizedigits)
cdef digit* D = (<PyLongObject*>x).ob_digit

cdef digit d
cdef ulong w
cdef size_t i, j, bit 
for i in range(sizedigits):
    # The least significant bit of digit number i of the output
    # integer is bit number "bit" of word "j".
    bit = i * PyLong_SHIFT
    j = bit // BITS_IN_LONG
    bit = bit % BITS_IN_LONG

    w = int_W(g, j)[0]
    d = w >> bit 

    # Do we need bits from the next word too?
    if BITS_IN_LONG - bit < PyLong_SHIFT and j+1 < sizewords:
        w = int_W(g, j+1)[0]
        d += w << (BITS_IN_LONG - bit)

    d = d & PyLong_MASK
    D[i] = d 

    # Keep track of last non-zero digit
    if d:
        sizedigits_final = i+1 

# Set correct size (use a pointer to hack around Cython's
# non-support for lvalues).
cdef Py_ssize_t* sizeptr = Py_SIZE_PTR(x)
if signe(g) > 0:
    sizeptr[0] = sizedigits_final
else:
    sizeptr[0] = -sizedigits_final

It assumes that x is of type Py_ssize_t, but this is no longer the case. It is now of type py_long, which is

ctypedef class __builtin__.py_long [object PyLongObject]:
    cdef digit* ob_digit

(according to https://github.com/cython/cython/blob/0.29.x/Cython/Includes/cpython/longintrepr.pxd).

@kliem
Copy link
Collaborator

kliem commented May 20, 2022

Given how bad this looks, the solution is apparently surprisingly easy:

Setting the size of a python long was done with Py_SIZE (https://docs.python.org/3.8/c-api/structures.html#c.Py_SIZE). Python 3.9 introduces Py_SET_SIZE and python 3.10 disables setting the size with Py_SIZE (https://docs.python.org/3.10/c-api/structures.html#c.Py_SIZE). So what we do is already illegal with python 3.10, but maybe with inlining or so it still works.

Python 3.11 now makes setting the size using Py_SIZE an error: https://docs.python.org/3.11/c-api/structures.html#c.Py_SIZE.

(The actual error message here is a bit cryptic, but basically Py_SIZE is no longer a reference, but a new value. Saving the address of an rvalue doesn't work.)

Tests are currently running here: https://github.com/kliem/cypari2/runs/6523399435?check_suite_focus=true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants