Skip to content

Conversation

dcreager
Copy link
Member

This adds a new Type variant for holding an instance of a typevar inside of a generic function or class. We don't handle specializing the typevars yet, but this should implement most of the typing rules for inside the generic function/class, where we don't know yet which specific type the typevar will be specialized to.

This PR does not yet handle the constraint that multiple occurrences of the typevar must be specialized to the same time. (There is an existing test case for this in generics/functions.md which is still marked as TODO.)

@dcreager dcreager added the ty Multi-file analysis & type inference label Mar 31, 2025
Copy link
Contributor

github-actions bot commented Mar 31, 2025

mypy_primer results

No ecosystem changes detected ✅

@@ -550,7 +552,7 @@ impl<'db> Type<'db> {

match (self, target) {
// We should have handled these immediately above.
(Type::Dynamic(_), _) | (_, Type::Dynamic(_)) => {
(Type::Dynamic(_) | Type::TypeVar(_), _) | (_, Type::Dynamic(_) | Type::TypeVar(_)) => {
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 have implemented all of these Type functions with the interpretation that a typevar is not fully static, since it can be specialized to a dynamic type.

Copy link
Member Author

Choose a reason for hiding this comment

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

We are actually going to do a simple syntactic check to determine if a typevar is fully static. The typevar is not fully static iff it has a not-fully-static bound or constraint. This lines up nicely with the definition in the typing spec:

We will refer to types that do not contain a gradual form as a sub-part as fully static types.

Copy link
Member Author

@dcreager dcreager left a comment

Choose a reason for hiding this comment

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

Summarizing a discussion from Discord:

@@ -550,7 +552,7 @@ impl<'db> Type<'db> {

match (self, target) {
// We should have handled these immediately above.
(Type::Dynamic(_), _) | (_, Type::Dynamic(_)) => {
(Type::Dynamic(_) | Type::TypeVar(_), _) | (_, Type::Dynamic(_) | Type::TypeVar(_)) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

We are actually going to do a simple syntactic check to determine if a typevar is fully static. The typevar is not fully static iff it has a not-fully-static bound or constraint. This lines up nicely with the definition in the typing spec:

We will refer to types that do not contain a gradual form as a sub-part as fully static types.

@carljm
Copy link
Contributor

carljm commented Apr 1, 2025

(Currently planning to hold off on reviewing this until that fully-static change mentioned above is made, but please ping if there's anything it would be useful to get additional eyes on sooner.)

dcreager added 5 commits April 1, 2025 15:22
* main:
  [red-knot] Add property tests for callable types (#17006)
  [red-knot] Disjointness for callable types (#17094)
  [red-knot] Flatten `Type::Callable` into four `Type` variants (#17126)
  mdtest.py: do a full mdtest run immediately when the script is executed (#17128)
  [red-knot] Fix callable subtyping for standard parameters (#17125)
  [red-knot] Fix more `redundant-cast` false positives (#17119)
  Sync vendored typeshed stubs (#17106)
  [red-knot] support Any as a class in typeshed (#17107)
  Visit `Identifier` node as part of the `SourceOrderVisitor` (#17110)
  [red-knot] Don't infer Todo for quite so many tuple type expressions (#17116)
  CI: Run pre-commit on depot machine (#17120)
  Error instead of `panic!` when running Ruff from a deleted directory (#16903) (#17054)
  Control flow graph: setup (#17064)
  [red-knot] Playground improvements (#17109)
  [red-knot] IDE crate (#17045)
  Update dependency vite to v6.2.4 (#17104)
  [red-knot] Add redundant-cast error (#17100)
  [red-knot] Narrowing on `in tuple[...]` and `in str` (#17059)
@dcreager
Copy link
Member Author

dcreager commented Apr 1, 2025

(Currently planning to hold off on reviewing this until that fully-static change mentioned above is made, but please ping if there's anything it would be useful to get additional eyes on sooner.)

Fully-static change is up, merge conflicts resolved. Should be good for another round of review!

Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

This is fantastic! Great work. Sorry for all the comments 😆 I think almost all of them are fine to address just with TODO comments.

dcreager added 18 commits April 2, 2025 15:05
* origin/main: (35 commits)
  [red-knot] Callable types are disjoint from literals (#17160)
  [red-knot] Fix inference for `pow` between two literal integers (#17161)
  [red-knot] Add GitHub PR annotations when mdtests fail in CI (#17150)
  [red-knot] Fix equivalence of differently ordered unions that contain `Callable` types (#17145)
  [red-knot] Add initial set of tests for unreachable code (#17159)
  [`airflow`] Move `AIR302` to `AIR301` and `AIR303` to `AIR302` (#17151)
  ruff_db: simplify lifetimes on `DiagnosticDisplay`
  [red-knot] Detect division-by-zero in unions and intersections (#17157)
  [`airflow`] Add autofix infrastructure to `AIR302` name checks (#16965)
  [`flake8-bandit`] Mark `str` and `list[str]` literals as trusted input (`S603`) (#17136)
  [`airflow`] Add autofix for `AIR302` attribute checks (#16977)
  [`airflow`] Extend `AIR302` with additional symbols (#17085)
  [`airflow`] Move `AIR301` to `AIR002` (#16978)
  [`airflow`] Add autofix for `AIR302` method checks (#16976)
  ruff_db: switch diagnostic rendering over to `std::fmt::Display`
  [red-knot] Add 'Goto type definition' to the playground (#17055)
  red_knot_ide: update snapshots
  red_knot_python_semantic: remove comment about `TypeCheckDiagnostic`
  ruff_db: delete most of the old diagnostic code
  red_knot: use `Diagnostic` inside of red knot
  ...
…ager/typevar-type

* origin/dcreager/typevar-type:
  Update crates/red_knot_python_semantic/src/types.rs
* origin/main:
  [red-knot] Fix more [redundant-cast] false positives (#17170)
  [red-knot] Three-argument type-calls take 'str' as the first argument (#17168)
  Control flow: `return` and `raise` (#17121)
  Bump 0.11.3 (#17173)
  [red-knot] Improve `Debug` implementation for `semantic_index::SymbolTable` (#17172)
  [red-knot] Fix `str(…)` calls (#17163)
  [red-knot] visibility_constraint analysis for match cases (#17077)
  [red-knot] Fix playground crashes when diagnostics are stale (#17165)
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Awesome work. Ship it!

@dcreager dcreager merged commit 64e7e1a into main Apr 3, 2025
23 checks passed
@dcreager dcreager deleted the dcreager/typevar-type branch April 3, 2025 18:36
dcreager added a commit that referenced this pull request Apr 3, 2025
* origin/main:
  [red-knot] Add `Type::TypeVar` variant (#17102)
  [red-knot] update to latest Salsa with fixpoint caching fix (#17179)
  Upgrade to Rust 1.86 and bump MSRV to 1.84 (#17171)
  [red-knot] Avoid unresolved-reference in unreachable code (#17169)
  Fix relative import resolution in `site-packages` packages when the `site-packages` search path is a subdirectory of the first-party search path (#17178)
  [DO NOT LAND] bump Salsa version (#17176)
  [syntax-errors] Detect duplicate keys in `match` mapping patterns (#17129)
maxmynter pushed a commit to maxmynter/ruff that referenced this pull request Apr 3, 2025
This adds a new `Type` variant for holding an instance of a typevar
inside of a generic function or class. We don't handle specializing the
typevars yet, but this should implement most of the typing rules for
inside the generic function/class, where we don't know yet which
specific type the typevar will be specialized to.

This PR does _not_ yet handle the constraint that multiple occurrences
of the typevar must be specialized to the _same_ time. (There is an
existing test case for this in `generics/functions.md` which is still
marked as TODO.)

---------

Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Carl Meyer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ty Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants