GeezerVison

@markshannon might be interested in reviewing this as the author of PEP 590.

@erlend-aasland Done.

What is the reason for not adding PyVectorcall_Function to the Limited API?

What is the reason for not adding PyVectorcall_Function to the Limited API?

It seemed like sort of an implementation detail. I am happy to add it (& tests) if there is consensus to do so.

It seemed like sort of an implementation detail. I am happy to add it (& tests) if there is consensus to do so.

I've never had use for it myself, so I have no opinion about this; I was just curious. Let's hear what the others say when they chime in.

Two questions.

First question

Why include PyObject_VectorcallDict? It seem to be an internal function for efficiently supporting the old calling convention for callables that support vectorcall.

Second question

The vectorcall API has two parts.

1. Calling through PyObject_Vectorcall
2. Declaring that an object can be called as "vector callable" by setting the Py_TPFLAGS_HAVE_VECTORCALL flag and type.tp_vectorcall_offset.

Any plans to add the ability to declare classes as having "vector callable" instances?

Thanks for the review @markshannon.

Why include PyObject_VectorcallDict? It seem to be an internal function for efficiently supporting the old calling convention for callables that support vectorcall.

I am happy to hear it. I have never found use for this function myself. I included in the PR for reasons of symmetry, and because it was prominently advertised in the docs next to PyObject_Vectorcall and PyObject_VectorcallMethod. I will remove it in the next update of the PR.

Any plans to add the ability to declare classes as having "vector callable" instances?

I think that this was already done in PR #93274. Please let me know if I misunderstood.

Finally, do you have any thoughts on PyVectorcall_Function brought up by @erlend-aasland ?

One other question.

Why add PyObject_VectorcallMethod, rather than exposing the lower level method-getting machinery?
PyObject_VectorcallMethod forces the attribute lookup to happen after the arguments are evaluated which is different from Python.
A PyObject *LoadMethod(PyObject *obj, PyObject *name, PyObject **self) function would allow C code to use Python evaluation order. PyObject_VectorcallMethod could be added as a utility function if desirable.

This is a sketch and is probably missing some error handling and/or incref/decrefs.

PyObject *PyObject_VectorcallMethod(
    PyObject *name, PyObject *const *args,
    size_t nargsf, PyObject *kwnames)
{
      /* Error if nargs < 1 */
      PyObject *obj = args[0];
      PyObject *callable = LoadMethod(obj, name, &args[0]);
      /* TO DO --- Handle error if callable == NULL */
      if (args[0] == NULL) {
            /* No self */
           return PyObject_Vectorcall(callable, args+1, (nargsf-1) | PY_VECTORCALL_ARGUMENTS_OFFSET, kwnames);
      }
      return PyObject_Vectorcall(callable, args, nargsf, kwnames);
}

Finally, do you have any thoughts on PyVectorcall_Function brought up by @erlend-aasland ?

I'd rather not add it.
Unless it is specified in such a way as to be useless, its existence would unnecessarily constrain the implementation.

Why add PyObject_VectorcallMethod, rather than exposing the lower level method-getting machinery?
PyObject_VectorcallMethod forces the attribute lookup to happen after the arguments are evaluated which is different from Python.

It is convenient to use a 1-function-does-it-all interface like PyObject_VectorcallMethod for performing method calls from binding projects. This is particularly true for the Py_LIMITED_API that is already heavy on CPython API calls due to the opaque nature of the interfaces. The difference in attribute lookup ordering is an interesting point. I was not aware of this, but it strikes me as a quite minor/subtle issue. (Maybe others will disagree though!)

Altogether, it sounds to me more like a general question about why PyObject_VectorcallMethod was designed the way it was? My goal with this PR was to expose something that is already in general use.

Is this okay @markshannon? (with a goto to avoid lots of if blocks with different numbers of Py_DECREFs)

I have made the requested changes; please review again

Thanks for making the requested changes!

@erlend-aasland, @markshannon: please review the changes made to this pull request.

The test_asyncio test fails on Windows (Azure Pipelines). Potentially a fluke/unrelated issue? I don't see how it could be related.

The test_asyncio test fails on Windows (Azure Pipelines). Potentially a fluke/unrelated issue? I don't see how it could be related.

It's a known issue that will be fixed by #98704

Thank you for the PR!

Awesome, many thanks for your help in getting it merged!

I realized that I forgot to add an entry to Doc/whatsnew/3.12.rst. Should I open a separate PR, or will somebody from the maintainers team do this?

Open a new PR please.