-
Notifications
You must be signed in to change notification settings - Fork 905
Update PyMemberDescrObject struct to match CPython #5647
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
Conversation
| pub struct PyMemberDescrObject { | ||
| pub d_common: PyDescrObject, | ||
| pub d_member: *mut PyGetSetDef, | ||
| pub d_member: *mut PyMemberDef, |
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.
It looks like this changed in python/cpython@042f31d, which was first released as part of Python 3.11 (I think, please double-check that).
In which case, I think the correct fix is to make this conditional on Python version.
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.
Ah great catch!
Confirmed you are correct, it was first released as part of Pyhon 3.11. You can verify by cloning cpython and running:
git tag --contains 042f31da552c19054acd3ef7bb6cfd857bce172b | grep "^v3\." | sort -V
Updated the code.
ngoldbaum
left a comment
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.
needs a release note too
pyo3-ffi/src/cpython/descrobject.rs
Outdated
| @@ -1,4 +1,4 @@ | |||
| use crate::{PyGetSetDef, PyMethodDef, PyObject, PyTypeObject}; | |||
| use crate::{PyGetSetDef, PyMemberDef, PyMethodDef, PyObject, PyTypeObject}; | |||
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 has to be conditional too to appease clippy
pyo3-ffi/src/cpython/descrobject.rs
Outdated
| #[cfg(not(Py_3_11))] | ||
| pub d_member: *mut PyGetSetDef, | ||
| #[cfg(Py_3_11)] | ||
| pub d_member: *mut crate::PyMemberDef, |
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.
Sorry to nitpick - it’s a little more idiomatic to add a new use statement that’s inside a #[cfg(Py_3_11)] conditional compilation block at the top of this file rather than using crate here.
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.
No worries. Updated
In pyo3-ffi the
PyMemberDescrObjecthad thed_memberattribute defined as a pointer toPyGetSetDefwhen it should be a pointer toPyMemberDef.The
PyMemberDescrObjectas defined in CPython:See: https://github.com/python/cpython/blob/ebf955df7a89ed0c7968f79faec1de49f61ed7cb/Include/cpython/descrobject.h#L44