Skip to content

Conversation

BillWagner
Copy link
Member

Add the sections and grammar for the ?. null conditional operator.

@BillWagner BillWagner force-pushed the feature-null-conditional branch from c5cd25d to dd2026d Compare October 26, 2020 20:17
@BillWagner
Copy link
Member Author

@jskeet This is also ready for committee review

@BillWagner BillWagner force-pushed the feature-null-conditional branch from dd2026d to fba28ab Compare October 26, 2020 20:21
@jskeet
Copy link
Contributor

jskeet commented Mar 5, 2021

@BillWagner I was going to mark this for review in the next meeting, but there are conflicts. Could you have a look at the conflicts and see if they're easily resolved?

@BillWagner
Copy link
Member Author

@jskeet Both conflicts are resolved.

They were from the ANTLR work that touched grammar productions modified in this PR.

@jskeet jskeet added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Mar 5, 2021
@jskeet
Copy link
Contributor

jskeet commented Mar 5, 2021

Great - added meeting:discuss label so we can talk about it next week.

```ANTLR
unary_expression
: primary_expression
| null_conditional_expression
Copy link
Contributor

Choose a reason for hiding this comment

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

Not having read a spec for this feature (https://tinyurl.com/seyughk is not sufficient) this grammar is curious. I might have expected to see null_condition_member_access alongside member_access (and likewise for element access) as an alternative in primary_expression. Are the sentences the approach here allows/disallows which placing the new operators in primary_expression would disallow/allow? TIA

Copy link
Contributor

Choose a reason for hiding this comment

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

Nigel, I'm struggling to understand your final sentence here. Could you rephrase it? (Or just explain it in the meeting.)

Copy link
Contributor

@Nigel-Ecma Nigel-Ecma Mar 10, 2021

Choose a reason for hiding this comment

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

There is a typo it should have been:

Are there sentences the approach here allows/disallows which placing the new operators in primary_expression would disallow/allow?

Why I'm asking: it seems null_conditional_member_access could logically sit alongside member_access (and ditto for the element access variant) primary_expression (more precisely primary_no_array_creation_expression). I'm trying to figure out if the decision to place these two operators in unary_expression and introduce productions rules for them which mimic a subset of primary_expression was a choice or a requirement to describe the required syntax and semantics?

There appear to be parts of the proposed changes which only need to exist because these new constructs are not part of primary_expression, and having them there would simply the description of the semantics. But would it also complicate the description in other areas? Right now I don't know.

I'm also unsure there is any need to describe different equivalent code for when these operators occur in statement expressions, it has the feel of inserting particular/irrelevant implementation details. (Consider for example that a boolean expression might be compiled differently if its result is stored in a variable or used as the predicate in an if – one produces a value to store the other branching code – but we don't say that in the Standard! Is this a similar case?)

Hope that makes a little more sense now, but its late (in NZ) so maybe it doesn't... We can figure that out tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, that makes more sense - we can discuss it all in the meeting. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Tagging @MadsTorgersen

I think this discussion is at the heart of Nigel's alternative PR, and the discussion on ?. being one token or two. Not making edits at this time based on this comment. I believe it accurately reflects this path toward specing the feature.

null_conditional_operations
: null_conditional_operations? '?' '.' identifier type_argument_list?
| null_conditional_operations? '?' '[' argument_list ']'
Copy link
Contributor

Choose a reason for hiding this comment

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

Does not this alternative along with the one below on line 2360 remove the restrictions that any element access starts with a primary_no_array_creation_expression and not a primary_expression?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Nigel-Ecma I think your concern here is addressed by the latest changes. As @jskeet pointed out two areas weren't integrated, but were written as stand alone feature changes.

Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

I'm not thinking clearly enough to comment on Nigel's comments, but hopefully what I have commented on is still useful :)

```ANTLR
unary_expression
: primary_expression
| null_conditional_expression
Copy link
Contributor

Choose a reason for hiding this comment

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

Nigel, I'm struggling to understand your final sentence here. Could you rephrase it? (Or just explain it in the meeting.)

@jskeet
Copy link
Contributor

jskeet commented Mar 10, 2021

Meeting March 10th: none of us can come up with a reason why this isn't a sibling of member_access. It would be simpler to specify that way.

Assigning to Mads to enlighten us - or we can rewrite (probably in a new PR).

@RexJaeschke RexJaeschke added this to the C# 6 milestone Mar 17, 2021
Add the sections and grammar for the `?.` null conditional operator.
An edit from await in catch and finally clauses (#3) snuck in.
@Nigel-Ecma
Copy link
Contributor

Meeting March 10th: none of us can come up with a reason why this isn't a sibling of member_access. It would be simpler to specify that way.

Assigning to Mads to enlighten us - or we can rewrite (probably in a new PR).

Added draft PR #251 for the alternative approach

@jskeet jskeet removed the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Apr 7, 2021
@jskeet
Copy link
Contributor

jskeet commented Apr 7, 2021

Removed meeting-discuss label as this is effectively superceded by #251 - at least, that's the hope. (If we decide not to go down that route, we'll need to do more work on this PR. If we decide we like the approach of #251, this PR can be closed.)

: statement_expression ';'
;
statement_expression
Copy link
Contributor

Choose a reason for hiding this comment

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

@BillWagner - Same as for member_declarator, here the latest change appears to replace statement_expression with only null-conditional options rather than add them to it. Has something gone awry or am I misreading this?

After discussions, I went back to the text from the Microsoft C# 6 draft. The C# 6 draft put all changes in one new clause for conditional expressions. When we went to move the two grammars for anonymous object creation and member invocation, those edits introduced grammar production errors.

The only concern now is that the anonymous object creation expression does reference the null conditional expression that is defined later in the Expressions clause.
@jskeet
Copy link
Contributor

jskeet commented Oct 20, 2021

Closing as #251 was accepted instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants