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

Expose PyCFunctionFast and PyCFunctionFastWithKeywords in limited API #11

Closed
8 of 10 tasks
encukou opened this issue Feb 1, 2024 · 9 comments
Closed
8 of 10 tasks

Comments

@encukou
Copy link
Collaborator

encukou commented Feb 1, 2024

The names _PyCFunctionFast and _PyCFunctionFastWithKeywords, function types needed with the METH_FASTCALL convention (which is part of limited API), are currently prefixed by underscores. So, the types essentially are part of the stable ABI, but since the names aren't needed to define the functions, they were left out.
Found in PyO3 -- turns out Rust is more strict about what gets called :)

Exposing them is a limited API change, so it might need a WG vote. I hope it's a no-brainer, but I do have a follow-up proposal that might be more controversial.


These should be properly added to the limited API, with leading underscores removed (PR: #114627):

and if so: The old names should be kept as aliases for backwards compatibility, until the fully-public API is changed/removed.

@vstinner
Copy link

vstinner commented Feb 4, 2024

and if so: The old names should be kept as aliases for backwards compatibility, until the fully-public API is changed/removed.

I prefer to force the few users of these two types to the new name. They can add compatibility in their code with 3 lines. It can be added to pythoncapi-compat for example.

@gvanrossum
Copy link

I mistakenly added a thumbs up. I meant a thumb down. This attitude causes pointless breakages.

@vstinner
Copy link

vstinner commented Feb 5, 2024

This attitude causes pointless breakages.

Well, I expressed my point of view. I don't think that we need 100% majority on every topic to take decisions.

I know that there is not a strong consensus towards removing "compatibility layers" in the C API, such as keeping old names as aliases to new names.

I mistakenly added a thumbs up.

Just click on the emoji to remove it. By the way, you can add more than one emoji per message if you want 😊


My use case is to move existing C extensions towards the limited C API step by step, rather than forcing to have to make all changes at once. It would be more interesting to discuss the overall strategy for such goal, if we want this goal, and how to go there. Otherwise, we might lose time into "details".

IMO replacing _PyCFunctionFast with PyCFunctionFast is quite easy. We can easily document the change and explain how to update affected code, and document a solution to have a single code base working on old and new Python versions.

@vstinner
Copy link

vstinner commented Feb 5, 2024

and if so: The old names should be kept as aliases for backwards compatibility, until the fully-public API is changed/removed.

Oh. I forgot something: if the old and new names are basically kept forever, a new Python implementation (or PyPy cpyext) which want to implement the full C API will have to support two names instead of just one. IMO it's not worth it to make the C API larger on this specific issue (_PyCFunctionFast and _PyCFunctionFastWithKeywords).

@gvanrossum
Copy link

The vote is pretty clear. Let's move on.

@iritkatriel
Copy link
Member

Oh. I forgot something: if the old and new names are basically kept forever, a new Python implementation (or PyPy cpyext) which want to implement the full C API will have to support two names instead of just one.

Why? Does the full C API include private functions?

@vstinner
Copy link

vstinner commented Feb 5, 2024

Why? Does the full C API include private functions?

From what I read about PyPy cpyext, the "API" is "what C extensions use". If C extensions use private functions, PyPy must support these private functions. For example, on my system, PyPy 3.9 cpyext implements around 400 private APIs:

$ grep -E '\<(_Py|_PY)' /usr/include/pypy3.9/ -R|wc -l
389

Examples: _Py_HashDouble(), _PyTime_GetSystemClock(), _PyCFunctionFast, etc.

I suppose that at least one C extension that PyPy wants to support use one of these private APIs.

@encukou
Copy link
Collaborator Author

encukou commented Feb 15, 2024

I don't think this is the place to decide about PyPy's needs, or whether we should force users to change working code.
I'll still add that these particular names appeared in public docs well before I added the note that such names are considered private. (If there's a stronger note somewhere in the docs, I'd like to know about it.)

The new names, and the aliases, are now added. Closing.

@encukou encukou closed this as completed Feb 15, 2024
vstinner added a commit to vstinner/cython that referenced this issue Feb 16, 2024
Python 3.13a4 adds a public PyCFunctionFastWithKeywords type.

* python/cpython@9e3729b
* python/cpython#114627
* capi-workgroup/decisions#11

Python 3.13a1 removed the private _PyCFunctionFastWithKeywords type.
vstinner added a commit to vstinner/cython that referenced this issue Feb 16, 2024
Python 3.13a4 adds a public PyCFunctionFastWithKeywords type and
removes the private _PyCFunctionFastWithKeywords type:

* python/cpython@9e3729b
* python/cpython#114627
* capi-workgroup/decisions#11
vstinner added a commit to vstinner/cython that referenced this issue Feb 16, 2024
Python 3.13a4 adds a public PyCFunctionFastWithKeywords and
PyCFunctionFast types and removes the private
_PyCFunctionFastWithKeywords and _PyCFunctionFast types:

* python/cpython@9e3729b
* python/cpython#114627
* capi-workgroup/decisions#11
scoder pushed a commit to cython/cython that referenced this issue Feb 16, 2024
…3a4 (GH-6003)

Python 3.13a4 adds a public PyCFunctionFastWithKeywords and
PyCFunctionFast types and removes the private
_PyCFunctionFastWithKeywords and _PyCFunctionFast types:

* python/cpython@9e3729b
* python/cpython#114627
* capi-workgroup/decisions#11
@vstinner
Copy link

I don't think this is the place to decide about PyPy's needs, or whether we should force users to change working code.

Well, I created #14 which discuss more or less this topic ;-)

scoder pushed a commit to cython/cython that referenced this issue Feb 18, 2024
…3a4 (GH-6003)

Python 3.13a4 adds a public PyCFunctionFastWithKeywords and
PyCFunctionFast types and removes the private
_PyCFunctionFastWithKeywords and _PyCFunctionFast types:

* python/cpython@9e3729b
* python/cpython#114627
* capi-workgroup/decisions#11
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

No branches or pull requests

4 participants