-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] defer inference of legacy TypeVar bound/constraints/defaults #20598
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
Conversation
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-10-09 21:06:16.051150791 +0000
+++ new-output.txt 2025-10-09 21:06:19.328159431 +0000
@@ -1,6 +1,6 @@
WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors.
fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_type_statement.py`: `PEP695TypeAliasType < 'db >::value_type_(Id(cc17)): execute: too many cycle iterations`
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(1643f)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/29ab321/src/function/execute.rs:217:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(1603f)): execute: too many cycle iterations`
_directives_deprecated_library.py:15:31: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `int`
_directives_deprecated_library.py:30:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `str`
_directives_deprecated_library.py:36:41: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@__add__`
@@ -364,6 +364,7 @@
generics_base_class.py:49:22: error[too-many-positional-arguments] Too many positional arguments to class `LinkedList`: expected 1, got 2
generics_base_class.py:61:18: error[too-many-positional-arguments] Too many positional arguments to class `MyDict`: expected 1, got 2
generics_basic.py:34:12: error[unsupported-operator] Operator `+` is unsupported between objects of type `AnyStr@concat` and `AnyStr@concat`
+generics_basic.py:49:44: error[invalid-legacy-type-variable] A `TypeVar` cannot have exactly one constraint
generics_basic.py:139:5: error[type-assertion-failure] Argument does not have asserted type `int`
generics_basic.py:140:5: error[type-assertion-failure] Argument does not have asserted type `int`
generics_basic.py:157:5: error[invalid-argument-type] Method `__getitem__` of type `bound method MyMap1[str, int].__getitem__(key: str, /) -> int` cannot be called with key of type `Literal[0]` on object of type `MyMap1[str, int]`
@@ -575,7 +576,8 @@
generics_upper_bound.py:43:1: error[type-assertion-failure] Argument does not have asserted type `list[int] | set[int]`
generics_upper_bound.py:51:8: error[invalid-argument-type] Argument to function `longer` is incorrect: Argument type `Literal[3]` does not satisfy upper bound `Sized` of type variable `ST`
generics_upper_bound.py:51:11: error[invalid-argument-type] Argument to function `longer` is incorrect: Argument type `Literal[3]` does not satisfy upper bound `Sized` of type variable `ST`
-generics_variance.py:14:6: error[invalid-legacy-type-variable] A legacy `typing.TypeVar` cannot be both covariant and contravariant
+generics_upper_bound.py:56:10: error[invalid-legacy-type-variable] A `TypeVar` cannot have both a bound and constraints
+generics_variance.py:14:6: error[invalid-legacy-type-variable] A `TypeVar` cannot be both covariant and contravariant
generics_variance.py:26:27: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Iterator[T_co@ImmutableList]`
generics_variance.py:57:28: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `B_co@func`
generics_variance.py:175:25: error[non-subscriptable] Cannot subscript object of type `<class 'Contra[typing.TypeVar]'>` with no `__class_getitem__` method
@@ -898,5 +900,5 @@
typeddicts_usage.py:28:17: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
typeddicts_usage.py:28:18: error[invalid-key] Invalid key access on TypedDict `Movie`: Unknown key "title"
typeddicts_usage.py:40:24: error[invalid-type-form] The special form `typing.TypedDict` is not allowed in type expressions. Did you mean to use a concrete TypedDict or `collections.abc.Mapping[str, object]` instead?
-Found 899 diagnostics
+Found 901 diagnostics
WARN A fatal error occurred while checking some files. Not all project files were analyzed. See the diagnostics list above for details. |
|
b7662e9
to
46a2e8b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
CodSpeed Performance ReportMerging #20598 will improve performances by 5.56%Comparing Summary
Benchmarks breakdown
Footnotes
|
9f019f3
to
125dff3
Compare
## Summary Typevar attributes (bound/constraints/default) can be either lazily evaluated or eagerly evaluated. Currently they are lazily evaluated for PEP 695 typevars, and eager for legacy and synthetic typevars. #20598 will make them lazy also for legacy typevars, and the ecosystem report on that PR surfaced the issue fixed here (because legacy typevars are much more common in the ecosystem than PEP 695 typevars.) Applying a transform to a typevar (normalization, materialization, or mark-inferable) will reify all lazy attributes and create a new typevar with eager attributes. In terms of Salsa identity, this transformed typevar will be considered different from the original typevar, whether or not the attributes were actually transformed. In general, this is not a problem, since all typevars in a given generic context will be transformed, or not, together. The exception to this was implicit-self vs explicit Self annotations. The typevar we created for implicit self was created initially using inferable typevars, whereas an explicit Self annotation is initially non-inferable, then transformed via mark-inferable when accessed as part of a function signature. If the containing class (which becomes the upper bound of `Self`) is generic, and has e.g. a lazily-evaluated default, then the explicit-Self annotation will reify that default in the upper bound, and the implicit-self would not, leading them to be treated as different typevars, and causing us to fail to solve a call to a method such as `def method(self) -> Self` correctly. The fix here is to treat implicit-self more like explicit-Self, initially creating it as non-inferable and then using the mark-inferable transform on it. This is less efficient, but restores the invariant that all typevars in a given generic context are transformed together, or not, fixing the bug. In the improved-constraint-solver work, the separation of typevars into "inferable" and "non-inferable" is expected to disappear, along with the mark-inferable transform, which would render both this bug and the fix moot. So this fix is really just temporary until that lands. There is a performance regression, but not a huge one: 1-2% on most projects, 5% on one outlier. This seems acceptable, given that it should be fully recovered by removing the mark-inferable transform. ## Test Plan Added mdtests that failed before this change.
| ^^^^^ | ||
| | ||
"#); | ||
// TODO: This should jump to the definition of `Alias` above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we create an issue for this. It's otherwise hard to remember
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it will be hard to remember, because I have to fix it in order to fix the cases in astral-sh/ty#256 related to TypeAliasType
. So in that sense we already have an issue that ensures we won't forget it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(And I plan to do this PR next.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only reviewed the tests so far
crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/generics/legacy/variables.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/generics/legacy/variables.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/generics/legacy/variables.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
crates/ty_python_semantic/resources/mdtest/diagnostics/legacy_typevars.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Waygood <[email protected]>
Thanks @AlexWaygood for the excellent review here! |
Summary
This allows us to handle self-referential bounds/constraints/defaults without panicking.
Handles more cases from astral-sh/ty#256
This also changes the way we infer the types of legacy TypeVars. Rather than understanding a constructor call to
typing[_extension].TypeVar
inside of any (arbitrarily nested) expression, and having to use a specialassigned_to
field of the semantic index to try to best-effort figure out what name the typevar was assigned to, we instead understand the creation of a legacyTypeVar
only in the supported syntactic position (RHS of a simple un-annotated assignment with one target). In any other position, we just infer it as creating an opaque instance oftyping.TypeVar
. (This behavior matches all other type checkers.)So we now special-case TypeVar creation in
TypeInferenceBuilder
, as a special case of an assignment definition, rather than deeper inside call binding. This does mean we re-implement slightly more of argument-parsing, but in practice this is minimal and easy to handle correctly.This is easier to implement if we also make the RHS of a simple (no unpacking) one-target assignment statement no longer a standalone expression. Which is fine to do, because simple one-target assignments don't need to infer the RHS more than once. This is a bonus performance (0-3% across various projects) and significant memory-usage win, since most assignment statements are simple one-target assignment statements, meaning we now create many fewer standalone-expression salsa ingredients.
This change does mean that inference of manually-constructed
TypeAliasType
instances can no longer find its Definition inassigned_to
, which regresses go-to-definition for these aliases. In a future PR,TypeAliasType
will receive the same treatment thatTypeVar
did in this PR (moving its special-case inference intoTypeInferenceBuilder
and supporting it only in the correct syntactic position, and lazily inferring its value type to support recursion), which will also fix the go-to-definition regression. (I decided a temporary edge-case regression is better in this case than doubling the size of this PR.)This PR also tightens up and fixes various aspects of the validation of
TypeVar
creation, as seen in the tests.We still (for now) treat all typevars as instances of
typing.TypeVar
, even if they were created usingtyping_extensions.TypeVar
. This means we'll wrongly error on e.g.T.__default__
on Python 3.11, even ifT
is atyping_extensions.TypeVar
instance at runtime. We share this wrong behavior with both mypy and pyrefly. It will be easier to fix after we pull in python/typeshed#14840.There are some issues that showed up here with typevar identity and
MarkTypeVarsInferable
; the fix here (using the neworiginal
field andis_identical_to
methods onBoundTypeVarInstance
andTypeVarInstance
) is a bit kludgy, but it can go away when we eliminateMarkTypeVarsInferable
.Test Plan
Added and updated mdtests.
Conformance suite impact
The impact here is all positive:
Ecosystem impact
Basically none; in the setuptools case we just issue slightly different errors on an invalid TypeVar definition, due to the modified validation code.