Skip to content

Allow arrays of nullable reference types #1386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: draft-v8
Choose a base branch
from
Draft
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions standard/types.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,13 @@
;

array_type
: non_array_type rank_specifier+
: array_type nullable_type_annotation rank_specifier+
| non_array_type rank_specifier+
Comment on lines 56 to +58
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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! 😁

Comment on lines +57 to +58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, no MLR, well done ;-)

This change impacts §17.2 which now needs to be revised.

;

non_array_type
: value_type

Check warning on line 62 in standard/types.md

View workflow job for this annotation

GitHub Actions / Markdown to Word Converter

standard/types.md#L62

MDC032::Line length 106 > maximum 81
| class_type
| interface_type
| delegate_type
| 'dynamic'
| type_parameter
| (class_type | interface_type | delegate_type | 'dynamic' | type_parameter) nullable_type_annotation?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the issue where string?[] and other arrays of reference-typed non-arrays were disallowed by the grammar.

| pointer_type // unsafe code support
;

Expand Down Expand Up @@ -732,7 +729,25 @@

> *Note:* The types `R` and `R?` are represented by the same underlying type, `R`. A variable of that underlying type can either contain a reference to an object or be the value `null`, which indicates “no reference.” *end note*

The syntactic distinction between a *nullable reference type* and its corresponding *non-nullable reference type* enables a compiler to generate diagnostics. A compiler must allow the *nullable_type_annotation* as defined in [§8.2.1](types.md#821-general). The diagnostics must be limited to warnings. Neither the presence or absence of nullable annotations, nor the state of the nullable context can change the compile time or runtime behavior of a program except for changes in any diagnostic messages generated at compile time.
The syntactic distinction between a *nullable reference type* and its corresponding *non-nullable reference type* enables a compiler to generate diagnostics. A compiler must allow the *nullable_type_annotation* as defined in [§8.2.1](types.md#821-general). The diagnostics must be limited to warnings. Neither the presence or absence of nullable annotations, nor the state of the nullable context can change the compile time or runtime behavior of a program except for changes in any diagnostic messages generated at compile time, with one exception:

A compiler must respect the effect that *nullable_type_annotation* has on the ordering of array rank specifiers. Whereas `A[][,]` is a single *array_type* with two *rank_specifier*s, the presence of a nullable annotation between the rank specifiers in `A[]?[,]` causes it to no longer be a single *array_type*, but rather two: an outer *array_type* with a single *rank_specifier* of `[,]`, and an element type of `A[]?` which is a *nullable_reference_type* containing an inner *array_type* with a single *rank_specifier* of `[]`. Because of this, `A[]?[,]` and `A[,][]` are represented by the same underlying type, while `A[]?[,]` and `A[][,]` are not.
Comment on lines +740 to +742
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This is not the place to define the semantics of array types, that is §17.2
  • The final sentence doesn’t cover the consequences of having the same underlying, which presumably exist or the statement is pointless. Are there implications for array creation and conversions which will need to addressed in the respective clauses?
  • There are “tonal” issues here:
    • A implementation must support the syntax and semantics of the language – rather and “respect the effect of”
    • A token has no power per se, the grammar states where a token is allowed and the semantics specifies the meaning of the grammar's phrases/sentences – rather then “the presence of a … causes”
  • What does need to go here is a simple statement that nullable annotations can effect the semantics with a cross reference to where that effect is defined (probably just somewhere in §17 at present, but the future may bring others now the “no effect” status has gone)
    • This change impacts at least one other place in the Standard – see Nullability: conversion behavior #1242 – but there may be more…
    • I am assuming here that you do not intend to use the “conditionally normative” device to maintain the no effect status in the bulk of the Standard. If you do this last point & sub-point need to be considered in that light.


> *Example*: The array ranks are interrupted by the '?' in the parameter type, changing the meaning of the underlying array type:
>
> <!-- Example: {template:"code-in-class-lib", name:"ArraysOfNullAbleArrays"} -->
> ```csharp
> #nullable enable
> class C
> {
> void M(string[][,]?[,,][,,,] arrays)
> {
> string? value = arrays[3, 3, 3][4, 4, 4, 4]?[1][2, 2];
> }
> }
> ```
>
> *end example*
Comment on lines +743 to +758
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with the above this needs to be in §17 somewhere. Giving examples of how adding nullable annotations to examples already in §17 could help the understanding, e.g. In §17.2.1 there is:

Example: The type in T[][,,][,] is a single-dimensional array of three-dimensional arrays of two-dimensional arrays of int. end example

In these terms what is T[]?[,,][,], T[]?[,,][,] and maybe even T[]?[,,]?[,]?

An example demonstrating the differences, in the same terms as above, between T[][,][,,][,,,][,,,,][,,,,,] and T[][,]?[,,][,,,]?[,,,,][,,,,,] might be helpful (multiple annotations, multiple ranks between & around them)


### 8.9.2 Non-nullable reference types

Expand Down Expand Up @@ -946,13 +961,13 @@
> public void M(string s)
> {
> int length = s.Length; // No warning. s is not null
>

Check warning on line 964 in standard/types.md

View workflow job for this annotation

GitHub Actions / Markdown to Word Converter

standard/types.md#L964

MDC032::Line length 87 > maximum 81
> _ = s == null; // Null check by testing equality. The null state of s is maybe null
> length = s.Length; // Warning, and changes the null state of s to not null
>

Check warning on line 967 in standard/types.md

View workflow job for this annotation

GitHub Actions / Markdown to Word Converter

standard/types.md#L967

MDC032::Line length 90 > maximum 81
> _ = s?.Length; // The ?. is a null check and changes the null state of s to maybe null
> if (s.Length > 4) // Warning. Changes null state of s to not null
> {

Check warning on line 970 in standard/types.md

View workflow job for this annotation

GitHub Actions / Markdown to Word Converter

standard/types.md#L970

MDC032::Line length 87 > maximum 81
> _ = s?[4]; // ?[] is a null check and changes the null state of s to maybe null
> _ = s.Length; // Warning. s is maybe null
> }
Expand Down Expand Up @@ -1012,7 +1027,7 @@
> {
> var t = new Test();
> if (t.DisappearingProperty != null)
> {

Check warning on line 1030 in standard/types.md

View workflow job for this annotation

GitHub Actions / Markdown to Word Converter

standard/types.md#L1030

MDC032::Line length 110 > maximum 81
> int len = t.DisappearingProperty.Length; // No warning. A compiler can assume property is stateful
> }
> }
Expand Down
Loading