EBNF for the `foreach_statement` is incorrect. (draft-v8)

[kaby76]

Describe the bug
I am a maintainer for https://github.com/antlr/grammars-v4 and I am in the process of bringing the grammar in grammars-v4 up to date for C#. I am also scraping and validating the grammars over here and in the published ECMA 334 .pdf.

The rule for the foreach_statement is incorrect, starting in version 7.

csharpstandard/standard/statements.md

Lines 1109 to 1112 in 0346abf

	foreach_statement
	: 'await'? 'foreach' '(' ref_kind? local_variable_type identifier
	'in' expression ')' embedded_statement
	;

Unless it really is not part of the spec, the rule should have an alternative for deconstruction_expression.

// Source: §13.9.5.1 General
foreach_statement
    : 'await'? 'foreach' '(' ref_kind? local_variable_type identifier
      'in' expression ')' embedded_statement
    | 'await'? 'foreach' '(' deconstruction_expression
      'in' expression ')' embedded_statement
    ;

Without the fix, the spec EBNF does not parse several input in the Roslyn v7 compiler itself. But, below is a simpler example that does not parse. (There are other places the Roslyn compiler diverges, so I don't know what is intended.)

Example

using System.Collections.Generic;

  class Program
  {
      static void Main()
      {
          var map = new Dictionary<string, int> { ["a"] = 1 };
          foreach (var (key, value) in map) { }
      }
  }

Attached is a zip file containing a full program with the example. It builds and runs with compiler version 7.

v7test.zip

Additional context
The current means of generating and testing the Antlr4 grammar is archaic. The "BuildGrammar" tool should generate a C#-target port, not a Java one, and definitely not using the ancient Antlr 4.9.2. There are better tools nowadays for analyzing the EBNF.

[Nigel-Ecma]

Thanks for this @kaby76.

There are quite a few issues with tuples and deconstruction in v7 and these are under active work and should all be fixed for the upcoming v8. The deconstructing foreach statement is not yet covered in any of the published work, but it should be in before v8 is done.

---

It is good to note that you agree ANTLR is archaic 😉.

When the grammar checker was being designed as part of the Github automation an ANTLR C# target was considered but issues with it saw a switch to Java, which is of course the original ANTLR target and supported well. By now testrig/grun is used internally as a key part of our toolchain across multiple platforms, this is of course a Java based tool and we’re not aware of any cross-platform alternative that does what we need for a C# target. This is one of the reasons we’re unlikely to switch to using a C# target in the foreseeable future, along with of course “if it ain’t broke don’t fix it”!