-
Notifications
You must be signed in to change notification settings - Fork 210
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
redo {spmatrix,matrix}iter_tp to use PyType_Ready #165
Conversation
Would you accept a PR to add PyPy to your CI matrix? |
@@ -2006,7 +2008,7 @@ PyMODINIT_FUNC initbase(void) | |||
if (PyType_Ready(&matrix_tp) < 0) | |||
INITERROR; | |||
|
|||
if (PyType_Ready(&matrix_tp) < 0) | |||
if (PyType_Ready(&matrixiter_tp) < 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible typo? Maybe the intent was to do this all along?
@@ -2150,9 +2150,6 @@ matrix_iter(matrix *obj) | |||
it = PyObject_GC_New(matrixiter, &matrixiter_tp); | |||
if (!it) return NULL; | |||
|
|||
matrixiter_tp.tp_iter = PyObject_SelfIter; | |||
matrixiter_tp.tp_getattro = PyObject_GenericGetAttr; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works on CPython and PyPy, but is not really supported in general. All assignments are meant to be done before calling PyType_Ready
Thanks for investigating & fixing this! @martinandersen |
Yes, I'll prepare a patch release tomorrow |
Awesome, thanks! |
xref conda-forge/cvxopt-feedstock#45
It turns out
spmatrixiter_tp
andmatrixiter_tp
never calledPyType_Ready
. They also modified pointers to functions on new instances, which is not officially supported by the C-API. This caused segfaults when using cvxopt on PyPy.