Allow arrays of nullable reference types

[jnm2]

Proposed replacement for @Nigel-Ecma's #1287 and @gafter's #1297

Will fix #1385

I was trying not to end up creating new names like non_array_non_nullable_reference_type and non_array_nullable_reference_type which don't sound like core concepts we'd want to be referencing elsewhere. (Cf #1287)

TODO:

• Add samples for the grammer tester. (Getting help from @Nigel-Ecma offline. I would like to create tests that show that string?[] and int[]?[] were disallowed prior to the grammar changes in this PR.)

[jnm2]

This has the property that when parsing string?[][,]?[,,][,,,]? we end up with two array_type nodes: one being string?[][,]?[,,][,,,], and one being the inner string?[][,].

@Nigel-Ecma It's not mutual left recursion! 😁

[Nigel-Ecma]

Nothing wrong with being concise; however unfortunately some of the conciseness is in the wrong places, and there are concerns this isn’t complete coverage of the feature.

[jnm2]

I have hit my time limit for the week but will try next week to understand the tools to set up grammar tests and run them.

[Nigel-Ecma]

Unfortunately I think there is quite a way to go yet.

Overall reading this I get a sense that too much is being placed on the number of occurrences of array_type/the change in shape of the array – these are really illusory changes (whether we think they should be or not) due to the erasure of nullability – meaning is not changing, even though adding/removing nullable annotations has the very non-illusory impact of generating compile time errors!

Trying to think of an illustration the best I came up with is language translation:

The new road avoids the swamp.

In Irish is:

Seachnaíonn an bóthar nua an portach.

These have the same meaning even if the structure has changed – translate the Irish word-by-word and you get:

Avoids the road new the swamp.

Arrays of nullable arrays doesn’t change the meaning of the code, what it changes is how the code must be written – e.g. rearrange indices (avoids the road new the swamp) – to achieve the same meaning.

Is this PR getting this across? Should it?

[Nigel-Ecma]

With array types there is not in general a single “corresponding nullable type” (§8.9.1) – there are multiple, e.g.:

static void ArrayEquivalentTypes(int[][,]?[,,]?[,,,]?[,,,,] a1)
{
    int[,,][][,]?[,,,]?[,,,,] a2 = a1;

    int[,,,][,,][][,]?[,,,,] a3_1 = a1;
    int[,,,][,,][][,]?[,,,,] a3_2 = a2;

    int[,,,,][,,,][,,][][,] a4_1 = a1;
    int[,,,,][,,,][,,][][,] a4_2 = a2;
    int[,,,,][,,,][,,][][,] a4_3_1 = a3_1;
    int[,,,,][,,,][,,][][,] a4_3_2 = a3_2;

    int[,,][][,]?[,,,]?[,,,,] b3_1 = a3_1;
    int[,,][][,]?[,,,]?[,,,,] b3_2 = a3_2;
    int[,,][][,]?[,,,]?[,,,,] b4_1 = a4_1;
    int[,,][][,]?[,,,]?[,,,,] b4_2 = a4_2;
    int[,,][][,]?[,,,]?[,,,,] b4_3_1 = a4_3_1;
    int[,,][][,]?[,,,]?[,,,,] b4_3_2 = a4_3_2;

    int[,,,][,,][][,]?[,,,,] c4_1 = a4_1;
    int[,,,][,,][][,]?[,,,,] c4_2 = a4_2;
    int[,,,][,,][][,]?[,,,,] c4_3_1 = a4_3_1;
    int[,,,][,,][][,]?[,,,,] c4_3_2 = a4_3_2;

    // etc...
}

This bundle of joy ;-) has four array types that are semantically equivalent to each other, and that’s not the limit by a long shot.

It needs to be specified that any two types (which need not be distinct) selected from a semantically equivalent set are implicitly convertible to each other. (Depending on implementation some, but not all, of the conversions may elicit a nullable warning.)

And while we are here, consider:

static void ArrayDifferentTypesSameLiteral()
{
    int[]?[,] a1 =
        {
            {   new int[] { 1, 2 },
                new int[] { 3, 4 }
            }
        };
    int[,][] a2 =
        {
            {   new int[] { 1, 2 },
                new int[] { 3, 4 }
            }
        };

Different array types which are semantically equivalent can be init’ed using the same array literal. That will need to be specified.

[Nigel-Ecma]

I will surprise everybody ;-) and say “compiler” -> ”implementation”.

Not changing the compile time behavior is no longer correct, add/remove a nullable annotation on an array type without changing all associated index operations will produce compile-time errors.

I’m not sure you can say changing the number of array_type productions in the parse is an exception per se – the annotations do not change the described array shape in anyway (one might argue that the existing description of arrays is less than clear on the shape, if so adding nullable arrays is the time to fix that).

[jnm2]

I don't know about taking on the change of compiler -> implementation. "Compiler" is used consistently 31 times in the surrounding context, and the sentence you're commenting on precedes this PR; the term is only shown with a green background in the diff because I tacked some text on to the end of the line.

[jnm2]

Not changing the compile time behavior is no longer correct, add/remove a nullable annotation on an array type without changing all associated index operations will produce compile-time errors.

The "with one exception" part already addresses this, doesn't it? It is correct that the compile time behavior does not change, with that one exception of adding or removing a nullable annotation on an array type... right?

[jnm2]

I’m not sure you can say changing the number of array_type productions in the parse is an exception per se – the annotations do not change the described array shape in anyway (one might argue that the existing description of arrays is less than clear on the shape, if so adding nullable arrays is the time to fix that).

Here is why I think that it is an exception per se. The general rule is that the presence or absence of nullable annotations does not change the compile time or runtime behavior. Thus, the removal of ? that is present in a C# program would generally not affect compile time or runtime behavior. However, the removal of ? from T[]?[,] does affect compile time and runtime behavior. The causal mechanism by which the presence of the ? has meaning is the parse of the grammar; with the ?, the parse has two array_types, and without the ?, the parse has only one array_type. The meaning of the resulting compile time behavior and runtime type are defined in terms of the nested structure (if any) of those array_type(s) produced by the parse.

[BillWagner]

Overall, I like the approach of #1297 better. I find it easier to follow the concepts. (But that's not a strong preference if others prefer this direction).

[Nigel-Ecma]

@BillWagner – unfortunately #1297 has issues, this one starts with a better grammar which binds the nullable annotation the right way – arrays used to have elements of non-array type, now they can have elements of nullable array type (but still not array type), and the grammar models that. There is still some way to go here both on arrays themselves, conversions, and in removing the “nullable annotations do not effect semantics” guarantee (as @jnm2 does not wish to use the “conditionally normative” device as a way out of that). However its not worth pursuing #1297 as an alternative.

[jskeet]

Note to myself as much as anything else: I've read through the change, and don't feel competent to express an opinion on it.

[jskeet]

Applied the C# 8 milestone, but if it's not ready, we should at least consider #1287.