discards, deconstruction, and assignment

[BillWagner]

Fixes #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_elements, which can be either variable_references or discard_tokens.

[jskeet]

Looks good to me, thanks Bill!

[Nigel-Ecma]

Thanks for spotting my egg-on-face typos!

I think there are some changes required, partly due to confusion over the use of “matching”.

[Nigel-Ecma]

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”.

[BillWagner]

I accepted the suggestion, and then unresolved to make sure I remember to address the curious semantic here.

[Nigel-Ecma]

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”.

[Nigel-Ecma]

If you are reverting/rewriting following the preceding suggestions do the same here.

[jskeet]

(Process note: I'll review again after @BillWagner has responded to @Nigel-Ecma's comments.)

[jskeet]

Taking offline to try to get the changes requested resolved between meetings.

[BillWagner]

Small example to demonstrate the "curious semantic":

using System;
using System.Collections.Generic;
using System.Linq;

// Allowed: A deconstructing foreach can discard all iteration variables.
foreach ((_, _) in Pairs())
{
    Console.WriteLine("==");
}

// Error CS8186: A foreach must declare its iteration variables.
foreach (_ in Enumerable.Range(1,10))
{
    Console.WriteLine("=");
}

static IEnumerable<(int, int)> Pairs() => Enumerable.Range(1,10).Select(n => (n, n + n));

[BillWagner]

Small example to demonstrate the "curious semantic":

...

The decision is recorded in LDM notes here. The key quote:

We can also consider allowing top-level discards in foreach loops:

foreach (_ in e) { ... } // I don't care about the values

But that does not seem too important, especially since you can use var _ to the same effect.

I'll add a note to that effect.

[BillWagner]

@jskeet @Nigel-Ecma I apologize for missing the last set of comments before the meeting. I updated the text today. I also checked the LDM notes for the justification for why a deconstructing foreach can have 0 iteration variables, but a synchronous foreach isn't allowed to. I added a note to that effect.

[BillWagner]

The base branch draft-v8 has been updated following the latest TC49-TG2 committee meeting. Please rebase this PR onto the latest draft-v8 and resolve any conflicts. Thanks!

[jskeet]

@BillWagner: This is your PR; was the comment requesting a rebase an automated one?

[BillWagner]

This is your PR; was the comment requesting a rebase an automated one?

Yes. As part of the work that led to #1754 I required that the agents not rebase any PR that was assigned to a committee member. I didn't want it to shift history underneath someone actively working. Instead, it left a comment indicating that a rebase might be needed because the base branch changed. I discovered a number of new requirements this month because we merged both draft-v8 PRs and draft-v9 feature PRs. It was useful, and I'll respond to your comments on that PR later. (and schedule a walk through with you.)

For this one, I've made the changes we discussed at the meeting, and it's ready for a final approval from you and @Nigel-Ecma