From be43234d6854712281201fe6e100f45d039f8d9b Mon Sep 17 00:00:00 2001 From: alexdrydew Date: Sat, 16 Nov 2024 15:50:42 +0000 Subject: [PATCH 1/6] optimize is_unsafe_overlapping_overload_signatures --- mypy/checker.py | 113 +++++++++++++++++++++++++----------------------- 1 file changed, 58 insertions(+), 55 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 1bee348bc252..be5eb36c729d 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -10,6 +10,7 @@ Callable, Dict, Final, + Generator, Generic, Iterable, Iterator, @@ -8039,33 +8040,29 @@ def are_argument_counts_overlapping(t: CallableType, s: CallableType) -> bool: return min_args <= max_args -def expand_callable_variants(c: CallableType) -> list[CallableType]: - """Expand a generic callable using all combinations of type variables' values/bounds.""" - for tv in c.variables: - # We need to expand self-type before other variables, because this is the only - # type variable that can have other type variables in the upper bound. - if tv.id.is_self(): - c = expand_type(c, {tv.id: tv.upper_bound}).copy_modified( - variables=[v for v in c.variables if not v.id.is_self()] - ) - break - - if not c.is_generic(): - # Fast path. - return [c] - +def get_type_var_group_variants( + variables: list[TypeVarLikeType], +) -> Generator[dict[TypeVarId, Type], None, None]: tvar_values = [] - for tvar in c.variables: + for tvar in variables: if isinstance(tvar, TypeVarType) and tvar.values: tvar_values.append(tvar.values) else: tvar_values.append([tvar.upper_bound]) - variants = [] for combination in itertools.product(*tvar_values): - tvar_map = {tv.id: subst for (tv, subst) in zip(c.variables, combination)} - variants.append(expand_type(c, tvar_map).copy_modified(variables=[])) - return variants + yield {tv.id: subst for (tv, subst) in zip(variables, combination)} + + +def expand_callable_self(c: CallableType) -> CallableType: + for tv in c.variables: + # We need to expand self-type before other variables, because this is the only + # type variable that can have other type variables in the upper bound. + if tv.id.is_self(): + return expand_type(c, {tv.id: tv.upper_bound}).copy_modified( + variables=[v for v in c.variables if not v.id.is_self()] + ) + return c def is_unsafe_overlapping_overload_signatures( @@ -8104,46 +8101,52 @@ def is_unsafe_overlapping_overload_signatures( # Note: We repeat this check twice in both directions compensate for slight # asymmetries in 'is_callable_compatible'. - for sig_variant in expand_callable_variants(signature): - for other_variant in expand_callable_variants(other): - # Using only expanded callables may cause false negatives, we can add - # more variants (e.g. using inference between callables) in the future. - if is_subset_no_promote(sig_variant.ret_type, other_variant.ret_type): - continue - if not ( - is_callable_compatible( - sig_variant, - other_variant, - is_compat=is_overlapping_types_for_overload, - check_args_covariantly=False, - is_proper_subtype=False, - is_compat_return=lambda l, r: not is_subset_no_promote(l, r), - allow_partial_overlap=True, - ) - or is_callable_compatible( - other_variant, - sig_variant, - is_compat=is_overlapping_types_for_overload, - check_args_covariantly=True, - is_proper_subtype=False, - is_compat_return=lambda l, r: not is_subset_no_promote(r, l), - allow_partial_overlap=True, - ) - ): - continue - # Using the same `allow_partial_overlap` flag as before, can cause false - # negatives in case where star argument is used in a catch-all fallback overload. - # But again, practicality beats purity here. - if not partial_only or not is_callable_compatible( + # We need to expand self-type before other variables, because this is the only + # type variable that can have other type variables in the upper bound. + signature = expand_callable_self(signature) + other = expand_callable_self(other) + + all_variables = {v.id: v for v in signature.variables} | {v.id: v for v in other.variables} + for tvar_map in get_type_var_group_variants(all_variables.values()): + sig_variant = expand_type(signature, tvar_map).copy_modified(variables=[]) + other_variant = expand_type(other, tvar_map).copy_modified(variables=[]) + + if is_subset_no_promote(sig_variant.ret_type, other_variant.ret_type): + continue + if not ( + is_callable_compatible( + sig_variant, + other_variant, + is_compat=is_overlapping_types_for_overload, + check_args_covariantly=False, + is_proper_subtype=False, + is_compat_return=lambda l, r: not is_subset_no_promote(l, r), + allow_partial_overlap=True, + ) + or is_callable_compatible( other_variant, sig_variant, - is_compat=is_subset_no_promote, + is_compat=is_overlapping_types_for_overload, check_args_covariantly=True, is_proper_subtype=False, - ignore_return=True, + is_compat_return=lambda l, r: not is_subset_no_promote(r, l), allow_partial_overlap=True, - ): - return True + ) + ): + continue + # Using the same `allow_partial_overlap` flag as before, can cause false + # negatives in case where star argument is used in a catch-all fallback overload. + # But again, practicality beats purity here. + if not partial_only or not is_callable_compatible( + other_variant, + sig_variant, + is_compat=is_subset_no_promote, + check_args_covariantly=True, + is_proper_subtype=False, + ignore_return=True, + allow_partial_overlap=True, + ): + return True return False From aa95616096867019ac04172f93333423d7aa194d Mon Sep 17 00:00:00 2001 From: alexdrydew Date: Sat, 16 Nov 2024 15:51:50 +0000 Subject: [PATCH 2/6] add docstring --- mypy/checker.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mypy/checker.py b/mypy/checker.py index be5eb36c729d..16463a86306c 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -8043,6 +8043,8 @@ def are_argument_counts_overlapping(t: CallableType, s: CallableType) -> bool: def get_type_var_group_variants( variables: list[TypeVarLikeType], ) -> Generator[dict[TypeVarId, Type], None, None]: + """Expand a group of type variables into all possible combinations of their values.""" + tvar_values = [] for tvar in variables: if isinstance(tvar, TypeVarType) and tvar.values: From c9b465fb82f2e8a0eddf36acb652915357b6abc6 Mon Sep 17 00:00:00 2001 From: alexdrydew Date: Sat, 16 Nov 2024 16:50:24 +0000 Subject: [PATCH 3/6] fix dict union for 3.8 --- mypy/checker.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index 16463a86306c..b3981f534db5 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -8108,7 +8108,9 @@ def is_unsafe_overlapping_overload_signatures( signature = expand_callable_self(signature) other = expand_callable_self(other) - all_variables = {v.id: v for v in signature.variables} | {v.id: v for v in other.variables} + all_variables = {v.id: v for v in signature.variables} + all_variables.update({v.id: v for v in other.variables}) + for tvar_map in get_type_var_group_variants(all_variables.values()): sig_variant = expand_type(signature, tvar_map).copy_modified(variables=[]) other_variant = expand_type(other, tvar_map).copy_modified(variables=[]) From ba26d12c285a7117cedcf1781a331f33a36ea6bc Mon Sep 17 00:00:00 2001 From: alexdrydew Date: Sat, 16 Nov 2024 16:58:53 +0000 Subject: [PATCH 4/6] fix type annotation --- mypy/checker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index b3981f534db5..65bbb8558af8 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -8,6 +8,7 @@ from typing import ( AbstractSet, Callable, + Collection, Dict, Final, Generator, @@ -8041,7 +8042,7 @@ def are_argument_counts_overlapping(t: CallableType, s: CallableType) -> bool: def get_type_var_group_variants( - variables: list[TypeVarLikeType], + variables: Collection[TypeVarLikeType], ) -> Generator[dict[TypeVarId, Type], None, None]: """Expand a group of type variables into all possible combinations of their values.""" From ba8615716286e2254bc78e5a359522d1077fde95 Mon Sep 17 00:00:00 2001 From: alexdrydew Date: Sun, 17 Nov 2024 21:06:28 +0000 Subject: [PATCH 5/6] disallow exactly same overloads with different return value --- mypy/checker.py | 36 +++++++++++++++++++-------- test-data/unit/check-overloading.test | 18 ++++++++++++-- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 65bbb8558af8..6dda0d326149 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -8059,8 +8059,6 @@ def get_type_var_group_variants( def expand_callable_self(c: CallableType) -> CallableType: for tv in c.variables: - # We need to expand self-type before other variables, because this is the only - # type variable that can have other type variables in the upper bound. if tv.id.is_self(): return expand_type(c, {tv.id: tv.upper_bound}).copy_modified( variables=[v for v in c.variables if not v.id.is_self()] @@ -8142,14 +8140,32 @@ def is_unsafe_overlapping_overload_signatures( # Using the same `allow_partial_overlap` flag as before, can cause false # negatives in case where star argument is used in a catch-all fallback overload. # But again, practicality beats purity here. - if not partial_only or not is_callable_compatible( - other_variant, - sig_variant, - is_compat=is_subset_no_promote, - check_args_covariantly=True, - is_proper_subtype=False, - ignore_return=True, - allow_partial_overlap=True, + + # Also earlier overload may not be more general overall but is more general when + # narrowed to common calls is handled here, so there is no unsafe overlap because + # it will still be caught by the earlier overload. + # Exeption here is when signature variants are exactly the same, in which case we + # should still consider them overlapping. + if ( + not partial_only + or not is_callable_compatible( + other_variant, + sig_variant, + is_compat=is_subset_no_promote, + check_args_covariantly=True, + is_proper_subtype=False, + ignore_return=True, + allow_partial_overlap=True, + ) + or is_callable_compatible( + sig_variant, + other_variant, + is_compat=lambda l, r: l == r, + check_args_covariantly=False, + is_proper_subtype=False, + ignore_return=True, + allow_partial_overlap=False, + ) ): return True return False diff --git a/test-data/unit/check-overloading.test b/test-data/unit/check-overloading.test index 9d01ce6bd480..cd258927dc36 100644 --- a/test-data/unit/check-overloading.test +++ b/test-data/unit/check-overloading.test @@ -1327,9 +1327,8 @@ def h(x: Sequence[str]) -> int: pass @overload def h(x: Sequence[T]) -> None: pass # E: Overloaded function signature 2 will never be matched: signature 1's parameter type(s) are the same or broader -# Safety of this highly depends on the implementation, so we lean towards being silent. @overload -def i(x: List[str]) -> int: pass +def i(x: List[str]) -> int: pass # E: Overloaded function signatures 1 and 2 overlap with incompatible return types @overload def i(x: List[T]) -> None: pass [builtins fixtures/list.pyi] @@ -6768,3 +6767,18 @@ class D(Generic[T]): a: D[str] # E: Type argument "str" of "D" must be a subtype of "C" reveal_type(a.f(1)) # N: Revealed type is "builtins.int" reveal_type(a.f("x")) # N: Revealed type is "builtins.str" + + +[case testOverloadOnExactSameTypeVariantWithIncompetibleReturnTypes] +from typing import TypeVar, overload + +class A: pass +class B: pass + +T = TypeVar('T', A, B) + +@overload +def f(x: B) -> int: ... # E: Overloaded function signatures 1 and 2 overlap with incompatible return types +@overload +def f(x: T) -> str: ... +def f(x): ... From 5ddfc479d92c568a6bec9b29c0403ef210b27151 Mon Sep 17 00:00:00 2001 From: alexdrydew Date: Tue, 19 Nov 2024 22:02:52 +0000 Subject: [PATCH 6/6] add test case for many type variables overload --- test-data/unit/check-overloading.test | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test-data/unit/check-overloading.test b/test-data/unit/check-overloading.test index cd258927dc36..d15e1e06266d 100644 --- a/test-data/unit/check-overloading.test +++ b/test-data/unit/check-overloading.test @@ -6782,3 +6782,28 @@ def f(x: B) -> int: ... # E: Overloaded function signatures 1 and 2 overlap wit @overload def f(x: T) -> str: ... def f(x): ... + + + +[case testOverloadOnManyTypeVariablesGenericClassDoesNotSignificantlyDegradePerformance] +from typing import Generic, TypeVar, overload + +class A: pass + +class B: pass + +class C: pass + +T1 = TypeVar("T1", A, B, C) +T2 = TypeVar("T2", A, B, C) +T3 = TypeVar("T3", A, B, C) +T4 = TypeVar("T4", A, B, C) +T5 = TypeVar("T5", A, B, C) +T6 = TypeVar("T6", A, B, C) + +class Container(Generic[T1, T2, T3, T4, T5, T6]): + @overload + def f(self, t1: int, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6) -> int: ... + @overload + def f(self, t1: str, t2: T2, t3: T3, t4: T4, t5: T5, t6: T6) -> str: ... + def f(self, t1, t2, t3, t4, t5, t6): ...