Skip to content

gh-91162: Disallow tuple[T][*anything] #92255

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

Closed
wants to merge 3 commits into from
Closed

gh-91162: Disallow tuple[T][*anything] #92255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
335747d
Fix starred tuple equality and pickling
mrahtz May 3, 2022
5d384b9
Update test comment
mrahtz May 3, 2022
feba81b
Disallow tuple[T][*anything]
mrahtz May 2, 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
19 changes: 16 additions & 3 deletions Lib/test/test_genericalias.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ def test_equality(self):
self.assertEqual(list[int], list[int])
self.assertEqual(dict[str, int], dict[str, int])
self.assertEqual((*tuple[int],)[0], (*tuple[int],)[0])
self.assertNotEqual(tuple[int], (*tuple[int],)[0])
self.assertNotEqual(tuple[int], Unpack[tuple[int]])
self.assertEqual(
tuple[
tuple( # Effectively the same as starring; TODO
Expand Down Expand Up @@ -394,6 +396,9 @@ def test_pickle(self):
self.assertEqual(loaded.__origin__, alias.__origin__)
self.assertEqual(loaded.__args__, alias.__args__)
self.assertEqual(loaded.__parameters__, alias.__parameters__)
if isinstance(alias, GenericAlias):
self.assertEqual(loaded.__unpacked__, alias.__unpacked__)


def test_copy(self):
class X(list):
Expand All @@ -419,11 +424,19 @@ def __deepcopy__(self, memo):
self.assertEqual(copied.__parameters__, alias.__parameters__)

def test_unpack(self):
alias = tuple[str, ...]
self.assertIs(alias.__unpacked__, False)
unpacked = (*alias,)[0]
alias1 = tuple[str, ...]
self.assertIs(alias1.__unpacked__, False)
unpacked = (*alias1,)[0]
self.assertIs(unpacked.__unpacked__, True)

# The (optional) third positional argument should control unpackedness.
alias2 = GenericAlias(tuple, int)
self.assertIs(alias2.__unpacked__, False)
alias3 = GenericAlias(tuple, int, False)
self.assertIs(alias3.__unpacked__, False)
alias4 = GenericAlias(tuple, int, True)
self.assertIs(alias4.__unpacked__, True)

def test_union(self):
a = typing.Union[list[int], list[str]]
self.assertEqual(a.__args__, (list[int], list[str]))
Expand Down
70 changes: 31 additions & 39 deletions Lib/test/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,25 @@ def test_no_duplicates_if_replacement_not_in_templates(self):
class GenericAliasSubstitutionTests(BaseTestCase):
"""Tests for type variable substitution in generic aliases.

Note that the expected results here are tentative, based on a
still-being-worked-out spec for what we allow at runtime (given that
implementation of *full* substitution logic at runtime would add too much
complexity to typing.py). This spec is currently being discussed at
https://github.com/python/cpython/issues/91162.
Note that not all the rules governing substitution behavior at runtime
is codified in PEPs; the source of truth for these tests is the tests
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
is codified in PEPs; the source of truth for these tests is the tests
are codified in PEPs; the source of truth for these tests is the tests

themselves.

Informally, the specification is as follows:
* In general, we aim to support as many valid substitutions (as specified
by the PEPs themselves) as possible at runtme.
* Leniency: In some cases, we also choose to allow some substitutions that
the PEPs themselves might forbid. This is partly for simplicity - we want
to minimise complexity in the runtime - and partly to enable users to
experiment with new ways to use types.
* Exceptions:
* PEP 646 Exception 1: Unpacked types (e.g. *tuple[int], *tuple[int, ...],
*Ts where Ts is a TypeVarTuple) cannot be used as arguments to generic
aliases which expect a fixed number of arguments. See the tests
themselves for examples.
* PEP 646 Exception 2: Unpacked TypeVarTuples can only be used as
arguments to generic aliases whose sole parameter is also an unpacked
TypeVarTuple.
"""

def test_one_parameter(self):
Expand All @@ -603,22 +617,16 @@ class C(Generic[T]): pass
('generic[T]', '[int]', 'generic[int]'),
('generic[T]', '[int, str]', 'TypeError'),
('generic[T]', '[tuple_type[int, ...]]', 'generic[tuple_type[int, ...]]'),
# Should raise TypeError: a) according to the tentative spec,
# unpacked types cannot be used as arguments to aliases that expect
# a fixed number of arguments; b) it's equivalent to generic[()].
('generic[T]', '[*tuple[()]]', 'generic[*tuple[()]]'),
# See PEP 646 Exception 1 in docstring.
('generic[T]', '[*tuple[()]]', 'TypeError'),
('generic[T]', '[*Tuple[()]]', 'TypeError'),
# Should raise TypeError according to the tentative spec: unpacked
# types cannot be used as arguments to aliases that expect a fixed
# number of arguments.
('generic[T]', '[*tuple[int]]', 'generic[*tuple[int]]'),
('generic[T]', '[*tuple[int]]', 'TypeError'),
('generic[T]', '[*Tuple[int]]', 'TypeError'),
# Ditto.
('generic[T]', '[*tuple[int, str]]', 'generic[*tuple[int, str]]'),
('generic[T]', '[*tuple[int, str]]', 'TypeError'),
('generic[T]', '[*Tuple[int, str]]', 'TypeError'),
# Ditto.
('generic[T]', '[*tuple[int, ...]]', 'generic[*tuple[int, ...]]'),
('generic[T]', '[*tuple[int, ...]]', 'TypeError'),
('generic[T]', '[*Tuple[int, ...]]', 'TypeError'),
# See PEP 646 Exception 2 in docstring.
('generic[T]', '[*Ts]', 'TypeError'),
('generic[T]', '[T, *Ts]', 'TypeError'),
('generic[T]', '[*Ts, T]', 'TypeError'),
Expand Down Expand Up @@ -663,30 +671,19 @@ class C(Generic[T1, T2]): pass
('generic[T1, T2]', '[int]', 'TypeError'),
('generic[T1, T2]', '[int, str]', 'generic[int, str]'),
('generic[T1, T2]', '[int, str, bool]', 'TypeError'),
('generic[T1, T2]', '[tuple_type[int, ...], tuple_type[str, ...]]', 'generic[tuple_type[int, ...], tuple_type[str, ...]]'),
# See PEP 646 Exception 1 in docstring.
('generic[T1, T2]', '[*tuple_type[int]]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[int, str]]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[int, str, bool]]', 'TypeError'),

# Should raise TypeError according to the tentative spec: unpacked
# types cannot be used as arguments to aliases that expect a fixed
# number of arguments.
('generic[T1, T2]', '[*tuple[int, str], *tuple[float, bool]]', 'generic[*tuple[int, str], *tuple[float, bool]]'),
('generic[T1, T2]', '[*Tuple[int, str], *Tuple[float, bool]]', 'TypeError'),

('generic[T1, T2]', '[*tuple_type[int, str], *tuple_type[float, bool]]', 'TypeError'),
('generic[T1, T2]', '[tuple_type[int, ...]]', 'TypeError'),
('generic[T1, T2]', '[tuple_type[int, ...], tuple_type[str, ...]]', 'generic[tuple_type[int, ...], tuple_type[str, ...]]'),
('generic[T1, T2]', '[*tuple_type[int, ...]]', 'TypeError'),

# Ditto.
('generic[T1, T2]', '[*tuple[int, ...], *tuple[str, ...]]', 'generic[*tuple[int, ...], *tuple[str, ...]]'),
('generic[T1, T2]', '[*Tuple[int, ...], *Tuple[str, ...]]', 'TypeError'),

('generic[T1, T2]', '[*tuple_type[int, ...], *tuple_type[str, ...]]', 'TypeError'),
('generic[T1, T2]', '[*Ts]', 'TypeError'),
('generic[T1, T2]', '[T, *Ts]', 'TypeError'),
('generic[T1, T2]', '[*Ts, T]', 'TypeError'),
# Should raise TypeError according to the tentative spec: unpacked
# types cannot be used as arguments to generics that expect a fixed
# number of arguments.
# Should raise TypeError - see PEP 646 Exception 1 in docstring.
# (None of the things in `generics` were defined using *Ts.)
('generic[T1, *tuple_type[int, ...]]', '[str]', 'generic[str, *tuple_type[int, ...]]'),
]
Expand Down Expand Up @@ -820,12 +817,7 @@ class C(Generic[*Ts]): pass
('tuple[T, *Ts]', '[int, str, bool]', 'tuple[int, str, bool]'),
('Tuple[T, *Ts]', '[int, str, bool]', 'Tuple[int, str, bool]'),

('C[T, *Ts]', '[*tuple[int, ...]]', 'C[*tuple[int, ...]]'), # Should be C[int, *tuple[int, ...]]
('C[T, *Ts]', '[*Tuple[int, ...]]', 'TypeError'), # Ditto
('tuple[T, *Ts]', '[*tuple[int, ...]]', 'tuple[*tuple[int, ...]]'), # Should be tuple[int, *tuple[int, ...]]
('tuple[T, *Ts]', '[*Tuple[int, ...]]', 'TypeError'), # Should be tuple[int, *Tuple[int, ...]]
('Tuple[T, *Ts]', '[*tuple[int, ...]]', 'Tuple[*tuple[int, ...]]'), # Should be Tuple[int, *tuple[int, ...]]
('Tuple[T, *Ts]', '[*Tuple[int, ...]]', 'TypeError'), # Should be Tuple[int, *Tuple[int, ...]]
('generic[T, *Ts]', '[*tuple_type[int, ...]]', 'TypeError'), # Should be generic[int, *tuple_type[int, ...]]

('C[*Ts, T]', '[int]', 'C[int]'),
('tuple[*Ts, T]', '[int]', 'tuple[int]'),
Expand Down
11 changes: 10 additions & 1 deletion Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,15 @@ def __repr__(self):
return f'ForwardRef({self.__forward_arg__!r}{module_repr})'


def _is_unpacked_type(x: Any) -> bool:
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
def _is_unpacked_type(x: Any) -> bool:
def _is_unpacked_type(x: object) -> bool:

return (
# E.g. *tuple[int]
(isinstance(x, GenericAlias) and x.__unpacked__)
or
# E.g. Unpack[tuple[int]]
(isinstance(x, _GenericAlias) and x.__origin__ is Unpack)
)

def _is_unpacked_typevartuple(x: Any) -> bool:
return (
isinstance(x, _UnpackGenericAlias)
Expand Down Expand Up @@ -995,7 +1004,7 @@ def __init__(self, name, *constraints, bound=None,
def __typing_subst__(self, arg):
msg = "Parameters to generic types must be types."
arg = _type_check(arg, msg, is_argument=True)
if (isinstance(arg, _GenericAlias) and arg.__origin__ is Unpack):
if _is_unpacked_type(arg):
raise TypeError(f"{arg} is not valid as type argument")
return arg

Expand Down
26 changes: 20 additions & 6 deletions Objects/genericaliasobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,9 @@ ga_richcompare(PyObject *a, PyObject *b, int op)

gaobject *aa = (gaobject *)a;
gaobject *bb = (gaobject *)b;
if (aa->starred != bb->starred) {
Py_RETURN_FALSE;
}
int eq = PyObject_RichCompareBool(aa->origin, bb->origin, Py_EQ);
if (eq < 0) {
return NULL;
Expand Down Expand Up @@ -604,8 +607,8 @@ static PyObject *
ga_reduce(PyObject *self, PyObject *Py_UNUSED(ignored))
{
gaobject *alias = (gaobject *)self;
return Py_BuildValue("O(OO)", Py_TYPE(alias),
alias->origin, alias->args);
return Py_BuildValue("O(OOb)", Py_TYPE(alias),
alias->origin, alias->args, alias->starred);
}

static PyObject *
Expand Down Expand Up @@ -685,7 +688,7 @@ static PyGetSetDef ga_properties[] = {
* Returns 1 on success, 0 on failure.
*/
static inline int
setup_ga(gaobject *alias, PyObject *origin, PyObject *args) {
setup_ga(gaobject *alias, PyObject *origin, PyObject *args, bool starred) {
if (!PyTuple_Check(args)) {
args = PyTuple_Pack(1, args);
if (args == NULL) {
Expand All @@ -700,6 +703,7 @@ setup_ga(gaobject *alias, PyObject *origin, PyObject *args) {
alias->origin = origin;
alias->args = args;
alias->parameters = NULL;
alias->starred = starred;
alias->weakreflist = NULL;

if (PyVectorcall_Function(origin) != NULL) {
Expand All @@ -718,16 +722,26 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
if (!_PyArg_NoKeywords("GenericAlias", kwds)) {
return NULL;
}
if (!_PyArg_CheckPositional("GenericAlias", PyTuple_GET_SIZE(args), 2, 2)) {
if (!_PyArg_CheckPositional("GenericAlias", PyTuple_GET_SIZE(args), 2, 3)) {
return NULL;
}

PyObject *origin = PyTuple_GET_ITEM(args, 0);
PyObject *arguments = PyTuple_GET_ITEM(args, 1);

bool starred;
if (PyTuple_Size(args) < 3) {
starred = false;
} else {
PyObject *py_starred = PyTuple_GET_ITEM(args, 2);
starred = PyLong_AsLong(py_starred);
}

gaobject *self = (gaobject *)type->tp_alloc(type, 0);
if (self == NULL) {
return NULL;
}
if (!setup_ga(self, origin, arguments)) {
if (!setup_ga(self, origin, arguments, starred)) {
Py_DECREF(self);
return NULL;
}
Expand Down Expand Up @@ -837,7 +851,7 @@ Py_GenericAlias(PyObject *origin, PyObject *args)
if (alias == NULL) {
return NULL;
}
if (!setup_ga(alias, origin, args)) {
if (!setup_ga(alias, origin, args, false)) {
Py_DECREF(alias);
return NULL;
}
Expand Down