Deconstruction assignment: basic implementation #11290
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tuple and underlying type unification. Part 1.
Tuple and underlying type unification. Part 2.
Tuple and underlying type unification. Part 3.
jcouv
force-pushed
the
tuple-deconstruction
branch
6 times, most recently
from
9528256
to
3d47b61
Compare
| new BoundExpression[] { BindValue(arguments[0].Expression, diagnostics, BindValueKind.Assignment) } : | ||
| SpecializedCollections.EmptyArray<BoundExpression>(); | ||
|
|
||
| return BadExpression(node, args); |
There was a problem hiding this comment.
You forgot to bind the right-hand-side here.
There was a problem hiding this comment.
I'll stop by to ask you more about this.
|
Btw, I started a new work items list for deconstructions: #11299. The known holes I'm tracking so far:
|
| /// Replaces substitution currently used by the rewriter for a placeholder node with a different substitution. | ||
| /// Asserts if there isn't already a substitution. | ||
| /// </summary> | ||
| private void UpdatePlaceholderReplacement(BoundValuePlaceholderBase placeholder, BoundExpression value) |
There was a problem hiding this comment.
Where is this used? It seems dangerous.
There was a problem hiding this comment.
The 'update' is not used yet. I'll remove.
|
🕐 |
|
I'm closing this PR and will create a new one for the features/tuples branch after I work out the parsing issue. |
Updating Features/tuples with latest from Future
jcouv
force-pushed
the
tuple-deconstruction
branch
from
3d47b61
to
33e78bd
Compare
jcouv
force-pushed
the
tuple-deconstruction
branch
from
26ca3ec
to
fa126d9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's a first basic implementation of deconstruction-assignment. I'm wondering if this should go into a feature branch instead as I only did limited IDE testing so far.
In terms of design:
BoundDeconstructionAssignmentOperator). This bound node contains the list of variables on the left (bound and checked), the receiver forDeconstruct, the symbol for theDeconstructmethod and information to complete the assignment of individual values (including conversions).-The
Deconstructmethod is resolved with restrictive rules at this point (we should find exactly one).Deconstructbelow),Deconstructwith those out parameters,Vlad, the initial approach that we discussed for binding was very difficult (significant refactoring on tricky code). So the bound node holds onto the assignment expressions for each variable, as opposed to a Conversion object. Not only was this much simpler and natural, I think this will actually help later (when dealing with nesting case).
@dotnet/roslyn-compiler for review.