Do no-type-slots dictoffset check as python lookup #5698
Conversation
Rather than just declaring it impossible. Fixes cython#5696
Cython/Utility/ExtensionTypes.c
Outdated
| if (b_dictoffset == -1 && PyErr_Occurred()) goto dictoffset_return; | ||
| #endif | ||
| if (b_dictoffset) { | ||
| __Pyx_TypeName b_name = __Pyx_PyType_GetName(b); |
There was a problem hiding this comment.
The declaration of b_name is between the goto and the label, which will fail in C++
There was a problem hiding this comment.
Adding another set of braces to enclose the scope of b_name fixes the c++ compilation error, but then running the test on PyPy I get
CFLAGS="-DCYTHON_USE_TYPE_SPECS=1" ../pypy/pypy3.9-c runtests.py cdef_multiple_inheritance_nodict --no-c --no-cleanup
...
File "tests/run/cdef_multiple_inheritance_nodict.pyx", line 16, in init cdef_multiple_inheritance_nodict (cdef_multiple_inheritance_nodict.cpp:7472)
cdef class Both(CBase, PyBase):
AttributeError: type object 'PyBase' has no attribute '__dictoffset__'
which seems to be an incompatibility in PyPy: classes never have __dictoffset__. It turns out there is an open issue about the lack of proper handling of _PyObject_GetDictPtr and tp_dictoffset for pybind11, that was never resolved. So while this PR fixes a corner case for CPython, it doesn't fix the PyPy test failure.
Sorry for the bother. I am not sure how to fix this.
There was a problem hiding this comment.
well, I guess I could get USE_CYTHON_TYPE_SLOTS to work on PyPy.
There was a problem hiding this comment.
I made the test
#if CYTHON_USE_TYPE_SLOTS || CYTHON_COMPILING_IN_PYPY
Not completely sure if this'll work or not though, but it seems easier than trying to get the whole of CYTHON_USE_TYPE_SLOTS to work with PyPy.
Sorry for the bother. I am not sure how to fix this.
It's still worth fixing the limited api side of it anyway, even if it doesn't fix PyPy.
Co-authored-by: Matti Picus <matti.picus@gmail.com>
Co-authored-by: scoder <stefan_ml@behnel.de>
|
Seems to have broken something in PyPy. I'm away from my PC for the rest of the week so I'll investigate properly when I get back. |
No, it's fine, that was a hickup in master on my part. |
…ythonGH-5698) ... rather than just declaring it impossible. Fixes cython#5696
Rather than just declaring it impossible.
Fixes #5696
(Note that this isn't sufficient to declare multiply-inherited cdef classes in the limited api - the calling code generates a few
PyTuple_GET_ITEMs. However, I have tested the dictoffset code itself)