[ty] Don't track inferability via different Type variants
#20677
+904
−547
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcreager
added
internal
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
5 | 5 | 4 |
unresolved-attribute |
3 | 11 | 0 |
invalid-return-type |
0 | 12 | 1 |
no-matching-overload |
1 | 11 | 0 |
unused-ignore-comment |
5 | 0 | 0 |
invalid-assignment |
1 | 1 | 2 |
type-assertion-failure |
3 | 0 | 0 |
redundant-cast |
1 | 0 | 0 |
unsupported-operator |
0 | 1 | 0 |
| Total | 19 | 41 | 7 |
dcreager
force-pushed
the
dcreager/non-non-inferable
branch
from
6bc7ceb to
417caf0
Compare
dcreager
force-pushed
the
dcreager/non-non-inferable
branch
from
417caf0 to
7b5684f
Compare
|
ecosystem analysis: Several projects have removed diagnostics, which look like correct better understanding of the types involved:
Still looking at some of the new diagnostics. It also looks like we aren't expanding type aliases in as many places as before. |
dcreager
force-pushed
the
dcreager/non-non-inferable
branch
from
7a69d29 to
df385d2
Compare
dcreager
force-pushed
the
dcreager/non-non-inferable
branch
from
f11b47c to
74cc6b7
Compare
dcreager
force-pushed
the
dcreager/non-non-inferable
branch
from
74cc6b7 to
6d12c00
Compare
dcreager
force-pushed
the
dcreager/non-non-inferable
branch
from
a2266c0 to
5f708ba
Compare
CodSpeed Performance ReportMerging #20677 will degrade performances by 4.28%Comparing Summary
Benchmarks breakdown
Footnotes
|
Resolves conflicts by: - Using TypeVarIdentity and BoundTypeVarIdentity for identity comparisons - Updating InferableTypeVars to use BoundTypeVarIdentity instead of BoundTypeVarInstance - Adding db parameter to all is_inferable() calls - Making BoundTypeVarIdentity implement salsa::Update for use in tracked functions - Updating SpecializationBuilder to use BoundTypeVarIdentity as map keys - Converting inferable typevars collection to use identities This fixes the regression where method calls on Top-materialized types were failing with invalid-argument-type errors due to typevar identity mismatches.
Update DisplayBoundTypeVarInstance to DisplayBoundTypeVarIdentity and move the display() method from BoundTypeVarInstance to BoundTypeVarIdentity. Since the display implementation only uses the identity fields (name and binding context), it makes more sense for it to operate on the identity type directly. This also reduces coupling to the full BoundTypeVarInstance. Updated all callers to use .identity(db).display(db) when displaying a BoundTypeVarInstance.
Change ConstrainedTypeVar to store BoundTypeVarIdentity instead of BoundTypeVarInstance. This makes constraints independent of typevar materialization, which is appropriate since constraints should only care about the logical identity of a typevar, not its specific bounds. Updated: - ConstrainedTypeVar::typevar field to BoundTypeVarIdentity - ConstraintSet::range() and negated_range() to accept BoundTypeVarIdentity - ConstrainedTypeVar::new_node() signature - All callers in function.rs to pass .identity(db) - Display code which now receives BoundTypeVarIdentity directly
* dcreager/typevar-identity: pre-commit clippy Update ConstraintSet to use BoundTypeVarIdentity clippy Move display method to BoundTypeVarIdentity
* dcreager/typevar-identity: missed one
dcreager
added a commit
that referenced
this pull request
Oct 13, 2025
…ing scope (#20822) Generic classes are not allowed to bind or reference a typevar from an enclosing scope: ```py def f[T](x: T, y: T) -> None: class Ok[S]: ... # error: [invalid-generic-class] class Bad1[T]: ... # error: [invalid-generic-class] class Bad2(Iterable[T]): ... class C[T]: class Ok1[S]: ... # error: [invalid-generic-class] class Bad1[T]: ... # error: [invalid-generic-class] class Bad2(Iterable[T]): ... ``` It does not matter if the class uses PEP 695 or legacy syntax. It does not matter if the enclosing scope is a generic class or function. The generic class cannot even _reference_ an enclosing typevar in its base class list. This PR adds diagnostics for these cases. In addition, the PR adds better fallback behavior for generic classes that violate this rule: any enclosing typevars are not included in the class's generic context. (That ensures that we don't inadvertently try to infer specializations for those typevars in places where we shouldn't.) The `dulwich` ecosystem project has [examples of this](https://github.com/jelmer/dulwich/blob/d912eaaffd60b4978ff92b91a8300e2cd234e0fc/dulwich/config.py#L251) that were causing new false positives on #20677. --------- Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
dcreager
added a commit
that referenced
this pull request
Oct 14, 2025
As part of #20598, we added `is_identical_to` methods to `TypeVarInstance` and `BoundTypeVarInstance`, which compare when two typevar instances refer to "the same" underlying typevar, even if we have forced their lazy bounds/constraints as part of marking typevars as inferable. (Doing so results in a different salsa interned struct ID, since we've changed the contents of the `bounds_or_constraints` field.) It turns out that marking typevars as inferable is not the only way that we might force lazy bounds/constraints; it also happens when we materialize a type containing a typevar. This surfaced as ecosystem report failures on #20677. That means that we need a more long-term fix to this problem. (`is_identical_to`, and its underlying `original` field, were meant to be a temporary fix until we removed the `MarkTypeVarsInferable` type mapping.) This PR extracts out a separate type (`TypeVarIdentity`) that only includes the fields that actually inform whether two typevars are "the same". All other properties of the typevar (default, bounds/constraints, etc) still live in `TypeVarInstance`. Call sites that care about typevar identity can now either store just `TypeVarIdentity` (if they never need access to those other properties), or continue to store `TypeVarInstance` but pull out its `identity` when performing those "are they the same typevar" comparisons. (All of this also applies respectively to `BoundTypeVar{Identity,Instance}`.) In particular, constraint sets now work on `BoundTypeVarIdentity`, and generic contexts still _store_ a `BoundTypeVarInstance` (since we might need access to defaults when specializing), but are keyed on `BoundTypeVarIdentity`.
…rable * origin/main: (26 commits) [ty] Add separate type for typevar "identity" (#20813) [ty] Diagnostic for generic classes that reference typevars in enclosing scope (#20822) Update Python compatibility from 3.13 to 3.14 in README.md (#20852) [syntax-errors]: break outside loop F701 (#20556) [ty] Treat `Callable`s as bound-method descriptors in special cases (#20802) [ty] Do not bind self to non-positional parameters (#20850) Fix syntax error false positives on parenthesized context managers (#20846) [ty] Remove 'pre-release software' warning (#20817) Render unsupported syntax errors in formatter tests (#20777) [ty] Treat functions, methods, and dynamic types as function-like `Callable`s (#20842) [ty] Move logic for `super()` inference to a new `types::bound_super` submodule (#20840) [ty] Fix false-positive diagnostics on `super()` calls (#20814) [ty] Move `class_member` to `member` module (#20837) [`ruff`] Use DiagnosticTag for more flake8 and numpy rules (#20758) [ty] Prefer declared base class attribute over inferred attribute on subclass (#20764) [ty] Log files that are slow to type check (#20836) Update cargo-bins/cargo-binstall action to v1.15.7 (#20827) Update CodSpeedHQ/action action to v4.1.1 (#20828) Update Rust crate pyproject-toml to v0.13.7 (#20835) Update Rust crate anstream to v0.6.21 (#20829) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have to track whether a typevar appears in a position where it's inferable or not. In a non-inferable position (in the body of the generic class or function that binds it), assignability must hold for every possible specialization of the typevar. In an inferable position, it only needs to hold for some specialization. #20093 is working on using constraint sets to model assignability of typevars, and the constraint sets that we produce will be the same for inferable vs non-inferable typevars; what changes is what we compare that constraint set to. (For a non-inferable typevar, the constraint set must equal the set of valid specializations; for an inferable typevar, it must not be
never.)When I first added support for tracking inferable vs non-inferable typevars, it seemed like it would be easiest to have separate
Typevariants for each. The alternative (which lines up with the Δ set in POPL15) would be to explicitly plumb through a list of inferable typevars through our type property methods. That seemed cumbersome.In retrospect, that was the wrong decision. We've had to jump through hoops to translate types between the inferable and non-inferable variants, which has been quite brittle. Combined with the original point above, that much of the assignability logic will become more identical between inferable and non-inferable, there is less justification for the two
Typevariants. And plumbing an extrainferableparameter through all of these methods turns out to not be as bad as I anticipated.