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

gh-91162: Fix substitution of unpacked tuples in generic aliases #92335

Merged
merged 9 commits into from
May 8, 2022
Merged

gh-91162: Fix substitution of unpacked tuples in generic aliases #92335

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bdb1c53
gh-91162: Fix substitution of unpacked tuples in generic aliases
serhiy-storchaka May 5, 2022
0d717f0
Merge branch 'main' into typing-subst-unpacked
serhiy-storchaka May 5, 2022
0b78566
Merge branch 'main' into typing-subst-unpacked
serhiy-storchaka May 5, 2022
1397f0a
Fix more old bugs.
serhiy-storchaka May 6, 2022
0067dc8
Improve formatting
serhiy-storchaka May 6, 2022
f0ba8cd
Fix a leak.
serhiy-storchaka May 6, 2022
e8c26d9
Merge branch 'main' into typing-subst-unpacked
serhiy-storchaka May 6, 2022
7e784a9
Merge branch 'main' into typing-subst-unpacked
serhiy-storchaka May 7, 2022
8e4e43d
Simplify tests.
serhiy-storchaka May 7, 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
3 changes: 2 additions & 1 deletion Include/internal/pycore_global_strings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,9 @@ struct _Py_global_strings {
STRUCT_FOR_ID(__subclasshook__)
STRUCT_FOR_ID(__truediv__)
STRUCT_FOR_ID(__trunc__)
STRUCT_FOR_ID(__typing_is_unpacked_typevartuple__)
STRUCT_FOR_ID(__typing_subst__)
STRUCT_FOR_ID(__typing_unpacked__)
STRUCT_FOR_ID(__typing_unpacked_tuple_args__)
STRUCT_FOR_ID(__warningregistry__)
STRUCT_FOR_ID(__weakref__)
STRUCT_FOR_ID(__xor__)
Expand Down
3 changes: 2 additions & 1 deletion Include/internal/pycore_runtime_init.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,8 +824,9 @@ extern "C" {
INIT_ID(__subclasshook__), \
INIT_ID(__truediv__), \
INIT_ID(__trunc__), \
INIT_ID(__typing_is_unpacked_typevartuple__), \
INIT_ID(__typing_subst__), \
INIT_ID(__typing_unpacked__), \
INIT_ID(__typing_unpacked_tuple_args__), \
INIT_ID(__warningregistry__), \
INIT_ID(__weakref__), \
INIT_ID(__xor__), \
Expand Down
68 changes: 34 additions & 34 deletions Lib/test/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,22 +603,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, ...]]'),
('generic[T]', '[*tuple_type[int]]', 'generic[int]'),
# Should raise TypeError: a) according to the tentative spec,
Copy link
Contributor

Choose a reason for hiding this comment

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

We can get rid of the comment here and a few lines below now that these cases do indeed raise a TypeError.

