[red-knot] Super-basic generic inference at call sites #17301

dcreager · 2025-04-08T21:00:29Z

This PR adds very basic inference of generic typevars at call sites. It does not bring in a full unification algorithm, and there are a few TODOs in the test suite that are not discharged by this. But it handles a good number of useful cases! And the PR does not add anything that would go away with a more sophisticated constraint solver.

In short, we just look for typevars in the formal parameters, and assume that the inferred type of the corresponding argument is what that typevar should map to. If a typevar appears more than once, we union together the corresponding argument types.

Cases we are not yet handling:

We are not widening literals.
We are not recursing into parameters that are themselves generic aliases.
We are not being very clever with parameters that are union types.

This depends on #17023.

* 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)

Co-authored-by: Alex Waygood <[email protected]>

Co-authored-by: Carl Meyer <[email protected]>

* main: [red-knot] Specialize `str.startswith` for string literals (#17351) [syntax-errors] `yield`, `yield from`, and `await` outside functions (#17298) [red-knot] Refresh diagnostics when changing related files (#17350) Add `Checker::import_from_typing` (#17340) Don't add chaperone space after escaped quote in triple quote (#17216) [red-knot] Silence errors in unreachable type annotations / class bases (#17342)

dcreager

@carljm, the "identity specialization" worked well, and let me remove the new InstanceType enum variant as we discussed.

dcreager · 2025-04-11T15:12:40Z

crates/red_knot_python_semantic/resources/mdtest/generics/functions.md

    return x

-# TODO: no error
 # TODO: revealed: str


What I was thinking is that when first parameter passed ("a"), then T typevar locks to Literal["a"] and by second parameter which is Literal["b"] it will invalidate the T.

This is a good way to think about how the implementation works here, and how it will need to grow to support some of the other mdtests that still have TODOs.

Right now, the only "solving" that we do is to see that the param is a typevar, and the argument is "some type", and add to the pending specialization that the typevar maps to the type. But if we've already seen that typevar in a different parameter, instead of replacing the previous type, or requiring the previous and new types to be the same (as you thought might be the case), we merge them together into a union. This is the only "unification" that the implementation in this PR does.

This first example in this section is one that this PR doesn't addres, where we'll need to not just blindly union everything together — the type annotation is meant to be an actual restriction that we should enforce.

dcreager · 2025-04-11T15:14:37Z

crates/red_knot_python_semantic/src/types.rs

-            .member_lookup_with_policy(
-                db,
-                name.into(),
-                MemberLookupPolicy::NO_INSTANCE_FALLBACK | policy,


Callers must now provide the full policy that they want to use, and NO_INSTANCE_FALLBACK is not implicitly added. That allows us to use try_call_dunder_with_policy (instead of copy/pasting it) down below in try_call_constructor, to find and call the __new__ class method.

dcreager · 2025-04-11T15:15:00Z

crates/red_knot_python_semantic/src/types.rs

-        mut argument_types: CallArgumentTypes<'_, 'db>,
+        argument_types: &mut CallArgumentTypes<'_, 'db>,


Taking in a mut reference here let's us use with_self down below, which reuses and modifies a CallArgumentTypes in place.

dcreager · 2025-04-11T15:16:04Z

crates/red_knot_python_semantic/src/types.rs

                MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
                    | MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,


This the lookup policy that does not include NO_INSTANCE_FALLBACK, where we can now use try_call_dunder_with_policy because of the change described above

crates/red_knot_python_semantic/src/types.rs

dcreager · 2025-04-11T15:27:15Z

crates/red_knot_python_semantic/src/types/call/arguments.rs


-    /// Create a new [`CallArgumentTypes`] by prepending a synthetic argument to the front of this
-    /// argument list.
-    pub(crate) fn prepend_synthetic(&self, synthetic: Type<'db>) -> Self {


This is redundant with with_self, as long as you can reuse the CallArgumentTypes instance

crates/red_knot_python_semantic/src/types/class.rs

dcreager · 2025-04-11T15:29:10Z

crates/red_knot_python_semantic/src/types/display.rs

+    pub fn display(&'db self, db: &'db dyn Db) -> DisplayGenericContext<'db> {
+        DisplayGenericContext {
+            typevars: self.variables(db),
+            db,
+        }
+    }


This isn't actually used in the PR, but I was using it for some printf debugging at one point, and it seemed worth keeping.

dcreager · 2025-04-11T15:30:11Z

crates/red_knot_python_semantic/src/types/generics.rs

+        let builder = self
+            .types
+            .entry(typevar)
+            .or_insert_with(|| UnionBuilder::new(self.db));
+        builder.add_in_place(ty);


Here is where (as discussed above) we are unioning together the mappings for a typevar that appears in multiple parameters

carljm

Fantastic.

carljm · 2025-04-12T00:31:46Z

crates/red_knot_python_semantic/resources/mdtest/generics/functions.md

    return x

-# TODO: no error
 # TODO: revealed: str


That example will also require type context (bidirectional checking).

crates/red_knot_python_semantic/resources/mdtest/generics/functions.md

carljm · 2025-04-12T00:57:32Z

crates/red_knot_python_semantic/resources/mdtest/generics/functions.md

    return x

-# TODO: no error
 # TODO: revealed: str


I know I said in the first comment here that I wasn't actually saying we should remove all these TODOs asking for wider types... but I kind of think we should? Unless someone wants to argue that we definitely want this change. Having TODOs around suggests to a contributor that a PR to widen all these types would be welcomed, when I'm not sure it would/should be.

crates/red_knot_python_semantic/src/types.rs

carljm · 2025-04-12T01:27:14Z

crates/red_knot_python_semantic/src/types/call.rs

 mod bind;
 pub(super) use arguments::{Argument, CallArgumentTypes, CallArguments};
-pub(super) use bind::Bindings;
+pub(super) use bind::{Bindings, CallableBinding};


This doesn't seem to be used in this module?

It's reexported here so that this line can pull it in from the call module instead of from call::bind. I think that's so that we can use pub(super) here to limit it to the types::* module tree, and not let it leak to the entire crate? (We'd need a pub(grandparent) to do that on its definition in bind.)

I honestly don't love that pattern. (Do we really need that level of control over visibility? And if so, would it be better to use more crates to achieve it? If we don't want more crates, is that not an argument that we don't really need that level of control?) But I don't feel that strongly, and it's the pattern currently being used, so I didn't want to conflate an extra thing into this PR.

crates/red_knot_python_semantic/src/types/class.rs

carljm · 2025-04-12T01:38:04Z

crates/red_knot_python_semantic/src/types/class.rs

+                    Type::FunctionLiteral(function),
+                    "__new__" | "__init__",
+                ) => Type::FunctionLiteral(
+                    function.with_generic_context(db, alias.origin(db).generic_context(db)),


I'm a bit confused by this. So we still have the specialized function, and we don't remove that specialization, but we attach the class' generic context to the function? What effect does having the specialization still attached have?

I guess I'm having some trouble following the semantics of the "generic context" field on a function type; maybe this is also related to your comment just above. I wonder if it would be clearer to have a separate dedicated field for this special case of "already-specialized method, but we want call binding to infer a specialization from it"?

I guess a maybe related question is, what about if the class is explicitly specialized already (e.g. C[int](...) constructor call)? In that case, it seems like we don't want/need to infer a specialization from the constructor, we just want to use the existing specialization and check the arguments against it. How is that handled currently?

I'm a bit confused by this. So we still have the specialized function, and we don't remove that specialization, but we attach the class' generic context to the function? What effect does having the specialization still attached have?

This is related to your question below: we shouldn't be doing this special-case lookup here at all, it should only happen down in ClassLiteralType, where neither the generic class nor its __new__ method have been specialized yet. So I've removed this.

I think the equivalent logic in ClassLiteralType (which we keep) is correct, at least for __new__ methods that are not further specialized. (For ones that are, we run afoul of my comment above about not handling that nested generic context especially well yet.) You would have:

class C[T]: def __new__(cls, x: T) -> "C"[T]: ...

In ClassLiteralType, we would looking up C.__new__. C is not yet specialized, so the method inherits the class's generic context and (via the with_generic_context call) becomes C.__new__[T]. Then when we call it, a specialization gets inferred, and that specialization gets applied to the class as well, resulting in e.g. C.__new__[int] → C[int].

carljm · 2025-04-12T01:47:59Z

crates/red_knot_python_semantic/src/types/class.rs

-        class_literal
-            .own_class_member(db, name)
-            .map_type(|ty| self.specialize_type(db, ty))
+        class_literal.own_class_member(db, name).map_type(|ty| {


Why do we have to implement the special case for __init__ and __new__ both here and in ClassLiteralType::own_class_member, when this method delegates to ClassLiteralType::own_class_member?

See above; we shouldn't have this logic in both places, only in ClassLiteralType, where the generic class has not yet been specialized

dcreager · 2025-04-15T20:55:25Z

Still working on more thorough tests of __new__ and __init__. Will move it back out of draft when ready for more review

* main: (44 commits) [`airflow`] Extend `AIR311` rules (#17422) [red-knot] simplify union size limit handling (#17429) [`airflow`] Extract `AIR311` from `AIR301` rules (`AIR301`, `AIR311`) (#17310) [red-knot] set a size limit on unions of literals (#17419) [red-knot] make large-union benchmark slow again (#17418) [red-knot] optimize building large unions of literals (#17403) [red-knot] Fix comments in type_api.md (#17425) [red-knot] Do not assume that `x != 0` if `x` inhabits `~Literal[0]` (#17370) [red-knot] make large-union benchmark more challenging (#17416) [red-knot] Acknowledge that `T & anything` is assignable to `T` (#17413) Update Rust crate clap to v4.5.36 (#17381) Raise syntax error when `\` is at end of file (#17409) [red-knot] Add regression tests for narrowing constraints cycles (#17408) [red-knot] Add some knowledge of `__all__` to `*`-import machinery (#17373) Update taiki-e/install-action digest to be7c31b (#17379) Update Rust crate mimalloc to v0.1.46 (#17382) Update PyO3/maturin-action action to v1.49.1 (#17384) Update Rust crate anyhow to v1.0.98 (#17380) dependencies: switch from `chrono` to `jiff` Update Rust crate bstr to v1.12.0 (#17385) ...

…nto dcreager/infer-function-calls * origin/dcreager/infer-function-calls: Real function body Add comment about potential class method inference Add TODO about nested contexts Remove TODOs

* main: [red-knot] Support `super` (#17174)

dcreager · 2025-04-16T19:07:20Z

Going to go ahead and merge this; if anyone sees anything that needs to be addressed as follow-on work, please let me know!

* main: (123 commits) [red-knot] Handle explicit class specialization in type expressions (#17434) [red-knot] allow assignment expression in call compare narrowing (#17461) [red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy (#17451) [red-knot] Type narrowing for assertions (take 2) (#17345) [red-knot] class bases are not affected by __future__.annotations (#17456) [red-knot] Add support for overloaded functions (#17366) [`pyupgrade`] Add fix safety section to docs (`UP036`) (#17444) [red-knot] more type-narrowing in match statements (#17302) [red-knot] Add some narrowing for assignment expressions (#17448) [red-knot] Understand `typing.Protocol` and `typing_extensions.Protocol` as equivalent (#17446) Server: Use `min` instead of `max` to limit the number of threads (#17421) [red-knot] Detect version-related syntax errors (#16379) [`pyflakes`] Add fix safety section (`F841`) (#17410) [red-knot] Add `KnownFunction` variants for `is_protocol`, `get_protocol_members` and `runtime_checkable` (#17450) Bump 0.11.6 (#17449) Auto generate `visit_source_order` (#17180) [red-knot] Initial tests for protocols (#17436) [red-knot] Dataclasses: synthesize `__init__` with proper signature (#17428) [red-knot] Dataclasses: support `order=True` (#17406) [red-knot] Super-basic generic inference at call sites (#17301) ...

We are currently representing type variables using a `KnownInstance` variant, which wraps a `TypeVarInstance` that contains the information about the typevar (name, bounds, constraints, default type). We were previously only constructing that type for PEP 695 typevars. This PR constructs that type for legacy typevars as well. It also detects functions that are generic because they use legacy typevars in their parameter list. With the existing logic for inferring specializations of function calls (#17301), that means that we are correctly detecting that the definition of `reveal_type` in the typeshed is generic, and inferring the correct specialization of `_T` for each call site. This does not yet handle legacy generic classes; that will come in a follow-on PR.

dcreager and others added 30 commits March 30, 2025 12:00

Build context for generic classes

3be8257

Handle explicit specialization before outputting lints

4c76cb5

Explicitly specialize classes

8048604

Track those structs

fe76d56

Add Specialization

4346e7d

Add CallableType::Specialized

2380ade

Use union to hold typevar constraints

178ee89

Add Type::TypeVar variant

feb1ea9

Fix failing tests

3c1ad79

doc Type::TypeVar

59430b6

More correct handling of final bounds/constraints

77ffa80

use list[T] so generic funcs are callable even with Never

6f86720

lint

cf81967

Add (currently failing) narrowing tests

6615df1

Typevars _can_ be fully static I guess

b2f5a2a

Simplify intersections with constrained typevars

590680c

Fix tests

debd60a

lint

aa391fd

Update crates/red_knot_python_semantic/src/types/type_ordering.rs

e57e62e

Co-authored-by: Alex Waygood <[email protected]>

Clarify that typevar is subtype of object too

3df79cc

Clarify non-fully-static bounded typevars aren't subtypes

5b08e93

Add more tests for constrained gradual typevars

82e810f

Update crates/red_knot_python_semantic/src/types.rs

a3d7253

Co-authored-by: Carl Meyer <[email protected]>

Simplify intersections with constrained typevars w/o glossing into union

15682d5

Simplify positive intersections too

9e07efe

Intersection of constraints is subtype of typevar

fb63c22

Better descriptions of intersections of constrained typevars

3459056

Add multiple narrowing example

71d425e

lint

233e938

dcreager added 6 commits April 10, 2025 21:09

Use identity specialization for new/init

4bd8128

Revert new instance variant

3734599

Remove unneeded helper method

836d02c

Add comment explaining the identity specialization

0744540

Revert unneeded policy change

d3067b0

dcreager commented Apr 11, 2025

View reviewed changes

dcreager marked this pull request as ready for review April 11, 2025 15:31

carljm approved these changes Apr 12, 2025

View reviewed changes

dcreager added 4 commits April 15, 2025 16:21

Remove TODOs

4823f43

Add TODO about nested contexts

87f3f62

Add comment about potential class method inference

beed962

Real function body

94bf3f6

dcreager marked this pull request as draft April 15, 2025 20:55

dcreager added 4 commits April 16, 2025 11:05

Remove unneeded special case lookup logic

8f43e30

Add tests for new and init

f38b43e

Merge remote-tracking branch 'origin/dcreager/infer-function-calls' i…

b852485

…nto dcreager/infer-function-calls * origin/dcreager/infer-function-calls: Real function body Add comment about potential class method inference Add TODO about nested contexts Remove TODOs

dcreager marked this pull request as ready for review April 16, 2025 17:04

dcreager added 3 commits April 16, 2025 13:10

Add failing test case for generic __init__

394c762

Merge branch 'main' into dcreager/infer-function-calls

347547b

* main: [red-knot] Support `super` (#17174)

clean up known class detection

523ddcb

dcreager merged commit 914095d into main Apr 16, 2025
23 checks passed

dcreager deleted the dcreager/infer-function-calls branch April 16, 2025 19:07

dcreager mentioned this pull request Apr 22, 2025

Create TypeVarInstance type for legacy typevars #16538

Merged

mtshiba mentioned this pull request Apr 28, 2025

Return type inference astral-sh/ty#128

Open

		mut argument_types: CallArgumentTypes<'_, 'db>,
		argument_types: &mut CallArgumentTypes<'_, 'db>,

		MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
		\| MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,

[red-knot] Super-basic generic inference at call sites #17301

[red-knot] Super-basic generic inference at call sites #17301

Uh oh!

Conversation

dcreager commented Apr 8, 2025

Uh oh!

dcreager left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carljm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcreager Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcreager commented Apr 15, 2025

Uh oh!

dcreager commented Apr 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dcreager Apr 16, 2025 •

edited

Loading