Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow calls ending with unions of tuples #55185

Closed
wants to merge 1 commit into from

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Jul 28, 2023

Previously, only calls with tuples were allowed, not unions of tuples. For example, this already works:

f(string, ...[number, string], string) ==> f(string, number, string, string)

But this does not:

f(string, ...([string] | [number, number])) ==> (f, string, string | number, number | undefined)

This PR allows union types like these as the last argument. It's possible to allow them anywhere, but quite a bit more complicated. And the code is already complicated enough: getEffectiveCallArguments now needs to return the minimum number of arguments as well as the argument list (which is the maximum number of arguments).

Also, this transformation should not happen when the tuple union argument is passed to a tuple union rest parameter: [number] | [string] is assignable to [number] | [string] but the transformed number | string is not. Checking for this requires passing around even more information.

Bottom line: I'm not sure the complexity is worth the new code. However, if this is a good idea, there are 3 things that need to be cleaned up:

  1. More precise error messages--maybe. Right now error messages are reported in terms of the maximum number of arguments:
  2. Allow tuples with trailing ...T types. Not done or tested, but straightfoward.
  3. Find a more elegant way to return the minimum number of arguments.

Fixes #42508
Supercedes #43882, but does less than half of that PR, in less than half the code.

Previously, only calls with tuples were allowed, not unions of
tuples. For example, this already works:

```
f(string, [number, string], string) ==> f(string, number, string, string)
```

But this does not:

```
f(string, [string] | [number, number]) ==> (f, string, string | number, number | undefined)
```

This PR allows union types like these *as the last argument*. It's
possible to allow them anywhere, but quite a bit more complicated. And
the code is already complicated enough: getEffectiveCallArguments now
needs to return the minimum number of arguments as well as the argument
list (which is the maximum number of arguments).

Also, this transformation should not happen when the tuple union
argument is passed to a tuple union rest parameter: `[number] |
[string]` is assignable to `[number] | [string]` but the transformed
`number | string` is not. Checking for this requires passing
around even more information.

Bottom line: I'm not sure the complexity is worth the new code.
However, if this is a good idea, there are 3 things that need to be cleaned up:

1. More precise error messages--maybe. Right now error messages are
reported in terms of the maximum number of arguments:
2. Allow tuples with trailing `...T` types. Not done or tested, but
straightfoward.
3. Find a more elegant way to return the minimum number of arguments.

Fixes microsoft#42508
Supercedes microsoft#43882, but does less than half of that PR, in less than half
the code.
@sandersn
Copy link
Member Author

After discussing this fix in the design meeting, we decided to not take this PR. It's too ad-hoc for the complexity of its code.

If/when we try to reform overload resolution's interaction with type parameter inference and contextual typing, we should address #42508 at the same time. Specifically, #42508 needs the ability to roll back inferences, same as overload type parameter inference. Then the fix here would be to make tuples generate multiple argument lists instead of trying to produce a single argument list. Each argument list can be applied against each overload (or a single signature if there's only one.)

@fatcerberus
Copy link

Each argument list can be applied against each overload (or a single signature if there's only one.)

Did somebody say "quadratic"? 🚎

@sandersn
Copy link
Member Author

sandersn commented Aug 1, 2023

Yes they did! We'll cross that bridge when we come to it. (One argument is: we check lots of other things that are quadratic because people ask us to, and expect them to change their types if compilation is too slow.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when spreading a union of tuples in a call
3 participants