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

Define _PyCFunctionFastWithKeywords() on CPython 3.7+ #1384

Merged
merged 2 commits into from Jan 15, 2021
Merged

Define _PyCFunctionFastWithKeywords() on CPython 3.7+ #1384

merged 2 commits into from Jan 15, 2021

Conversation

ijl
Copy link
Contributor

@ijl ijl commented Jan 13, 2021

@ijl ijl changed the title Link _PyCFunctionFastWithKeywords() on CPython 3.7+ Define _PyCFunctionFastWithKeywords() on CPython 3.7+ Jan 13, 2021
@ijl
Copy link
Contributor Author

ijl commented Jan 13, 2021

Regarding the 3.7 cfg, this was introduced in cpython commit 6969eaf4682beb01bc95eeb14f5ce6c01312e297 and first tagged in v3.7.0a1.

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

I've got a tiny nit on the changelog and this is good to merge. Looking at the docs you linked I noticed there's a couple of other mismatches we have:

  • we incorrectly use args: *mut *mut PyObject in _PyCFunctionFast when it should also be *const *mut PyObject like you have used here.
  • we're also missing PyCMethod for Python 3.9 and up.

This PR is good to merge with the suggested changelog entry. However, if you've got a moment and willing to fix the above points at the same time I'd be very grateful! 👍

CHANGELOG.md Outdated Show resolved Hide resolved
@nw0
Copy link
Contributor

nw0 commented Jan 15, 2021

we incorrectly use args: *mut *mut PyObject in _PyCFunctionFast when it should also be *const *mut PyObject like you have used here.

This is actually correct for 3.6:

typedef PyObject *(*_PyCFunctionFast) (PyObject *self, PyObject **args,
                                       Py_ssize_t nargs, PyObject *kwnames);

src/ffi/methodobject.rs Outdated Show resolved Hide resolved
@davidhewitt
Copy link
Member

👍 thanks for the review @nw0 - in which case I'll merge this now and we can leave further corrections for #1289

Co-authored-by: Nicholas Sim <nsim+github@posteo.net>
@ijl
Copy link
Contributor Author

ijl commented Jan 15, 2021

I see you committed the rewording, thanks.

@davidhewitt davidhewitt merged commit 3de51d5 into PyO3:master Jan 15, 2021
@ijl ijl deleted the pycfunctionfastwithkeyboards branch January 15, 2021 14:55
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 this pull request may close these issues.

None yet

3 participants