Skip to content
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.

Sign up for GitHub

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

Jump to bottom

C# 6.0 feature: expression bodied members #4

Merged
merged 11 commits into from
May 13, 2021
Merged

C# 6.0 feature: expression bodied members #4

merged 11 commits into from
May 13, 2021

Conversation

BillWagner
Copy link
Member

Enable members to be expressions in addition to block statements.

@BillWagner
Copy link
Member Author

@jskeet This one is also ready for committee review.

@BillWagner BillWagner mentioned this pull request Nov 30, 2020
Closed
@RexJaeschke RexJaeschke mentioned this pull request Jan 14, 2021
Merged
@jskeet jskeet added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Feb 5, 2021
jskeet
jskeet reviewed Feb 10, 2021
View reviewed 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.

Looks okay to me apart from the inconsistency Nigel has spotted (and the grammar line).

standard/classes.md Show resolved Hide resolved
standard/classes.md Show resolved Hide resolved
standard/classes.md Show resolved Hide resolved
standard/classes.md Outdated
The *accessor_declarations*, which shall be enclosed in "`{`" and "`}`" tokens, declare the accessors ([§15.7.3](classes.md#1573-accessors)) of the property. The accessors specify the executable statements associated with reading and writing the property.
A *property_body* may either consist of an ***accessor body*** or an expression body ([§15.6.1](classes.md#1561-general)). In an accessor body, *accessor_declarations*, which shall be enclosed in "`{`" and "`}`" tokens, declare the accessors ([§15.7.3](classes.md#1573-accessors)) of the property. The accessors specify the executable statements associated with reading and writing the property.

An expression body consisting of `=>` followed by an *expression* `E` and a semicolon is exactly equivalent to the statement body `{ get { return E; } }`, and can therefore only be used to specify getter-only properties where the result of the getter is given by a single expression.
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: this is correct because we're only updating to C# 6. In C# 7 of course, there can be expression-bodied setters as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jskeet I just looked at PR #69, which I created for v7 edits in this area, and we don't use either term there.

@jskeet
Copy link
Contributor

jskeet commented Feb 10, 2021

We discussed "statement body" vs "block body" and decided to go for "block body" unless there's an objection. We should fix 12.16.1 to refer to block bodies as well (in the example comment)

Possible reasons to not do this:

  • "Statement-bodied lambda" is a somewhat-well-known-phrase
  • It may affect consistency with the Roslyn API

We will make sure we get fuller approval than usual (in particular @gafter, please comment) in case there are good reasons we've missed.

@RexJaeschke RexJaeschke added this to the C# 6 milestone Mar 17, 2021
@BillWagner BillWagner force-pushed the feature-expression-bodied-members branch from abb347f to 884adc7 Compare March 17, 2021 14:16
@jskeet
Copy link
Contributor

jskeet commented Apr 7, 2021

I don't think there have been any changes since the last meeting - so this meeting, let's just make sure we have concrete plans.

@BillWagner BillWagner self-assigned this Apr 7, 2021
@jskeet
Copy link
Contributor

jskeet commented Apr 7, 2021

Next work for @BillWagner:

  • Addressing comments
  • Change statement body to block body

@BillWagner BillWagner force-pushed the feature-expression-bodied-members branch from 4a5f7fe to ca48fb1 Compare April 14, 2021 15:36
@BillWagner
Copy link
Member Author

BillWagner commented Apr 14, 2021 •
edited
Loading

@Nigel-Ecma @jskeet All changes requested have been applied in the Classes clause.

I did not change 12.6.1 (Expressions, anonymous functions, general). The term body is used in the grammar in that clause. I think we can replace it with statement, and replace "block body" with "statement body" in the subsequent paragraph. If we agree, I'll make that change. Otherwise this should be ready to merge and mark as complete for V6.

@BillWagner
Remove ';' from grammar for method_body.
73e6a2f 
That should not have been included.
@jskeet
Copy link
Contributor

jskeet commented Apr 14, 2021

Thanks Bill. I now can't find the right comment to reply to... and I'm confused about why I saw it still with | ; but without the ; on its own... Not sure what's going on there.
It's just possible that we'll need to put it back in at some point for abstract methods and interfaces of course - I'll check that tomorrow. (If that's required, then presumably it's a bug from earlier versions.)

@BillWagner
Copy link
Member Author

@jskeet

I think you're last comment is correct: The grammar should include the ; production for both abstract and partial methods.

(On the other note about finding the conversations: When I make the update, I 'resolve' the conversation, which hides it in the UI. You can "view resolved conversation" to see them, then you can un-resolve as well.

@Nigel-Ecma
Copy link
Contributor

@jskeet @BillWagner

  • The conversion block body -> statement body looks good
  • Agree method_body should have an | ';' alternative
  • In §12.6.1 block can be replaced by statement, that's not right or what we've done in method_body – we've called the block option a "statement body". So as suggested in the subsequent para I think "block body" does become "statement body". Also in §12.6.3.15, Finding the best common type of a set of expressions, I think "block bodies" should become "statement bodies".

@jskeet
Copy link
Contributor

jskeet commented May 4, 2021

@Nigel-Ecma: I think it would be better to use "block body" everywhere that we're referring to something with a block. What's your reasoning for preferring "statement body" in some places?

@Nigel-Ecma
Copy link
Contributor

@jskeet @BillWagner

No preference, I assumed I'd missed something...

Jon while your Feb 11 comment above records the decision was to go with block body; it includes the note "Statement-bodied lambda" is a somewhat-well-known-phrase...

When I came to review I noted that the edit was the other way around so I assumed that note or something else had made the decision go the other way, something I don't recall but could have forgotten, so I just checked the edit was consistent and nothing missed.

I don't have a strong preference either way, just that it must be consistent, which it now is.

Bill?

@jskeet
Copy link
Contributor

jskeet commented May 4, 2021

Right - I think @BillWagner took things in the opposite direction of what I'd expected. (Thanks for spotting this.)

We were definitely looking to make "block body" the canonical term, as per an email I sent on Feb 11th:

We're aware that "statement-bodied lambda expression" is a term in relatively wide use... but we believe we can be more self-explanatory and consistent if we use "block body" everywhere, effectively removing any trace of "statement body" from the standard (other than possibly adding a note).

@BillWagner
Copy link
Member Author

@jskeet @Nigel-Ecma

Good catch. I did reverse that. I'll update this PR now.

Nigel-Ecma
Nigel-Ecma approved these changes May 4, 2021
View reviewed changes
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

Now is consistent the other way around :-)

jskeet
jskeet reviewed May 13, 2021
View reviewed 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.

Very nearly there - just a couple of nits.

standard/classes.md Show resolved Hide resolved
standard/classes.md Outdated Show resolved Hide resolved
@BillWagner BillWagner merged commit 4216150 into draft-v6 May 13, 2021
@BillWagner BillWagner deleted the feature-expression-bodied-members branch May 13, 2021 13:51
@KalleOlaviNiemitalo KalleOlaviNiemitalo mentioned this pull request Jun 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RexJaeschke RexJaeschke RexJaeschke left review comments

@jskeet jskeet jskeet approved these changes

@Nigel-Ecma Nigel-Ecma Nigel-Ecma approved these changes

Assignees

@BillWagner BillWagner

Labels
meeting: discuss This issue should be discussed at the next TC49-TG2 meeting
Projects
None yet
Milestone
C# 6
Development

Successfully merging this pull request may close these issues.
4 participants
@BillWagner @jskeet @Nigel-Ecma @RexJaeschke