[Version 9.0] Feature support for record classes

[BillWagner]

All commits from #983 have been squashed and added to this PR.

This PR / feature makes numerous changes to the grammar. We still need corresponding updates to the test and validation suite.

The following notes are carried over for additional work needed:

A. I put the new subclause "With expression" prior to "Arithmetic operators", which once the V8 features "Indices and Ranges" and "Pattern matching" have been merged, should immediately follow "Range operator" and "Switch expression." Make sure these are all in the correct place. 12.4.2 Operator precedence and associativity will also need to be revised accordingly.

B. New subclause §rec-class-prtmem Printing members mentions a method System.Runtime.CompilerServices.RuntimeHelpers.EnsureSufficientExecutionStack(). It's quite possible that this is an implementation-specific detail, in which case, we'll need to find words to make it abstract, and possibly update the Portability annex accordingly.

[RexJaeschke]

We need to add "with" as a new row to the table in the (earlier) section "Operator precedence and associativity." And as we have historically organized the sections in this clause in descending precedence order, we might need to move this new section up or down, accordingly.

[RexJaeschke]

Do we really want to expose this type in the spec?

[RexJaeschke]

Based on Issue #1683, I have updated the grammar for class_declaration.

Re my choice of grammar rule name prefixes, “non-record” and “record,” for a reader starting at 15.1, which says nothing about records, you might think we should not use the word “record” here. I’m OK with that. As a strawman, how about we call the two class flavors “plain class” and “record class?” (I think “class” and “record class” is not a clear enough distinction, as “class” might mean any class or just a non-record class.)

I currently use the two-favor distinction in new definitions, as follows:

A non_record_class_declaration declares a non-record class, while a record_class_declaration declares a record class.

This might then become:

A plain_class_declaration declares a plain class, while a record_class_declaration declares a record class.

I’m thinking that all existing uses of “class” will apply to both flavors, so no distinction will be needed for them.

For now, I’ll use “non-record” and “record.”

[BillWagner]

An earlier version of this feature is already present on alpha-v9 from a prior meeting. Edits made to this PR since then are not yet on alpha-v9; they will land at the next propagation. If you need them on alpha-v9 sooner, please open a separate PR targeting alpha-v9.
Thanks!

[RexJaeschke]

After researching and writing-up "Primary constructors" as a general feature in v12 for classes and structs outside of record variants, I have made a lot of tweaks to this spec (and to the V10 record struct spec) to ease the transition to that v12 spec.

[jskeet]

Applied "meeting: discuss" so that we can all perform a first round of review before the next meeting.

[Nigel-Ecma]

This is a review looking at a high-level, the details semantics of records has not been reviewed.

Issue: The grammar changes break all the existing tests.

Often when adding new constructs to the language, records in this case, the existing tests all pass. In short this is down to the tests checking the shape of the tree, adding a new branch kind to a list of branch choices doesn’t change that (think adding a new statement kind to the list of statement kinds). However if where existing branches fit is changed then exisiting tests will fail. There is nothing wrong with this per se, but it should only be done if there is a good reason.

This proposal changes the location where classes fit into the overall grammar (by adding a branch node above them), there are a lot of classes in C# programs, and the result is all the tests fail. If there was a strong reason for this change then the expected parse for all tests could be updated (not hard, a little tedious).

However in this case the change is unnecessary:

• class_declaration & class_body can be restored to their previous form;
• record_class_body needs an optional ending semicolon, as it has in the source material;
• type_declaration needs record_class_body added; and
• non_record_class_declaration is consigned to the bin (yay)

class_declaration
    : attributes? class_modifier* 'partial'? 'class' identifier
        type_parameter_list? class_base? type_parameter_constraints_clause*
        class_body ';'?
    ;

class_body
    : '{' class_member_declaration* '}'
    ;

record_class_body
    : class_body ';'?
    | ';'
    ;

type_declaration
    : class_declaration
    | record_class_declaration
    | struct_declaration
    | interface_declaration
    | enum_declaration
    | delegate_declaration
    ;

All tests pass, including a new beta one for records (not yet released).

These changes do not fix all the concerns with the grammar for records, in particular there is unneeded sharing of rules – reduces the number of grammar rules but adds prose rules to split the overlaid rules up again (e.g. the source material has record_base and class_base, this proposal overlays them and adds prose rules), that’s not a win. (Sometimes overlaying makes sense, e.g. using class_body in record_body, no added prose rules required).

However these issues can be addressed in later reviews.

non-record class – no no no!

Sorry but this hurt my eyes and ears. Classes have been a central feature of C# of around a quarter of a century, changing them to be called non-record classes is a poor idea with no saving grace. Call them classes and remove any added non_record_class bits from rule names (note the grammar changes up consigned non_record_class_declaration to the bin).

I would also call records record, not record class, which wouldn’t prevent v10 adding record struct; it is just adding words for no real benefit.

The source material states:

Record types are reference types, similar to a class declaration.

Not that a record declaration is a class declaration, and uses rules such as record_declaration, record_base & record_body, in v10 it adds record_struct_X.

TL;DR

• Restore the grammar rules listed at the top – don’t break things unnecessarily. And it “fixes” all the tests 🙂.
• No “non-record class”, its a “class” with a quarter century of use.
• Recommend dropping non_record_ prefix in grammar rule names (sometimes the source material gets it right 😉)

TTFN