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

Collection literals: output type inferences #7293

Merged
merged 2 commits into from
Jun 23, 2023

Conversation

cston
Copy link
Member

@cston cston commented Jun 22, 2023

Add rules for collection literal expressions to input type inference and output type inference sections.

@@ -317,6 +317,13 @@ A *collection literal inference* is made *from* an expression `E` *to* a type `
- Otherwise, if `Eᵢ` has type `Uᵢ` then a *lower-bound inference* is made *from* `Uᵢ` *to* `Tₑ`.
- Otherwise, no inferences are made for `Eᵢ`.

> 11.6.3.7 Output type inferences
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the spec has it as 12.6.3.7 now. Don't know whether there's a need for us to keep in sync.

Copy link
Member Author

Choose a reason for hiding this comment

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

The links here are to standard-v6 currently. We could update all to v7 in the future and update the section numbers at the same time.

>
> An *output type inference* is made *from* an expression `E` *to* a type `T` in the following way:
>
> - **If `E` is a *collection literal* with elements `Eᵢ` and `T` is a [*collection type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) with an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) `Tₑ`, then an *output type inference* is made *from* each `Eᵢ` *to* `Tₑ`.**
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't understand why this part is necessary. I thought the language earlier in this section (L314-L318) covers the same thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe for stuff like M(([1], ["a"]))?

Copy link
Member Author

@cston cston Jun 23, 2023

Choose a reason for hiding this comment

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

The output type inference change is to walk into elements to infer from the output types of elements, such as the lambda return type in:

F([null, () => 1]);

static void F<T>(Func<T>[] arg) { }


> 11.6.3.2 The first phase
>
> For each of the method arguments `Eᵢ`:
> - An *input type inference* is made *from* `Eᵢ` *to* the corresponding *parameter type* `Tᵢ`.
Copy link
Member Author

@cston cston Jun 23, 2023

Choose a reason for hiding this comment

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

Extracted existing rules to an input type inference section based on suggestion from @MadsTorgersen.

@cston cston requested a review from 333fred June 23, 2023 06:03

> An *input type inference* is made *from* an expression `E` *to* a type `T` in the following way:
>
> - If `E` is a *collection literal* with elements `Eᵢ` and `T` is a [*collection type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) with an [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/statements.md#1295-the-foreach-statement) `Tₑ`, then an *input type inference* is made *from* each `Eᵢ` *to* `Tₑ`.
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little concerned about the interaction of this rule with tuples. I think what the LDM wants is multi-level walks here, where a tuple of collection literals would walk through multiple levels if T is such a type, but this wouldn't do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

The rule for collection literals and the rule for tuple literals both use this input type inference rule for their parts.

The [*type inference*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#1163-type-inference) rules are **updated** to include inferences from collection literal expressions.
The [*type inference*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#1163-type-inference) rules are updated as follows.

The existing rules for the [*first phase*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11632-the-first-phase) are extracted to a new *input type inference* section, and a rule is added to *input type inference* and *output type inference* for collection literal expressions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The existing rules for the [*first phase*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11632-the-first-phase) are extracted to a new *input type inference* section, and a rule is added to *input type inference* and *output type inference* for collection literal expressions.
The existing rules for the [*first phase*](https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#11632-the-first-phase) are extracted to a new *input type inference* section, and a rule is added to *input type inference* and *output type inference* for collection literal expressions.

@cston cston merged commit 188f47d into dotnet:main Jun 23, 2023
@cston cston deleted the collections-output-inferences branch June 23, 2023 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants