bpo-37483: add _PyObject_CallOneArg() function

[jdemeyer]

https://bugs.python.org/issue37483

[aeros]

@jdemeyer On lines 145 and 146 of abstract.h:
return _PyObject_Vectorcall(func, args, 1 | PY_VECTORCALL_ARGUMENTS_OFFSET, NULL);

What is the purpose of using the constant PY_VECTORCALL_ARGUMENTS_OFFSET in a function that is made specifically for single arguments? Unless I am misunderstanding something, return _PyObject_Vectorcall(func, args, 1, NULL); would effectively have the same functionality. Is there any circumstance where this function is used and 1 | PY_VECTORCALL_ARGUMENTS_OFFSET does not result in 1?

[jdemeyer]

What is the purpose of using the constant PY_VECTORCALL_ARGUMENTS_OFFSET in a function that is made specifically for single arguments?

It's meant for optimizing calls to bound methods. If you're calling a bound method with 1 argument, you're effectively calling a function with 2 arguments when you add self. So args[-1] is meant to store self. See https://www.python.org/dev/peps/pep-0590/#py-vectorcall-arguments-offset

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[aeros]

@jdemeyer Wouldn't 1 | PY_VECTORCALL_ARGUMENTS_OFFSET always resolve to the same value in the case of this function though? The primary use for it seems to be calculating the offset when the number of args is unknown:
nargs | PY_VECTORCALL_ARGUMENTS_OFFSET

or for the example provided in PEP 590:
nargs & PY_VECTORCALL_ARGUMENTS_OFFSET

What is the purpose of using PY_VECTORCALL_ARGUMENTS_OFFSET when the the number of arguments for the function is static? As far as I can tell, the result of 1 | PY_VECTORCALL_ARGUMENTS_OFFSET will always be 1 any time _PyObject_CallOneArg() is utilized. Is this incorrect?

Also, apologies for the questions if these answers are fairly obvious. I have some understanding of C fundamentals, but I'm significantly more experienced with Python. I'm just trying to get a solid understanding of both so that I can be more useful as a contributor.

[jdemeyer]

I have made the requested changes; please review again

[bedevere-bot]

Thanks for making the requested changes!

@skrah, @methane: please review the changes made to this pull request.

[jdemeyer]

Wouldn't 1 | PY_VECTORCALL_ARGUMENTS_OFFSET always resolve to the same value in the case of this function though?

PY_VECTORCALL_ARGUMENTS_OFFSET is a constant flag, so of course 1 | PY_VECTORCALL_ARGUMENTS_OFFSET is a constant value. But that's not the point.

The PY_VECTORCALL_ARGUMENTS_OFFSET flag is meant for the called function. It indicates that the value args[-1] may temporarily be changed as scratch space. That's the reason for the array _args of length two in this PR: _args[0] is the same as args[-1] which can be used as scratch space by the called function.

[aeros]

@jdemeyer Oh that makes sense, I see. I hadn't realized the purpose of it within this function was solely to be an indicating flag, in general I'm far more used to working on much smaller scale projects and codebases where almost everything has a specific functional purpose. Seeing the two constant values being used for the nargs argument of PyObject_Vectorcall had seemed redundant to me initially, but now I understand the reasoning.

Thank you for taking the time to fully explain it, I certainly appreciate it.

[skrah]

Thanks for the changes, the "approval" was a GitHub thing, I'm actually -0 on such large replacement PRs.

[jdemeyer]

It's not doing replacement for the sake of replacement. As I explained on bpo-37483, it gives a nice performance gain (in terms of runtime and stack usage).

[bedevere-bot]

@methane: Please replace # with GH- in the commit message next time. Thanks!

[skrah]

I mean large replacement. Note that I did not protest against the changes in memoryobject.c.