PEP 590: update

[jdemeyer]

Rewrites/clarify some parts of PEP 590 and change some details, adding myself as author.

Main changes:

• Specify that kwnames must be either NULL or a non-empty tuple (this is easier for the callee to detect the case of no keyword arguments).
• Allow subclassing the same way as PEP 580.
• Remove PyCall_MakeTpCall (what's the use case for making this public API?)
• Change METH_VECTORCALL to mean that the signature is of type vectorcall (using the actual vectorcall signature is more natural and faster since you don't need a wrapper at all)
• In PyObject_Vectorcall, allow passing a keyword dict (better to do conversion from dict to tuple in one place, instead of requiring every caller to do that)
• Pass argument vector as PyObject *const *args (it's already like that in the implementation)

Details:

• Change the type of tp_vectorcall_offset to Py_ssize_t (why uintptr_t? every other offset also has type Py_ssize_t)
• New macro PyVectorcall_NARGS(n) for n & ~PY_VECTORCALL_ARGUMENTS_OFFSET (cleaner and more future-proof in case we ever add more special bits)
• New inline function PyVectorcall_Function(obj) to return the vectorcallfunc pointer stored in obj (or NULL).
• Change PyTupleObject in signature to PyObject (it's already like that in the implementation, passing a specific struct in the Python/C API is normally not done)
• Rename PY_VECTORCALL_ARGUMENTS_OFFSET -> PY_VECTORCALL_PREPEND (I didn't like the word "offset" for that, while "prepend" refers to _PyObject_Call_Prepend)
• Rename vectorcall -> vectorcallfunc (analogous to other function pointer typedefs like binaryfunc)
• Rename PyCall_MakeVectorCall -> PyVectorcall_Call

CC @encukou @markshannon

See also the new implementation at python/cpython#13185

[encukou]

What's the reason for PyVectorcall_Check()? The _Check functions are usually used for type checking. Is the type flag not enough?

I'm not convinced about the limitation that kwnames can't be the empty tuple. That sounds like an unnecessary limitation.

IMO Py_TPFLAGS_METHOD_DESCRIPTOR was a better name: it's concerned with the unbound→bound method mechanism. "C functions" don't this.

IMO PyCall_MakeVectorCall was a good name; "CFunction" is a type, but "Vectorcall" is not.

[jdemeyer]

it's concerned with the unbound→bound method mechanism.

But Python 3 doesn't really have "unbound methods". It has functions.

[jdemeyer]

I'm not convinced about the limitation that kwnames can't be the empty tuple. That sounds like an unnecessary limitation.

It's a simplification and optimization. Imagine a C function which doesn't take keywords. With the current PEP 590, it needs to check for that using

if (kwnames != NULL && PyTuple_GET_SIZE(kwnames))

I suggest to simplify that to

if (kwnames != NULL)

[jdemeyer]

What's the reason for PyVectorcall_Check()? The _Check functions are usually used for type checking. Is the type flag not enough?

There is no good reason. I just wanted to have a simpler way to write

PyType_HasFeature(Py_TYPE(obj), Py_TPFLAGS_HAVE_VECTORCALL)

Second, it's also more future-proof if we decide to remove the flag Py_TPFLAGS_HAVE_VECTORCALL.

[jdemeyer]

"CFunction" is a type, but "Vectorcall" is not.

Right, but there are other C API functions referring to a protocol like PySequence_GetItem and PyNumber_Add. I think PyVectorcall_Call could fit in that.

[encukou]

NULL vectrorcall

No longer allow the vectorcall pointer in the instance to be NULL (for simplicity)

Unfortunately, that makes the type optimization impossible -- we want list to have vectorcall, but not all the other types.

METHOD_DESCRIPTOR/FUNCTION_DESCRIPTOR

But Python 3 does have unbound methods:

>>> list.append
<method 'append' of 'list' objects>

"method" describes the mechanism better than "function".

limitation that kwnames can't be the empty tuple

You make a fair point. But let's enforce this, at least in debug mode.

PyVectorcall_Check

Since I'm currently at PyCon sprints, I asked Victor for another point of view here. He agrees that it's not necessary.
Please don't add the extra function. Use PyType_HasFeature instead.

PyVectorcall_Call / PyCall_MakeVectorCall

Let's keep PyCall as the namespace here. Maybe PyCall_VectorCall would work, too.

[jdemeyer]

we want list to have vectorcall, but not all the other types.

Is there a problem with using a generic vectorcall wrapper for all types? This could be set up by PyType_Ready. It's just an idea, feel free to say that it sucks :-)

[markshannon]

Remove the non-empty part

[jdemeyer]

Why? The protocol is easier to use if there is a unique way to specify "no keyword arguments".

[scoder]

I don't have a strong opinion here, but I agree with Jeroen. The caller probably knows already if the tuple is empty or not, so why not help the callee here? As long as PyObject_VectorCall() is used for calling into vectorcallees, it can normalise that case internally, and everyone who does not use it is on their own anyway.

[markshannon]

Given it is a function pointer, the func seems redundant.

[jdemeyer]

It's just clearer to use a name ending in func. CPython has many such names for function pointer types ending in func, for example

typedef PyObject *(*wrapperfunc)(PyObject *self, PyObject *args,
                                 void *wrapped);

[encukou]

Disallowing an empty tuple adds a non-obvious edge case.
What's wrong with non-obvious edge cases? They rely on either everyone implementing the protocol reading the docs and noticing this case, or on checking for the case and enforcing it. In the ideal world, that would be enough. But if we have a choice, and it doesn't hurt performance, let's not add one.

[jdemeyer]

Disallowing an empty tuple adds a non-obvious edge case.

I argue the oppsite: allowing an empty tuple adds a non-obvious edge case.

We should really view this from the side of the callee (the C function), as that's what external projects will implement. From that viewpoint, having two ways to indicate "no keywords" is a non-obvious edge case.

The side of the caller (which should ensure not to send an empty tuple) is CPython and there the issue of people implementing the protocol wrongly doesn't arise. External C code is not expected to manually use tp_vectorcall_offset to make vectorcalls: it is expected to use an API like PyCall_Vectorcall() and that API will ensure to replace an empty tuple with NULL.

I see it as an application of https://en.wikipedia.org/wiki/Robustness_principle (Be conservative in what you send, be liberal in what you accept): PyCall_Vectorcall should accept an empty tuple but it should not send an empty tuple to the vectorcall function.

[markshannon]

Leave this as it is. METH_VECTORCALL is not equivalent to vectorcall.

[markshannon]

I'm not convinced about the limitation that kwnames can't be the empty tuple. That sounds like an unnecessary limitation.

It's a simplification and optimization. Imagine a C function which doesn't take keywords. With the current PEP 590, it needs to check for that using

if (kwnames != NULL && PyTuple_GET_SIZE(kwnames))

I suggest to simplify that to

if (kwnames != NULL)

Although simpler, it is no faster as the additional check is only required during error handling.
Overall, allowing empty tuples is more general and no slower.

[jdemeyer]

Although simpler, it is no faster the additional check is only required during error handling.

Not only during error handling. It's relevant for functions that can take keyword arguments but which are called without keyword arguments.

Overall, allowing empty tuples is more general and no slower.

What would "more general" even mean here? The protocol should be as simple to use as possible and not allowing empty tuples makes it simpler.

What's wrong with disallowing an empty tuple?

[jdemeyer]

I pushed an update with some of the suggested edits.

[encukou]

Workflow/process note: in the future, please don't force-push to PRs. It makes the changes harder to review.
The PR will be squashed into one commit when it's merged.

[jdemeyer]

Mark, Petr: do you agree with all the changes that you did not comment on?

[jdemeyer]

Is there a problem with using a generic vectorcall wrapper for all types? This could be set up by PyType_Ready. It's just an idea, feel free to say that it sucks :-)

I changed my mind here. Let's allow NULL for simplicity.

I would also like to add a macro PyVectorcall_FUNC(obj) to return the vectorcallfunc pointer stored inside obj (which may be NULL if obj doesn't support vectorcall or if the pointer happens to be NULL).

I already made these changes in a new commit.

[jdemeyer]

I replaced PyObject **args by PyObject *const *args. I believe this is what we decided already and it's like that anyway in the implementation.

[markshannon]

Could this PR be split into smaller PRs, please? It makes it a lot easier to merge if distinct changes are in different PRs.

Also, can you separate the clarifications from the modifications, so that the clarifications can be merged without needing to discuss the merits of the various modifications.

[encukou]

Putting the non-controversial changes in one PR first, and getting them merged, should help.
Jeroen, I'd be happy to do it, just let me know so we don't duplicate effort.

[jdemeyer]

PyVectorcall_Call / PyCall_MakeVectorCall

Let's keep PyCall as the namespace here. Maybe PyCall_VectorCall would work, too.

I'd like to come back to this. While working on the implementation, I found the name PyCall_Vectorcall confusing. C API functions are typically named as

{type or protocol}_{operation}

For example:

• PyNumber_Add uses the number protocol to do the operation Add
• PyCFunction_Call acts on PyCFunction instances and does the operation Call (which means taking an args tuple and kwargs dict).

Now the function under discussion executes the operation Call (it takes an args tuple and kwargs dict) and it does that using the vectorcall protocol. So PyVectorcall_Call really seems to be the best name.

[jdemeyer]

At this point, it's not clear to me what are the controversial changes (for example, I addressed some comments by you two yesterday but I don't know if you're happy with those changes).

As far as I can tell, the major points under discussion are:

• allowing an empty tuple
• meaning of METH_VECTORCALL

Then some bikeshedding:

• naming vectorcall or vectorcallfunc
• naming PyCall_MakeVectorCall or PyCall_VectorCall or PyVectorcall_Call
• type of tp_vectorcall_offset (Py_ssize_t or uintptr_t or ...)

Is there something not on this list that you disagree with?

[encukou]

I tried to compile a list of the changes here (and which ones are settled).
But the PR keeps changing, so the list is probably outdated now.
This PR is not easy to review :(

• Renaming the PEP
• Listing Jeroen as the author
• Rewording the Abstract
• Abstract note that this doesn't affect the language
• Rename "Rationale" to "Motivation"
• Moving & rewording Motivation section about types
• Renaming "arguments offset" to "prepend"
• Kwarg name tuple must be non-empty
• PyObject **args ⇒ PyObject *const *args
• PyTupleObject *kwnames → PyObject *kwnames
• vectorcall → vectorcallfunc
• offset: uintptr_t → Py_ssize_t
• Rewording section on Py_TPFLAGS_HAVE_VECTORCALL
• Replacing the Py_TPFLAGS_METHOD_DESCRIPTOR specification
• PyVectorcall_NARGS macro
• Removing restriction on base classes & documenting what happens
• Rewording the "New C API and changes to CPython" list
• METH_VECTORCALL signature change
• METH_VECTORCALL is preferred as it avoids a wrapper function
• Note that METH_VECTORCALL is unspecified
• Adding wrapper_descriptor and method-wrapper to the list of things that do vectorcall
• Convention-specific wrappers for builtin_function_or_method and method_descriptor
• "Using the vectorcall protocol for classes" section
• Argument Clinic should use METH_VECTORCALL, not vectorcall
• "Third-party extension classes using vectorcall" section
• Rewording "Performance implications of these changes"
• Empty "Stable ABI" section
• bpo-29259 in Alternative Suggestions
• Rewording Victor's "fastcall" acknowledgement

[jdemeyer]

Putting the non-controversial changes in one PR first, and getting them merged, should help.
Jeroen, I'd be happy to do it, just let me know so we don't duplicate effort.

I don't plan any further changes to this PR for now. If it's clear to you what's controversial and what not, feel free to make those changes.

[encukou]

If it's clear to you what's controversial and what not, feel free to make those changes.

Alright. It'll take me some time to sort through this.

[jdemeyer]

Thanks for that list. I'll briefly comment on the unchecked boxes:

• Kwarg name tuple must be non-empty

Being discussed already, you know my opinion. Good to revert for now if you don't agree.

• vectorcall → vectorcallfunc

Bikeshedding, I don't care much.

• offset: uintptr_t → Py_ssize_t

Bikeshedding, I don't care much.

• Replacing the Py_TPFLAGS_METHOD_DESCRIPTOR specification

Note that the actual specification is not changed, only the wording. I changed it because I found the original text unclear. Feel free to do whatever you want with this.

• METH_VECTORCALL signature change

Under discussion, would be good to revert for now.

• Adding wrapper_descriptor and method-wrapper to the list of things that do vectorcall

• Convention-specific wrappers for builtin_function_or_method and method_descriptor

Both of these are meant as optional things that could possibly be done as part of the implementation but not strictly part of the PEP. They won't be in the initial implementation and they may or may not be done later. I'd still like to keep them in the PEP if only to have some place where these ideas are written down. You could group these in a section "Possible future CPython changes" or something.

• Argument Clinic should use METH_VECTORCALL, not vectorcall

Related to the discussion about METH_VECTORCALL, can be reverted together with the change to METH_VECTORCALL.

• Rewording "Performance implications of these changes"

I didn't want to claim performance improvements without evidence. Maybe the best would be to remove this section completely?

• bpo-29259 in Alternative Suggestions

What's controversial about this? bpo-29259 is very close to PEP 590, so it would be good to mention it somewhere.

[encukou]

I'd still like to keep them in the PEP if only to have some place where these ideas are written down.

I agree with using the PEP for listing the "optional things we'd like to get into Python 3.8 if we can make it" (as long as they're marked as optional).

[encukou]

All the points are covered in GH-1035 now; I'll keep track of them until they're resolved. Discussions can continue here.

[jdemeyer]

About METH_VECTORCALL

The reason for me to change the meaning of METH_VECTORCALL was motivated by the part about Argument Clinic (AC).

AC works by generating code and a PyMethodDef entry. We all agree that it's a good idea to have AC-generated code use the vectorcall protocol directly without additional wrappers (this is in the PEP). But if you want to do that, you need a way to indicate that in PyMethodDef. So in any case, you need something like my proposal for the METH_VECTORCALL flag.

So we need that functionality and I see no reason why it should be reserved for AC, we might as well make it public (and then we can of course bikeshed about the name...).

[encukou]

I see no reason why [METH_VECTORCALL with vectorcallfunc signature] be reserved for AC

To make the callable argument usable, all callees would need to get self directly out of PyCFunctionObject, which is currently an undocumented implementation detail.
Making PyCFunctionObject part of the API means we can't change it in the future.

[jdemeyer]

which is currently an undocumented implementation detail.

PyCFunction_GetSelf is undocumented but it's actually officially part of the stable ABI (remember the discussion in PEP 580 about deprecating PyCFunction_GetFlags). So it's not an implementation detail.

[encukou]

PyCFunction_GetSelf is part of the stable ABI (for introspection, like [].append.__self__). But the C function doesn't have access to the callable object, so it won't use PyCFunction_GetSelf.
Making this the preferred way to write extension functions would be a major change.

[jdemeyer]

But the C function doesn't have access to the callable object, so it won't use PyCFunction_GetSelf.

Yes, it does have access to the callable object, since it's called with the vectorcallfunc signature. So the METH_VECTORCALL C function would look like

static PyObject *
myfunc(PyObject *callable, PyObject *const *args,
       Py_ssize_t nargs, PyObject *kwnames)
{
    PyObject *self = PyCFunction_GetSelf(callable);
    nargs = PyVectorcall_NARGS(nargs);

    /* From this point on, everything is exactly the same as
     * METH_FASTCALL|METH_KEYWORDS */
}

Making this the preferred way to write extension functions would be a major change.

I'm not saying that this should be the preferred way. I'm just saying: why not allow it? We need the functionality anyway for Argument Clinic, so it's not additional effort to support it.

[jdemeyer]

Actually, scratch that. You need to support two types (builtin_function_or_method and method_descriptor), so it's more complicated than that.

So now I would suggest the minimal approach: leave METH_FASTCALL exactly as it is currently and don't introduce a new METH_VECTORCALL in the PEP (but we do need it privately)

[jdemeyer]

Could this PR be split into smaller PRs, please? It makes it a lot easier to merge if distinct changes are in different PRs.

Done now. Closing this PR as everything is now on separate PRs.