classification
Title: Rename _PyObject_FastCallKeywords
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: duplicate
Dependencies: Superseder:
Assigned To: Nosy List: Mark.Shannon, jdemeyer, petr.viktorin
Priority: normal Keywords: patch

Created on 2019-05-20 08:40 by jdemeyer, last changed 2019-05-21 15:13 by jdemeyer. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 13438 closed jdemeyer, 2019-05-20 08:47
Messages (14)
msg342892 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-05-20 08:40
As preparation for PEP 590, rename

_PyObject_FastCallKeywords -> _PyObject_Vectorcall
_PyObject_FastCallDict -> _PyObject_VectorcallDict
msg342893 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2019-05-20 08:50
Sorry for the bikeshedding here, but I'll read those function names a
lot. Is it possible to rename Vectorcall, which looks good on its own
but not inside _PyObject_VectorcallDict()?

_PyObject_FastCallDict is much easier to read.


Has the PEP been accepted? It still says "Draft".
msg342894 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-05-20 08:52
> Is it possible to rename Vectorcall, which looks good on its own but not inside _PyObject_VectorcallDict()?

I don't understand what you mean here?

> _PyObject_FastCallDict is much easier to read.

You think that "_PyObject_FastCallDict" is easier to read than "_PyObject_VectorcallDict"? I don't think that there is much difference.

> Has the PEP been accepted? It still says "Draft".

It's very close to being accepted, just some details to work out.
msg342895 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-20 08:52
> Has the PEP been accepted? It still says "Draft".

I would prefer to wait until the PEP is accepted before starting to push changes ;-)

--

Cython uses the FASTCALL calling convention. Please check with Stefan Behnel to see fix it's ok to "remove" _PyObject_FastCallKeywords and _PyObject_FastCallDict. It would be bad to suddently break all extensions compiled with Cython.

Maybe we could add new functions and wait for beta2 or later to remove the old ones.
msg342896 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2019-05-20 08:55
> You think that "_PyObject_FastCallDict" is easier to read than "_PyObject_VectorcallDict"? I don't think that there is much difference.

Marvellous. How much do you work with the C-API?
msg342897 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-05-20 08:56
Now to justify the naming: the plan is to keep the name "fast call" for the PyMethodDef flag METH_FASTCALL but to use the name "vectorcall" in the more general API.

I think that's a good idea regardless of PEP 590, as it makes it clear that _PyObject_Vectorcall is useful for more than just METH_FASTCALL functions.
msg342898 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-05-20 09:01
> I would prefer to wait until the PEP is accepted before starting to push changes ;-)

I don't think it's wrong to propose and discuss changes already now. With the first beta of Python 3.8 getting close, it's better to be prepared already for the moment that PEP 590 is accepted.

> Cython uses the FASTCALL calling convention. Please check with Stefan Behnel to see fix it's ok to "remove" _PyObject_FastCallKeywords and _PyObject_FastCallDict. It would be bad to suddently break all extensions compiled with Cython.

I checked, Cython does not use those *generic* _PyObject_FastCall functions. It does use more specialized functions like _PyFunction_FastCallDict and _PyMethodDef_RawFastCallKeywords.
msg342900 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2019-05-20 09:07
It is one thing to name something __vectorcall, and quite another to mix camel and normal case.

When I'm scanning the code very quickly, I initially parse _PyObject_VectorcallDict as PyObject_VectorallDict or_PyObject_Vector_callDict.


From the perspective of reading, it is one of the most obnoxious
names I've seen in the Python code base.
msg342901 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-05-20 09:18
> From the perspective of reading, it is one of the most obnoxious names I've seen in the Python code base.

PyObject_Vectorcall() is the name in PEP 590, so if you want to change it, better write to python-dev about it.
msg342902 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2019-05-20 09:21
Only people with very much time on their hands have the luxury to discuss changes like this on python-dev.
msg342903 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2019-05-20 09:23
Also, thank you for lecturing a core dev again.
msg342987 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-05-21 08:45
The consensus on PEP 590 now seems to be to keep the name `_PyObject_FastCallDict` and rename only `_PyObject_FastCallKeywords` -> `_PyObject_Vectorcall`.

For the record: I don't agree with this decision but I'll implement it.
msg343041 - (view) Author: Jeroen Demeyer (jdemeyer) * Date: 2019-05-21 12:52
What's your opinion on renaming also

_PyCFunction_FastCallKeywords -> _PyCFunction_Vectorcall
_PyFunction_FastCallKeywords -> _PyFunction_Vectorcall

If you agree, I will add it to the PR.
msg343063 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2019-05-21 14:57
If and when a function uses the vectorcall protocol (including the argument offset flag), it makes sense to rename it to vectorcall. Doing it before is misleading, IMO.

Splitting fixes to unrelated issues (like bpo-36907) from PEP 590 is great, but this one needs to be merged together with PEP 590.
History
Date User Action Args
2019-05-21 15:13:39jdemeyersetstatus: open -> closed
resolution: duplicate
stage: patch review -> resolved
2019-05-21 14:57:12petr.viktorinsetmessages: + msg343063
2019-05-21 12:52:19jdemeyersetmessages: + msg343041
2019-05-21 12:51:43vstinnersetnosy: - vstinner
2019-05-21 12:50:18jdemeyersettitle: Rename _PyObject_FastCall functions -> Rename _PyObject_FastCallKeywords
2019-05-21 08:45:02jdemeyersetmessages: + msg342987
2019-05-20 09:23:58skrahsetnosy: - skrah
2019-05-20 09:23:41skrahsetnosy: + skrah
messages: + msg342903
2019-05-20 09:22:58skrahsetnosy: - skrah
2019-05-20 09:21:53skrahsetmessages: + msg342902
2019-05-20 09:18:31jdemeyersetmessages: + msg342901
2019-05-20 09:07:52skrahsetmessages: + msg342900
2019-05-20 09:01:20jdemeyersetmessages: + msg342898
2019-05-20 08:56:23jdemeyersetmessages: + msg342897
2019-05-20 08:55:03skrahsetmessages: + msg342896
2019-05-20 08:52:58vstinnersetnosy: + vstinner
messages: + msg342895
2019-05-20 08:52:53jdemeyersetmessages: + msg342894
2019-05-20 08:50:15skrahsetnosy: + skrah
messages: + msg342893
2019-05-20 08:47:48jdemeyersetkeywords: + patch
stage: patch review
pull_requests: + pull_request13347
2019-05-20 08:40:40jdemeyercreate