Skip to content
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.

Sign up for GitHub

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

bpo-43224: Implement substitution of unpacked TypeVarTuple in C #31828

Merged
merged 5 commits into from
Apr 30, 2022
Merged

bpo-43224: Implement substitution of unpacked TypeVarTuple in C #31828

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
048cd5a
bpo-43224: Implement substitution of unpacked TypeVarTuple in C
serhiy-storchaka Mar 11, 2022
f4dd9b3
Raise explicit error if substitute a bare TypeVarTuple
serhiy-storchaka Mar 12, 2022
30b5e9f
Merge branch 'main' into typevartuple-subst-c
serhiy-storchaka Mar 12, 2022
985f50f
Merge branch 'main' into typevartuple-subst-c
JelleZijlstra Apr 29, 2022
0b9a4d3
Update unpack test cases
JelleZijlstra Apr 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Include/internal/pycore_global_strings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ struct _Py_global_strings {
STRUCT_FOR_ID(__truediv__)
STRUCT_FOR_ID(__trunc__)
STRUCT_FOR_ID(__typing_subst__)
STRUCT_FOR_ID(__typing_unpacked__)
STRUCT_FOR_ID(__warningregistry__)
STRUCT_FOR_ID(__weakref__)
STRUCT_FOR_ID(__xor__)
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_runtime_init.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,7 @@ extern "C" {
INIT_ID(__truediv__), \
INIT_ID(__trunc__), \
INIT_ID(__typing_subst__), \
INIT_ID(__typing_unpacked__), \
INIT_ID(__warningregistry__), \
INIT_ID(__weakref__), \
INIT_ID(__xor__), \
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ def test_var_substitution(self):
T2 = TypeVar('T2')
class G(Generic[Unpack[Ts]]): pass

for A in G, Tuple:
for A in G, Tuple, tuple:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There need to be more tests for cases where we substitute an unpacked tuple into the TypeVarTuple. I found a crash on your branch:

 % ./python.exe 
Python 3.11.0a6+ (heads/serhiy-storchaka-typevartuple-subst-c:a35af462a3, Mar 11 2022, 19:02:45) [Clang 13.0.0 (clang-1300.0.29.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information
>>> from typing import Unpack, TypeVarTuple
>>> Ts = TypeVarTuple("Ts")
>>> t = tuple[Ts]
>>> t[int, Unpack[tuple[int, str]], str]
Assertion failed: (iparam != varparam), function _Py_subs_parameters, file genericaliasobject.c, line 382.
zsh: abort      ./python.exe

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Is not tuple[Ts] illegal?

I have added a runtime error for this case. It is consistent for Tuple, tuple and user generics.

B = A[Unpack[Ts]]
if A != Tuple:
self.assertEqual(B[()], A[()])
Expand Down
8 changes: 6 additions & 2 deletions Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1622,11 +1622,15 @@ def __repr__(self):
return '*' + repr(self.__args__[0])

def __getitem__(self, args):
if (len(self.__parameters__) == 1 and
isinstance(self.__parameters__[0], TypeVarTuple)):
if self.__typing_unpacked__():
return args
return super().__getitem__(args)

def __typing_unpacked__(self):
# If x is Unpack[tuple[...]], __parameters__ will be empty.
return bool(self.__parameters__ and
isinstance(self.__parameters__[0], TypeVarTuple))


class Generic:
"""Abstract base class for generic types.
Expand Down
112 changes: 98 additions & 14 deletions Objects/genericaliasobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,23 @@ tuple_add(PyObject *self, Py_ssize_t len, PyObject *item)
return 0;
}

static Py_ssize_t
tuple_extend(PyObject **dst, Py_ssize_t dstindex,
PyObject **src, Py_ssize_t count)
{
assert(count >= 0);
if (_PyTuple_Resize(dst, PyTuple_GET_SIZE(*dst) + count - 1) != 0) {
return -1;
}
assert(dstindex + count <= PyTuple_GET_SIZE(*dst));
for (Py_ssize_t i = 0; i < count; ++i) {
PyObject *item = src[i];
Py_INCREF(item);
PyTuple_SET_ITEM(*dst, dstindex + i, item);
}
return dstindex + count;
}

PyObject *
_Py_make_parameters(PyObject *args)
{
Expand Down Expand Up @@ -236,7 +253,8 @@ _Py_make_parameters(PyObject *args)
If obj doesn't have a __parameters__ attribute or that's not
a non-empty tuple, return a new reference to obj. */
static PyObject *
subs_tvars(PyObject *obj, PyObject *params, PyObject **argitems)
subs_tvars(PyObject *obj, PyObject *params,
PyObject **argitems, Py_ssize_t nargs, Py_ssize_t varparam)
{
PyObject *subparams;
if (_PyObject_LookupAttr(obj, &_Py_ID(__parameters__), &subparams) < 0) {
Expand All @@ -250,14 +268,27 @@ subs_tvars(PyObject *obj, PyObject *params, PyObject **argitems)
Py_DECREF(subparams);
return NULL;
}
for (Py_ssize_t i = 0; i < nsubargs; ++i) {
for (Py_ssize_t i = 0, j = 0; i < nsubargs; ++i) {
PyObject *arg = PyTuple_GET_ITEM(subparams, i);
Py_ssize_t iparam = tuple_index(params, nparams, arg);
if (iparam >= 0) {
arg = argitems[iparam];
if (iparam == varparam) {
j = tuple_extend(&subargs, j,
argitems + iparam, nargs - nparams + 1);
if (j < 0) {
return NULL;
}
}
else {
if (iparam >= 0) {
if (iparam > varparam) {
iparam += nargs - nsubargs;
}
arg = argitems[iparam];
}
Py_INCREF(arg);
PyTuple_SET_ITEM(subargs, j, arg);
j++;
}
Py_INCREF(arg);
PyTuple_SET_ITEM(subargs, i, arg);
}

obj = PyObject_GetItem(obj, subargs);
Expand All @@ -271,6 +302,23 @@ subs_tvars(PyObject *obj, PyObject *params, PyObject **argitems)
return obj;
}

static int
_is_unpacked_typevartuple(PyObject *arg)
{
PyObject *meth;
int res = _PyObject_LookupAttr(arg, &_Py_ID(__typing_unpacked__), &meth);
if (res > 0) {
PyObject *tmp = PyObject_CallNoArgs(meth);
Py_DECREF(meth);
if (tmp == NULL) {
return -1;
}
res = PyObject_IsTrue(tmp);
Py_DECREF(tmp);
}
return res;
}

PyObject *
_Py_subs_parameters(PyObject *self, PyObject *args, PyObject *parameters, PyObject *item)
{
Expand All @@ -283,11 +331,27 @@ _Py_subs_parameters(PyObject *self, PyObject *args, PyObject *parameters, PyObje
int is_tuple = PyTuple_Check(item);
Py_ssize_t nitems = is_tuple ? PyTuple_GET_SIZE(item) : 1;
PyObject **argitems = is_tuple ? &PyTuple_GET_ITEM(item, 0) : &item;
if (nitems != nparams) {
return PyErr_Format(PyExc_TypeError,
"Too %s arguments for %R",
nitems > nparams ? "many" : "few",
self);
Py_ssize_t varparam = 0;
for (; varparam < nparams; varparam++) {
PyObject *param = PyTuple_GET_ITEM(parameters, varparam);
if (Py_TYPE(param)->tp_iter) { // TypeVarTuple
break;
}
}
if (varparam < nparams) {
if (nitems < nparams - 1) {
return PyErr_Format(PyExc_TypeError,
"!Too few arguments for %R",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"!Too few arguments for %R",
"Too few arguments for %R",

Or does ! do something here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was temporary, for debugging.

self);
}
}
else {
if (nitems != nparams) {
return PyErr_Format(PyExc_TypeError,
"Too %s arguments for %R",
nitems > nparams ? "many" : "few",
self);
}
}
/* Replace all type variables (specified by parameters)
with corresponding values specified by argitems.
Expand All @@ -300,8 +364,13 @@ _Py_subs_parameters(PyObject *self, PyObject *args, PyObject *parameters, PyObje
if (newargs == NULL) {
return NULL;
}
for (Py_ssize_t iarg = 0; iarg < nargs; iarg++) {
for (Py_ssize_t iarg = 0, jarg = 0; iarg < nargs; iarg++) {
PyObject *arg = PyTuple_GET_ITEM(args, iarg);
int unpack = _is_unpacked_typevartuple(arg);
if (unpack < 0) {
Py_DECREF(newargs);
return NULL;
}
PyObject *subst;
if (_PyObject_LookupAttr(arg, &_Py_ID(__typing_subst__), &subst) < 0) {
Py_DECREF(newargs);
Expand All @@ -310,17 +379,32 @@ _Py_subs_parameters(PyObject *self, PyObject *args, PyObject *parameters, PyObje
if (subst) {
Py_ssize_t iparam = tuple_index(parameters, nparams, arg);
assert(iparam >= 0);
assert(iparam != varparam);
if (iparam > varparam) {
iparam += nitems - nparams;
}
arg = PyObject_CallOneArg(subst, argitems[iparam]);
Py_DECREF(subst);
}
else {
arg = subs_tvars(arg, parameters, argitems);
arg = subs_tvars(arg, parameters, argitems, nitems, varparam);
}
if (arg == NULL) {
Py_DECREF(newargs);
return NULL;
}
PyTuple_SET_ITEM(newargs, iarg, arg);
if (unpack) {
jarg = tuple_extend(&newargs, jarg,
&PyTuple_GET_ITEM(arg, 0), PyTuple_GET_SIZE(arg));
Py_DECREF(arg);
if (jarg < 0) {
return NULL;
}
}
else {
PyTuple_SET_ITEM(newargs, jarg, arg);
jarg++;
}
}

return newargs;
Expand Down