Skip to content

Draft grammar to permit nullable types as array element types.#1297

Closed
gafter wants to merge 4 commits into
dotnet:draft-v8from
gafter:neal_arrays_of_nullable
Closed

Draft grammar to permit nullable types as array element types.#1297
gafter wants to merge 4 commits into
dotnet:draft-v8from
gafter:neal_arrays_of_nullable

Conversation

@gafter

@gafter gafter commented Mar 19, 2025

Copy link
Copy Markdown
Member

Proposed replacement for #1287

@Nigel-Ecma Can you please help me set up the testing for this PR?

@gafter
gafter marked this pull request as draft March 19, 2025 22:28
@gafter
gafter requested a review from Nigel-Ecma March 19, 2025 22:58
@gafter gafter added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Mar 19, 2025
Comment thread standard/types.md
array_type
: non_array_type rank_specifier+
| non_array_type rank_specifier+
| non_array_type ( nullable_type_annotation rank_specifier+ )+

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this allow int??[]?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, because a nullable type isn't a non_array_type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I'm reading it right, non_array_type includes value_type which includes nullable_value_type which I believe int? would be.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd agree with @gafter. nullable_value_type is defined as non_nullable_value_type nullable_type_annotation. There's no additional condition for a nullable_value_type. So, int?? isn't a valid type.

@jnm2 jnm2 Nov 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

non_array_type may expand into value_type, which may expand into nullable_value_type, which expands into non_nullable_value_type nullable_type_annotation. Therefore, non_array_type may expand into non_nullable_value_type nullable_type_annotation. Substituting that:

non_array_type ( nullable_type_annotation rank_specifier+ )+

may be

non_nullable_value_type nullable_type_annotation ( nullable_type_annotation rank_specifier+ )+

Notice how nullable_type_annotation appears twice. The proposed grammar allows int??[] as a valid construction.

@gafter
gafter marked this pull request as ready for review March 20, 2025 20:32

@Nigel-Ecma Nigel-Ecma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See TG2 email discussion for details.

@Nigel-Ecma
Nigel-Ecma marked this pull request as draft April 14, 2025 05:58
@jskeet

jskeet commented Jun 11, 2025

Copy link
Copy Markdown
Contributor

We want to wait for a meeting where @gafter is present to discuss this as well, but @jnm2 is keen to get his hands dirty with it when he finds time.

@jnm2 jnm2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We want to wait for a meeting where @gafter is present to discuss this as well, but @jnm2 is keen to get his hands dirty with it when he finds time.

I made it happen! See #1386 for my proposed grammar changes. If we take my PR as the direction, @gafter's other changes in this PR will be valuable to move over.

@BillWagner BillWagner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once grammar validation is added, I'm ready to accept this.

Comment thread standard/types.md
array_type
: non_array_type rank_specifier+
| non_array_type rank_specifier+
| non_array_type ( nullable_type_annotation rank_specifier+ )+

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd agree with @gafter. nullable_value_type is defined as non_nullable_value_type nullable_type_annotation. There's no additional condition for a nullable_value_type. So, int?? isn't a valid type.

@jskeet

jskeet commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

Closing in favor of #1386 - @gafter if you feel strongly that we should come back to this instead, please reopen and we can discuss.

@jskeet jskeet closed this Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meeting: discuss This issue should be discussed at the next TC49-TG2 meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants