Fix tuple assignment and tuple element type inference#1359
Fix tuple assignment and tuple element type inference#1359jskeet merged 6 commits intodotnet:draft-v8from
Conversation
Fixes dotnet#1155 This PR follows the process outlined in dotnet#1155 (comment)
BillWagner
force-pushed
the
additional-tuple-work
branch
from
29fba23 to
79be266
Compare
Fixes dotnet#1244 I followed the recommended text from [collection expressions](https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/collection-expressions.md#type-inference) because we well need them in the future. I did break apart the first phase and the input type inference because tuple elements now needs subscripts as well. We could put them together, but the small `i` and `j` start to look the same.
BillWagner
changed the title
BillWagner
added
the
meeting: priority
|
@jskeet This is ready to review for the next meeting. |
Nigel-Ecma
left a comment
Nigel-Ecma
left a comment
There was a problem hiding this comment.
My possible fix to #1155 doesn’t deal with the case of a tuple_expression not having a type (RIP null type), and one without a type is not representable directly as a ValueTuple<…> as there is no type for one or more of the type params…
A re-think is needed, till then I’ll mark this PR as draft
standard/expressions.md
Outdated
| A tuple expression is evaluated by evaluating each of its element expressions in order from left to right. | ||
|
|
||
| A tuple value can be obtained from a tuple expression by converting it to a tuple type ([§10.2.13](conversions.md#10213-implicit-tuple-conversions)), by reclassifying it as a value ([§12.2.2](expressions.md#1222-values-of-expressions))) or by making it the target of a deconstructing assignment ([§12.21.2](expressions.md#12212-simple-assignment)). | ||
| A tuple value is obtained from a tuple expression by evaluating it and storing the result in corresponding `System.ValueTuple<...>` type, and initializing each of its fields in order from left to right by evaluating the corresponding tuple element expression of `E`, converting it to the corresponding element type of `T` using the implicit conversion found, and initializing the field with the result. |
There was a problem hiding this comment.
Unfortunately this isn’t right, see review comment
jskeet
left a comment
jskeet
left a comment
There was a problem hiding this comment.
Let's talk about this at the meeting. If we can come to an agreement about the second commit, would we be able to separate that out from the first and get it merged, to reduce the scope of outstanding changes?
Changes made during the July meeting. Co-authored-by: Jon Skeet <skeet@pobox.com>
|
@BillWagner will split into two PRs, I'll approve the one that is "2nd commit and onwards" and Bill can merge it. |
|
New task: Split into 2 PRs, one with the first commit, one with the 2nd and 3rd commits. Then, that 2nd PR can be merged. |
|
And @Nigel-Ecma will think about required changes for the first commit. |
This reverts commit 79be266.
|
@Nigel-Ecma @jskeet The first commit in this PR has been reverted. I've updated the description so this PR only fixes the one issue. The other commit has been broken off into #1366, which I assigned to Nigel to noodle on for the next meeting. We should be able to merge this one before the next meeting. |
jskeet
left a comment
jskeet
left a comment
There was a problem hiding this comment.
One more nit, but otherwise yay.
Co-authored-by: Jon Skeet <skeet@pobox.com>
|
@Nigel-Ecma Are you good with merging this now that the incorrect part is split off? If so, can you approve. |
Nigel-Ecma
dismissed
their stale review
The change request was for the part of the PR now split off as #1366 and is not relevant here anymore.
This replaces dotnet#1350. It's simpler due to dotnet#1359 centralizing the tie between an argument and the corresponding parameter's type. Fixes dotnet#981
This replaces dotnet#1350. It's simpler due to dotnet#1359 centralizing the tie between an argument and the corresponding parameter's type. Fixes dotnet#981
4 participants
Fixes #1244: type inference from tuple element expressions
I followed the recommended text from collection expressions because we well need them in the future. I did break apart the first phase and the input type inference because tuple elements now needs subscripts as well. We could put them together, but the small
iandjstart to look the same.