[ty] Combine CallArguments and CallArgumentTypes

[dcreager]

We previously had separate CallArguments and CallArgumentTypes types in support of our two-phase call binding logic. CallArguments would store only the arity/kind of each argument (positional, keyword, variadic, etc). We then performed parameter matching using only this arity/kind information, and then infered the type of each argument, placing the result of this second phase into a new CallArgumentTypes.

In #18996, we will need to infer the types of splatted arguments before performing parameter matching, since we need to know the argument type to accurately infer its length, which informs how many parameters the splatted argument is matched against.

That makes this separation of Rust types no longer useful. This PR merges everything back into a single CallArguments. In the case where we are performing two-phase call binding, the types will be initialized to None, and updated to the actual argument type during the second check_types phase.

[This is a refactoring in support of fixing the merge conflicts on #18996. I've pulled this out into a separate PR to make it easier to review in isolation.]

[github-actions]

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

[carljm]

Haven't looked at the code here yet, but I'm curious if defaulting to Type::Unknown is the best option here, vs using an Option<Type<'db>> Rust type? It seems like we might at some point regret the semantic muddiness of the former, where in principle Type::Unknown could mean "we haven't tried to infer any type for this argument yet", or it could equally well mean "we inferred a type for this argument, and the type that we found is "Unknown".

[dcreager]

Haven't looked at the code here yet, but I'm curious if defaulting to Type::Unknown is the best option here, vs using an Option<Type<'db>> Rust type? It seems like we might at some point regret the semantic muddiness of the former, where in principle Type::Unknown could mean "we haven't tried to infer any type for this argument yet", or it could equally well mean "we inferred a type for this argument, and the type that we found is "Unknown".

Yeah I started that way but it was shaping up to be a much more invasive change, especially in the argument expansion code. The only thing we'd use that distinction for at the moment is to verify that we've already inferred the type of a splatted argument, like here.

Rather, I think a better approach in the spirit of your suggestion would be to keep separate CallArguments and CallArgumentTypes, but where the first has a Vec<Option<Type<'db>>> and the second has a Vec<Type<'db>>. (Maybe the types would deserve different names at that point? CallArgumentsBuilder and CallArguments?)

[dcreager]

Yeah I started that way but it was shaping up to be a much more invasive change, especially in the argument expansion code.

Tried it again and with some cleanup I had done in the argument expansion code it looks better on the second attempt! Done

[dhruvmanila]

Looks good, thank you!