# 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[()]]'),
('generic[T]', '[*Tuple[()]]', 'TypeError'),
('generic[T]', '[*tuple_type[()]]', 'TypeError'),
('generic[T]', '[*tuple_type[int, str]]', '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'),
# Ditto.
('generic[T]', '[*tuple[int, str]]', 'generic[*tuple[int, str]]'),
('generic[T]', '[*Tuple[int, str]]', 'TypeError'),
# Ditto.
('generic[T]', '[*tuple[int, ...]]', 'generic[*tuple[int, ...]]'),
('generic[T]', '[*Tuple[int, ...]]', 'TypeError'),
('generic[T]', '[*tuple_type[int, ...]]', 'TypeError'),
('generic[T]', '[*Ts]', 'TypeError'),
('generic[T]', '[T, *Ts]', 'TypeError'),
('generic[T]', '[*Ts, T]', 'TypeError'),
Expand Down Expand Up @@ -664,23 +658,29 @@ class C(Generic[T1, T2]): pass
('generic[T1, T2]', '[int, str]', 'generic[int, str]'),
('generic[T1, T2]', '[int, str, bool]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[int]]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[int, str]]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[int, str]]', 'generic[int, str]'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the tentative spec we'd decided on at #91162 (comment), even though this is valid and easy to evaluate, we should also disallow it so that the rule stays simple: "If the generic accepts a fixed number of arguments, it can't take any unpacked type arguments."

Ditto various cases below.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the spec should be changed. There is no problem to unpack a tuple of fixed length into the corresponding number of type variables. I think that it is simpler to allow this than add a special exception.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pradeep90 Do you want to argue the case here? I agreed with you in #32341 (comment) tbh mainly because it didn't seem a huge deal and I didn't want to drag out the discussion there, but I actually agree with Serhiy here that it seems simpler to just allow it. Is there context that Serhiy and I are missing here?

As a starting point, I think the rule here could be something like: "You can only use an unpacked arbitrary-length type (such as an unpacked TypeVarTuple or an unpacked arbitrary-length tuple) in the arguments list if the generic accepts an arbitrary number of type parameters". That seems a simple enough rule to me - it disallows tuple[T][*tuple[int, ...]] and tuple[T][*Ts] while allowing tuple[T][*tuple[int]]. Or are there edge cases it wouldn't cover?

Copy link
Contributor

Choose a reason for hiding this comment

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

As a starting point, I think the rule here could be something like: "You can only use an unpacked arbitrary-length type (such as an unpacked TypeVarTuple or an unpacked arbitrary-length tuple) in the arguments list if the generic accepts an arbitrary number of type parameters". That seems a simple enough rule to me

I disagree that it's a simple enough rule. Given an already-complex PEP, this seems like yet another arcane rule for users to keep in mind. The only benefit it gives us is that we allow the fairly useless example of generic[T1, T2][*tuple[int, str]]. Contrast that to the simpler rule that "*<anything> is only for variadic types". That way, users don't have to ever be surprised by weird syntax such as list[T][*tuple[int]].

I don't want to block runtime behavior on this. The runtime can always be more flexible and accept arbitrary cases. Type checkers can forbid this during type checking to keep things simple for users.

In short, for the runtime implementation, I'm ok with whatever is simpler for you and Serhiy. I disagree with changing the spec we agreed on for the sake of this.

Copy link
Contributor

@mrahtz mrahtz May 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, yeah, to be clear - this conversation is totally just about the spec for what we allow at runtime.

If you're happy with the runtime being flexible on this, then so am I. You can go ahead and resolve this, Serhiy.

('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]', '[int, *tuple_type[str]]', 'generic[int, str]'),
('generic[T1, T2]', '[*tuple_type[int], str]', 'generic[int, str]'),
('generic[T1, T2]', '[*tuple_type[int], *tuple_type[str]]', 'generic[int, str]'),
('generic[T1, T2]', '[*tuple_type[int, str], *tuple_type[()]]', 'generic[int, str]'),
('generic[T1, T2]', '[*tuple_type[()], *tuple_type[int, str]]', 'generic[int, str]'),
('generic[T1, T2]', '[*tuple_type[int], *tuple_type[()]]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[()], *tuple_type[int]]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[int, str], *tuple_type[float]]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[int], *tuple_type[str, float]]', '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, ...]]'),
# 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_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]', '[int, *tuple_type[str, ...]]', 'TypeError'),
('generic[T1, T2]', '[*tuple_type[int, ...], 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'),
Expand Down Expand Up @@ -720,7 +720,7 @@ class C(Generic[T1, T2, T3]): pass
tests = [
# Alias # Args # Expected result
('generic[T1, bool, T2]', '[int, str]', 'generic[int, bool, str]'),
('generic[T1, bool, T2]', '[*tuple_type[int, str]]', 'TypeError'),
('generic[T1, bool, T2]', '[*tuple_type[int, str]]', 'generic[int, bool, str]'),
]

for alias_template, args_template, expected_template in tests:
Expand Down Expand Up @@ -765,17 +765,17 @@ class C(Generic[*Ts]): pass
('tuple[*Ts]', '[int, str]', 'tuple[int, str]'),
('Tuple[*Ts]', '[int, str]', 'Tuple[int, str]'),

('C[*Ts]', '[*tuple_type[int]]', 'C[*tuple_type[int]]'), # Should be C[int]
('tuple[*Ts]', '[*tuple_type[int]]', 'tuple[*tuple_type[int]]'), # Should be tuple[int]
('Tuple[*Ts]', '[*tuple_type[int]]', 'Tuple[*tuple_type[int]]'), # Should be Tuple[int]
('C[*Ts]', '[*tuple_type[int]]', 'C[int]'),
('tuple[*Ts]', '[*tuple_type[int]]', 'tuple[int]'),
('Tuple[*Ts]', '[*tuple_type[int]]', 'Tuple[int]'),

('C[*Ts]', '[*tuple_type[*Ts]]', 'C[*tuple_type[*Ts]]'), # Should be C[*Ts]
('tuple[*Ts]', '[*tuple_type[*Ts]]', 'tuple[*tuple_type[*Ts]]'), # Should be tuple[*Ts]
('Tuple[*Ts]', '[*tuple_type[*Ts]]', 'Tuple[*tuple_type[*Ts]]'), # Should be Tuple[*Ts]
('C[*Ts]', '[*tuple_type[*Ts]]', 'C[*Ts]'),
('tuple[*Ts]', '[*tuple_type[*Ts]]', 'tuple[*Ts]'),
('Tuple[*Ts]', '[*tuple_type[*Ts]]', 'Tuple[*Ts]'),

('C[*Ts]', '[*tuple_type[int, str]]', 'C[*tuple_type[int, str]]'), # Should be C[int, str]
('tuple[*Ts]', '[*tuple_type[int, str]]', 'tuple[*tuple_type[int, str]]'), # Should be tuple[int, str]
('Tuple[*Ts]', '[*tuple_type[int, str]]', 'Tuple[*tuple_type[int, str]]'), # Should be Tuple[int, str]
('C[*Ts]', '[*tuple_type[int, str]]', 'C[int, str]'),
('tuple[*Ts]', '[*tuple_type[int, str]]', 'tuple[int, str]'),
('Tuple[*Ts]', '[*tuple_type[int, str]]', 'Tuple[int, str]'),

('C[*Ts]', '[tuple_type[int, ...]]', 'C[tuple_type[int, ...]]'),
('tuple[*Ts]', '[tuple_type[int, ...]]', 'tuple[tuple_type[int, ...]]'),
Expand Down Expand Up @@ -820,11 +820,11 @@ 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'), # 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, ...]]', '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, ...]]
('Tuple[T, *Ts]', '[*Tuple[int, ...]]', 'TypeError'), # Should be Tuple[int, *Tuple[int, ...]]

('C[*Ts, T]', '[int]', 'C[int]'),
Expand Down
68 changes: 50 additions & 18 deletions Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,16 @@ def _check_generic(cls, parameters, elen):
raise TypeError(f"Too {'many' if alen > elen else 'few'} arguments for {cls};"
f" actual {alen}, expected {elen}")

def _unpack_args(args):
newargs = []
for arg in args:
subargs = getattr(arg, '__typing_unpacked_tuple_args__', None)
if subargs is not None and not (subargs and subargs[-1] is ...):
newargs.extend(subargs)
else:
newargs.append(arg)
return newargs

def _prepare_paramspec_params(cls, params):
"""Prepares the parameters for a Generic containing ParamSpec
variables (internal helper).
Expand Down Expand Up @@ -995,7 +1005,8 @@ 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 ((isinstance(arg, _GenericAlias) and arg.__origin__ is Unpack) or
(isinstance(arg, GenericAlias) and getattr(arg, '__unpacked__', False))):
raise TypeError(f"{arg} is not valid as type argument")
return arg

Expand Down Expand Up @@ -1354,19 +1365,17 @@ def __getitem__(self, args):
if self.__origin__ in (Generic, Protocol):
# Can't subscript Generic[...] or Protocol[...].
raise TypeError(f"Cannot subscript already-subscripted {self}")
if not self.__parameters__:
raise TypeError(f"{self} is not a generic class")

# Preprocess `args`.
if not isinstance(args, tuple):
args = (args,)
args = tuple(_type_convert(p) for p in args)
args = _unpack_args(args)
if (self._paramspec_tvars
and any(isinstance(t, ParamSpec) for t in self.__parameters__)):
args = _prepare_paramspec_params(self, args)
elif not any(isinstance(p, TypeVarTuple) for p in self.__parameters__):
# We only run this if there are no TypeVarTuples, because we
# don't check variadic generic arity at runtime (to reduce
# complexity of typing.py).
_check_generic(self, args, len(self.__parameters__))

new_args = self._determine_new_args(args)
r = self.copy_with(new_args)
Expand All @@ -1390,16 +1399,28 @@ def _determine_new_args(self, args):
params = self.__parameters__
# In the example above, this would be {T3: str}
new_arg_by_param = {}
typevartuple_index = None
for i, param in enumerate(params):
if isinstance(param, TypeVarTuple):
j = len(args) - (len(params) - i - 1)
if j < i:
raise TypeError(f"Too few arguments for {self}")
new_arg_by_param.update(zip(params[:i], args[:i]))
new_arg_by_param[param] = args[i: j]
new_arg_by_param.update(zip(params[i + 1:], args[j:]))
break
if typevartuple_index is not None:
raise TypeError(f"More than one TypeVarTuple parameter in {self}")
typevartuple_index = i

alen = len(args)
plen = len(params)
if typevartuple_index is not None:
i = typevartuple_index
j = alen - (plen - i - 1)
if j < i:
raise TypeError(f"Too few arguments for {self};"
f" actual {alen}, expected at least {plen-1}")
new_arg_by_param.update(zip(params[:i], args[:i]))
new_arg_by_param[params[i]] = tuple(args[i: j])
new_arg_by_param.update(zip(params[i + 1:], args[j:]))
else:
if alen != plen:
raise TypeError(f"Too {'many' if alen > plen else 'few'} arguments for {self};"
f" actual {alen}, expected {plen}")
new_arg_by_param.update(zip(params, args))

new_args = []
Expand Down Expand Up @@ -1707,14 +1728,25 @@ def __repr__(self):
return '*' + repr(self.__args__[0])

def __getitem__(self, args):
if self.__typing_unpacked__():
if self.__typing_is_unpacked_typevartuple__:
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))
@property
def __typing_unpacked_tuple_args__(self):
assert self.__origin__ is Unpack
assert len(self.__args__) == 1
arg, = self.__args__
if isinstance(arg, _GenericAlias):
assert arg.__origin__ is tuple
return arg.__args__
return None

@property
def __typing_is_unpacked_typevartuple__(self):
assert self.__origin__ is Unpack
assert len(self.__args__) == 1
return isinstance(self.__args__[0], TypeVarTuple)


class Generic:
Expand Down
Loading