[ty] Retry parameter matching for argument type expansion #20153

dhruvmanila · 2025-08-29T14:12:27Z

Summary

This PR addresses an issue for a variadic argument when involved in argument type expansion of overload call evaluation.

The issue is that the expansion of the variadic argument could result in argument list of different arity. For example, in *args: tuple[int] | tuple[int, str], the expansion would lead to the variadic argument being unpacked into 1 and 2 element respectively. This means that the parameter matching that was performed initially isn't sufficient and each expanded argument list would need to redo the parameter matching again.

This is currently done by redoing the parameter matching directly, maintaining the state of argument forms (and the conflicting forms), and updating the Bindings values if it changes.

Closes: astral-sh/ty#735

Test Plan

Update existing mdtest.

github-actions · 2025-09-01T09:47:15Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

github-actions · 2025-09-01T09:50:31Z

`mypy_primer` results

No ecosystem changes detected ✅
No memory usage changes detected ✅

dhruvmanila · 2025-09-02T16:13:48Z

(This isn't completely ready but I'd appreciate some initial feedback on the approach.)

dcreager · 2025-09-03T02:08:40Z

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

+                            // If the argument corresponding to this type is variadic, we need to
+                            // update the tuple length because expanding could change the length.
+                            // For example, in `tuple[int] | tuple[int, int]`, the length of the
+                            // first type is 1, while the length of the second type is 2.
+                            if let Some(expanded_type) = new_expanded_types[index] {
+                                let length = expanded_type
+                                    .try_iterate(db)
+                                    .map(|tuple| tuple.len())
+                                    .unwrap_or(TupleLength::unknown());
+                                new_arguments[index] = Argument::Variadic(length);


👍 Yep this looks like a good approach! It should pull out the more precise length of each union element during expansion

dcreager · 2025-09-03T02:08:58Z

crates/ty_python_semantic/src/types/call/bind.rs

+#[derive(Default)]
+struct ArgumentForms {


I like the cleanup from introducing this new type, too

dcreager · 2025-09-03T02:09:45Z

crates/ty_python_semantic/src/types/call/bind.rs

+                // The spec mentions that each expanded argument list should re-evaluate from step
+                // 2 which is the type checking step but we're re-evaluating from step 1. The tldr
+                // is that it allows ty to match the correct overload in case a variadic argument
+                // would expand into different number of arguments with each expansion. Refer to
+                // https://github.com/astral-sh/ty/issues/735 for more details.
+                for overload in &mut self.overloads {
+                    // Clear the state of all overloads before re-evaluating from step 1
+                    overload.reset();
+                    overload.match_parameters(db, expanded_arguments, &mut argument_forms);
+                }


I don't know if we need to reflect this in our comments, but I think the spec is not wrong here -- the difference is that in the spec "step 1" means only "eliminate impossible overloads due to arity mismatch" whereas our step 1 is bigger than that, it also includes "match arguments to parameters", which in the spec is implicitly part of step 2, not part of step 1. This is why we need to "repeat step 1" and the spec does not. It may be clearer to say that we don't separate step 1 and 2 in the same way the spec does.

That's a good point, I didn't realize this subtle difference, I'll update the comment to reflect this.

dhruvmanila · 2025-09-04T07:47:54Z

openlibrary (https://github.com/internetarchive/openlibrary)
+ openlibrary/coverstore/archive.py:422:13: error[no-matching-overload] No overload of function `__new__` matches arguments
- Found 711 diagnostics
+ Found 712 diagnostics

I'm not exactly sure what's wrong here. I've created a playground snippet which breaks the union into it's elements which is what the type expansion would retry with (https://play.ty.dev/1990e010-195f-4800-9e31-b77894e37dd3). It's only the c variable which has a fixed length of 2 because the outermost type is a tuple containing two elements.

carljm · 2025-09-08T21:10:37Z

@dcreager I'm assuming this is in your review queue unless you request review from me.

dcreager

This is great, thank you for tackling this!

dcreager · 2025-09-11T13:50:06Z

crates/ty_python_semantic/src/types/call/bind.rs

-            let argument_types = match argument_types.as_ref() {
-                Tuple::Fixed(_) => {
-                    Cow::Owned(argument_types.concat(self.db, &Tuple::homogeneous(Type::unknown())))
-                }
-                Tuple::Variable(_) => argument_types,
-            };
-


❤️ Love that we can get rid of this workaround!

crates/ty_python_semantic/src/types/tuple.rs

crates/ty_python_semantic/src/types/call/bind.rs

dhruvmanila added the ty Multi-file analysis & type inference label Aug 29, 2025

dhruvmanila force-pushed the dhruv/retry-parameter-matching branch from eb554de to 29832ec Compare September 1, 2025 09:44

dhruvmanila force-pushed the dhruv/retry-parameter-matching branch 3 times, most recently from 52e6a51 to 648f6fa Compare September 2, 2025 13:47

dhruvmanila changed the title ~~[ty] [WIP] Retry parameter matching for argument type expansion~~ [ty] Retry parameter matching for argument type expansion Sep 2, 2025

dhruvmanila marked this pull request as ready for review September 2, 2025 16:13

dhruvmanila requested review from AlexWaygood, carljm, dcreager and sharkdp as code owners September 2, 2025 16:13

dcreager reviewed Sep 3, 2025

View reviewed changes

dhruvmanila requested a review from dcreager September 4, 2025 15:00

dhruvmanila added 3 commits September 8, 2025 11:53

[ty] Re-try argument matching for argument type expansion

19f1a44

Revert skipping step 5 filtering

71c92f0

Update comment per review

4184ce9

dhruvmanila force-pushed the dhruv/retry-parameter-matching branch from 63fe1a7 to 4184ce9 Compare September 8, 2025 06:24

carljm removed their request for review September 8, 2025 21:10

sharkdp removed their request for review September 10, 2025 07:01

dcreager approved these changes Sep 11, 2025

View reviewed changes

dhruvmanila added 3 commits September 12, 2025 14:01

Address review feedback

32f28f4

Merge branch 'main' into dhruv/retry-parameter-matching

6f02f99

Fix merge commit

c3550a1

dhruvmanila enabled auto-merge (squash) September 12, 2025 08:37

dhruvmanila merged commit bb9be26 into main Sep 12, 2025
37 checks passed

dhruvmanila deleted the dhruv/retry-parameter-matching branch September 12, 2025 08:40

		#[derive(Default)]
		struct ArgumentForms {

[ty] Retry parameter matching for argument type expansion #20153

[ty] Retry parameter matching for argument type expansion #20153

Uh oh!

Conversation

dhruvmanila commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

github-actions bot commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

dhruvmanila commented Sep 2, 2025

Uh oh!

dcreager Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

dcreager Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

dcreager Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

carljm Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila commented Sep 4, 2025

Uh oh!

carljm commented Sep 8, 2025

Uh oh!

dcreager left a comment

Choose a reason for hiding this comment

Uh oh!

dcreager Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dhruvmanila commented Aug 29, 2025 •

edited

Loading

github-actions bot commented Sep 1, 2025 •

edited

Loading

github-actions bot commented Sep 1, 2025 •

edited

Loading

`mypy_primer` results