-
Notifications
You must be signed in to change notification settings - Fork 784
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
ffi cleanup: abstract.h #1436
ffi cleanup: abstract.h #1436
Conversation
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.
Good catch 👍🏼 , thank you!
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 is a great catch, thanks!
I'm going to rebase this, force-push and merge shortly. Thanks again. |
Some functions available in earlier versions were moved to this directory, but are still valid.
Hmm, after rebasing we still have issues: the "macro" forms of |
Ouch. I've double-checked and PyIter_Check in 3.7 tries to use the contents of the opaque PyTypeObject, even under the limited API. I wonder why nobody noticed? I think the best thing to do here is to remove the functions again :( will do this by the weekend. |
While these are defined as macros in the Python C API, they rely on access to the PyTypeObject structure, which is not part of the limited API for those versions.
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.
Thanks for finishing this off.
// Defined as this macro in Python 3.6, 3.7 limited API, but relies on | ||
// non-limited PyTypeObject. Don't expose this since it cannot be used. | ||
#[cfg(not(any(Py_LIMITED_API, PyPy)))] | ||
#[inline] | ||
pub unsafe fn PyIter_Check(o: *mut PyObject) -> c_int { |
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 PR makes PyIter_Check available to Python earlier than 3.8 under the limited API.
This was due to some interaction between the
cfg(not(Py_LIMITED_API)
onffi::cpython
, and the cfg on PyIter_Check within theffi::cpython
module.However, it's always been in the limited API: as a macro before 3.8, and as a function since 3.8. Under the non-limited API it's always a macro.
Consequently we can implement PyTryFrom for PyIterator for all versions. I've (speculatively) commented out the cfg preventing that.
Similar changes for PyIndex_Check.