-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
Generate vectorcall code to parse arguments using Argument Clinic #87613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
To optimize the creation of objects, a lot of "tp_new" methods are defined twice: once in the legacy way (tp_new slot), once with the new VECTORCALL calling convention (tp_vectorcall slot). My concern is that the VECTORCALL implementation copy/paste most of the code just to parse arguments, whereas the specific code is just a few lines. Example with the float type constructor: static PyObject *
float_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
{
PyObject *return_value = NULL;
PyObject *x = NULL;
if ((type == &PyFloat_Type) &&
!_PyArg_NoKeywords("float", kwargs)) {
goto exit;
}
if (!_PyArg_CheckPositional("float", PyTuple_GET_SIZE(args), 0, 1)) {
goto exit;
}
if (PyTuple_GET_SIZE(args) < 1) {
goto skip_optional;
}
x = PyTuple_GET_ITEM(args, 0);
skip_optional:
return_value = float_new_impl(type, x);
exit:
return return_value;
}
/*[clinic input]
@classmethod
float.\_\_new__ as float_new
x: object(c_default="NULL") = 0
/
Convert a string or number to a floating point number, if possible.
[clinic start generated code]*/
static PyObject *
float_new_impl(PyTypeObject *type, PyObject *x)
/*[clinic end generated code: output=ccf1e8dc460ba6ba input=f43661b7de03e9d8]*/
{
if (type != &PyFloat_Type) {
if (x == NULL) {
x = _PyLong_GetZero();
}
return float_subtype_new(type, x); /* Wimp out */
}
if (x == NULL) {
return PyFloat_FromDouble(0.0);
}
/* If it's a string, but not a string subclass, use
PyFloat_FromString. */
if (PyUnicode_CheckExact(x))
return PyFloat_FromString(x);
return PyNumber_Float(x);
}
static PyObject *
float_vectorcall(PyObject *type, PyObject * const*args,
size_t nargsf, PyObject *kwnames)
{
if (!_PyArg_NoKwnames("float", kwnames)) {
return NULL;
}
Py_ssize_t nargs = PyVectorcall_NARGS(nargsf);
if (!_PyArg_CheckPositional("float", nargs, 0, 1)) {
return NULL;
}
PyObject *x = nargs >= 1 ? args[0] : NULL;
return float_new_impl((PyTypeObject *)type, x);
} Here the float_new() function (tp_new slot) is implemented with Argument Clinic: float_new() C code is generated from the [clinic input] DSL: good! My concern is that float_vectorcall() code is hand written, it's boring to write and boring to maintain. Would it be possible to add a new [clinic input] DSL for vectorcall? I expect something like that: static PyObject *
float_vectorcall(PyObject *type, PyObject * const*args,
size_t nargsf, PyObject *kwnames)
{
if (!_PyArg_NoKwnames("float", kwnames)) {
return NULL;
}
Py_ssize_t nargs = PyVectorcall_NARGS(nargsf);
if (!_PyArg_CheckPositional("float", nargs, 0, 1)) {
return NULL;
}
PyObject *x = nargs >= 1 ? args[0] : NULL;
return float_vectorcall_impl(type, x);
}
static PyObject *
float_vectorcall_impl(PyObject *type, PyObject *x)
{
return float_new_impl((PyTypeObject *)type, x);
} where float_vectorcall() C code would be generated, and float_vectorcall_impl() would be the only part written manually. float_vectorcall_impl() gets a clean API and its body is way simpler to write and to maintain! |
I agree with we should update AC to generate vectorcall. |
I don't have the bandwidth to work on this issue, so I just close it. |
Dong-hee, did you ever get to this? :) If so, I'd be interested in hearing what you found out. |
Sorry, I didn't make progress on this issue. One thing I considered at that moment was supporting positional arguments only cases first. Implementing keyword arguments will be quite complex and not many use cases exist we should cover. so it is not urgent. If you want to take a look at this issue, it will be fine. |
enumrate is good example when you want to handle keyword arguments. as you can read it's too complex. Lines 101 to 150 in 1344cfa
|
No need to be sorry :) I don't have the bandwidth to look at it right now, but I hope to be able to pick it up this fall. Let's keep the issue open. If you find time for this, that is great, but there is no urgency :) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: