search

capi-workgroup / decisions

Discussion and voting on specific issues
5 stars 1 forks source link

Expose PyCFunctionFast and PyCFunctionFastWithKeywords in limited API #11

Closed encukou closed 7 months ago

encukou commented 8 months ago

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 commented 7 months ago

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 commented 7 months ago

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

vstinner commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

iritkatriel commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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.

vstinner commented 7 months ago

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 https://github.com/capi-workgroup/decisions/issues/14 which discuss more or less this topic ;-)