discards, deconstruction, and assignment#1730
Open
BillWagner wants to merge 1 commit into
Open
Conversation
Fixes dotnet#1630 Modify the language for deconstructing assignment (and the related `foreach` expansion to decouple the number of variables assigned from the classification as a deconstruction. It should be the presence of *deconstructor_element*s, which can be either *variable_reference*s or *discard_token*s.
BillWagner
added
the
meeting: priority
jskeet
approved these changes
Jul 14, 2026
jskeet
left a comment
jskeet
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me, thanks Bill!
Nigel-Ecma
requested changes
Jul 15, 2026
Nigel-Ecma
left a comment
Nigel-Ecma
left a comment
Contributor
There was a problem hiding this comment.
Thanks for spotting my egg-on-face typos!
I think there are some changes required, partly due to confusion over the use of “matching”.
| > *Note*: ANTLR grammar semantics enforce this requirement due to the ordering of the alternatives. *Semantically* there is no overlap between the four alternatives, this is a syntactic disambiguation. | ||
|
|
||
| The *simple_assignment* and *compound_assignment* expressions assign a new value to a variable, a property, or an indexer element. Event assignment ([§12.23.6](expressions.md#12236-event-assignment)), a subset of *compound_assignment*, assigns a new value to an event. The *ref_assignment* expression assigns a variable reference ([§9.5](variables.md#95-variable-references)) to a reference variable ([§9.7](variables.md#97-reference-variables-and-returns)). The *deconstructing_assignment* assigns values to two or more targets. | ||
| The *simple_assignment* and *compound_assignment* expressions assign a new value to a variable, a property, or an indexer element. Event assignment ([§12.23.6](expressions.md#12236-event-assignment)), a subset of *compound_assignment*, assigns a new value to an event. The *ref_assignment* expression assigns a variable reference ([§9.5](variables.md#95-variable-references)) to a reference variable ([§9.7](variables.md#97-reference-variables-and-returns)). The *deconstructing_assignment* assigns values to the non-discard targets of a *deconstructor*, which has two or more *deconstructor_element*s. |
Contributor
There was a problem hiding this comment.
Deconstructors are syntactically constrained to have 2 or more elements, the text here “deconstructor, which has two or more” could be read as allowing for <2 elements. I suggest just dropping the words:
Suggested change
| The *simple_assignment* and *compound_assignment* expressions assign a new value to a variable, a property, or an indexer element. Event assignment ([§12.23.6](expressions.md#12236-event-assignment)), a subset of *compound_assignment*, assigns a new value to an event. The *ref_assignment* expression assigns a variable reference ([§9.5](variables.md#95-variable-references)) to a reference variable ([§9.7](variables.md#97-reference-variables-and-returns)). The *deconstructing_assignment* assigns values to the non-discard targets of a *deconstructor*, which has two or more *deconstructor_element*s. | |
| The *simple_assignment* and *compound_assignment* expressions assign a new value to a variable, a property, or an indexer element. Event assignment ([§12.23.6](expressions.md#12236-event-assignment)), a subset of *compound_assignment*, assigns a new value to an event. The *ref_assignment* expression assigns a variable reference ([§9.5](variables.md#95-variable-references)) to a reference variable ([§9.7](variables.md#97-reference-variables-and-returns)). The *deconstructing_assignment* assigns values to the non-discard targets of a *deconstructor*. |
| #### 13.9.5.4 Deconstructing foreach | ||
|
|
||
| A deconstructing foreach replaces the declaration and initialisation of a single iteration variable per iteration, in the synchronous and asynchronous foreach statements, with a collection of zero or more iteration variables declared per iteration within the *deconstructor*([§12.23.3](expressions.md#12233-deconstructing-assignment)) of a *deconstructing_assignment*. | ||
| A deconstructing foreach replaces the declaration and initialisation of a single iteration variable per iteration, in the synchronous and asynchronous foreach statements, with a *deconstructor* ([§12.23.3](expressions.md#12233-deconstructing-assignment)) whose *deconstructor_element*s are matched, per iteration, against the elements obtained by deconstructing each collection element. |
Contributor
There was a problem hiding this comment.
In C# (but not all languages) deconstruction is distinct from pattern matching. The former selects values from a compound value and cannot fail, the latter matches against a value and can fail.
So this change as written is wrong, there is no matching involved. This either needs rewriting or simply reverting:
Suggested change
| A deconstructing foreach replaces the declaration and initialisation of a single iteration variable per iteration, in the synchronous and asynchronous foreach statements, with a *deconstructor* ([§12.23.3](expressions.md#12233-deconstructing-assignment)) whose *deconstructor_element*s are matched, per iteration, against the elements obtained by deconstructing each collection element. | |
| A deconstructing foreach replaces the declaration and initialisation of a single iteration variable per iteration, in the synchronous and asynchronous foreach statements, with a collection of zero or more iteration variables declared per iteration within the *deconstructor*([§12.23.3](expressions.md#12233-deconstructing-assignment)) of a *deconstructing_assignment*. |
The “curious” semantic here is that “zero or more” iteration variables are allowed. Given the semantics are partly reconstructed based on an implementation it should be determined whether this is an “accident of implementation” and the number should really be “one or more”.
| A deconstructing foreach replaces the declaration and initialisation of a single iteration variable per iteration, in the synchronous and asynchronous foreach statements, with a *deconstructor* ([§12.23.3](expressions.md#12233-deconstructing-assignment)) whose *deconstructor_element*s are matched, per iteration, against the elements obtained by deconstructing each collection element. | ||
|
|
||
| All variables assigned to by the *deconstructor* must be declared within the *deconstructor*, it is a compile time error for any *deconstructor_element* to be a *variable_reference*. | ||
| Each *deconstructor_element* shall be either a *declaration_expression*, which declares an iteration variable, or a discard, which is a placeholder that matches its corresponding element without declaring a variable. It is a compile-time error for any *deconstructor_element* to be a *variable_reference*. |
Contributor
There was a problem hiding this comment.
See previous comment on line 1444, there is no matching. Same action required: either reword or revert:
Suggested change
| Each *deconstructor_element* shall be either a *declaration_expression*, which declares an iteration variable, or a discard, which is a placeholder that matches its corresponding element without declaring a variable. It is a compile-time error for any *deconstructor_element* to be a *variable_reference*. | |
| All variables assigned to by the *deconstructor* must be declared within the *deconstructor*, it is a compile time error for any *deconstructor_element* to be a *variable_reference*.``` |
| ``` | ||
|
|
||
| This follows the behavior of synchronous foreach ([§13.9.5.2](statements.md#13952-synchronous-foreach)), differing by replacing the delaration and initialisation of a single iteration variable with a *deconstructing_assignment* which declares and assigns zero or more initialisation variables: | ||
| This follows the behavior of synchronous foreach ([§13.9.5.2](statements.md#13952-synchronous-foreach)), differing by replacing the declaration and initialisation of a single iteration variable with a *deconstructing_assignment* whose *deconstructor* declares an iteration variable for each *declaration_expression* it contains (discards declare none): |
Contributor
There was a problem hiding this comment.
This change doesn’t read well to me – it is saying each *declaration_expression" declares a variable…
I suggest either rewriting or reverting as for the above cases, but do not forget the “curious” semantic mentioned for 1444 above and maybe “revert” to “one or more”.
|
|
||
| - `C` and `E` are determined as for synchronous foreach | ||
| - `e` is not visible or accessible anywhere in the program accept as indicated in the above code | ||
| - `e` is not visible or accessible anywhere in the program except as indicated in the above code |
Contributor
There was a problem hiding this comment.
Egg-on-face
| ``` | ||
|
|
||
| This follows the behavior of asynchronous foreach ([§13.9.5.3](statements.md#13953-asynchronous-foreach)), differing by replacing the delaration and initialisation of a single iteration variable with a *deconstructing_assignment* which declares and assigns zero or more initialisation variables: | ||
| This follows the behavior of asynchronous foreach ([§13.9.5.3](statements.md#13953-asynchronous-foreach)), differing by replacing the declaration and initialisation of a single iteration variable with a *deconstructing_assignment* whose *deconstructor* declares an iteration variable for each *declaration_expression* it contains (discards declare none): |
Contributor
There was a problem hiding this comment.
If you are reverting/rewriting following the preceding suggestions do the same here.
| This follows the behavior of asynchronous foreach ([§13.9.5.3](statements.md#13953-asynchronous-foreach)), differing by replacing the declaration and initialisation of a single iteration variable with a *deconstructing_assignment* whose *deconstructor* declares an iteration variable for each *declaration_expression* it contains (discards declare none): | ||
|
|
||
| - `enumerator` is not visible or accessible anywhere in the program accept as indicated in the above code | ||
| - `enumerator` is not visible or accessible anywhere in the program except as indicated in the above code |
Contributor
There was a problem hiding this comment.
Not only did I probably author this on 1477 I copied & pasted it! Jeepers 😫.
Contributor
|
(Process note: I'll review again after @BillWagner has responded to @Nigel-Ecma's comments.) |
Contributor
|
Taking offline to try to get the changes requested resolved between meetings. |
jskeet
removed
the
meeting: priority
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes #1630
Modify the language for deconstructing assignment (and the related
foreachexpansion to decouple the number of variables assigned from the classification as a deconstruction. It should be the presence of deconstructor_elements, which can be either variable_references or discard_tokens.