Expression body syntax for accessors, ctor and finalize #8594

lachbaer · 2016-02-11T10:45:03Z

Allowing => arrow operator on get, set, add and remove accessors.
(Proposal #7881)

- src\Tools\Source\CompilerGeneratorTools\Source\CSharpSyntaxGenerator\SourceWriter.cs Change DetermineMinimalOptionalField to output more default fields, so that BlockSyntax and ArrowExpressionClauseSyntax are allowed for AccessorDeclarationSyntax in Syntax.xml. That also changed the definition in Syntax.xml.Generated.cs for "public static CatchClauseSyntax CatchClause()"

…redundancy

davkean · 2016-02-12T15:33:27Z

tag @dotnet/roslyn-compiler

Thanks for you contribution, just to set expectations, this has to be approved by the language design committee before we'll merge this in.

The jobs have run, and there's test failures:

    Microsoft.CodeAnalysis.CSharp.UnitTests.ParsingErrorRecoveryTests.TestMethodAfterPropertyGet
    Microsoft.CodeAnalysis.CSharp.UnitTests.ParsingErrorRecoveryTests.Repro674564
    Microsoft.CodeAnalysis.CSharp.UnitTests.ParsingErrorRecoveryTests.TestPropertyAccessorsWithoutBodiesOrSemicolons
    Microsoft.CodeAnalysis.CSharp.UnitTests.ParserErrorMessageTests.CS1043ERR_SemiOrLBraceExpected
    Microsoft.CodeAnalysis.CSharp.Semantic.UnitTests.Semantics.ExpressionBodiedMemberTests.BlockBodyAndExpressionBody_11
    Microsoft.CodeAnalysis.CSharp.Semantic.UnitTests.Semantics.ExpressionBodiedMemberTests.BlockBodyAndExpressionBody_08
    Microsoft.CodeAnalysis.CSharp.Semantic.UnitTests.Semantics.ExpressionBodiedMemberTests.BlockBodyAndExpressionBody_07
    Microsoft.CodeAnalysis.CSharp.Semantic.UnitTests.Semantics.ExpressionBodiedMemberTests.BlockBodyAndExpressionBody_02
    Microsoft.CodeAnalysis.CSharp.Semantic.UnitTests.Semantics.ExpressionBodiedMemberTests.BlockBodyAndExpressionBody_01

These will need to be resolved, if the tests are now expecting a behavior that has changed, either update them or delete them if they know longer make sense.

davkean · 2016-02-12T16:22:02Z

@dotnet-bot test eta please

…nstead of ERR_SemiOrLBraceExpected

…cExpression

…43ERR_SemiOrLBraceExpected()

lachbaer · 2016-02-14T23:01:28Z

Can anyone who has more experience than me shed a light on the windows tests are often hanging at the end? I think that there may be a race condition, but I have no idea where to look for it.

davkean · 2016-02-15T02:37:48Z

@dotnet-bot retest prtest/win/dbg/unit64 please
// Previous failure: http://dotnet-ci.cloudapp.net/job/roslyn_prtest_win_dbg_unit64/3830/
// Retest reason: Machine when offline

lachbaer · 2016-02-15T07:38:45Z

As my implementation has changed some of the shipped APIs, I have overloaded those methods with the old signature in SyntaxFactory.cs (for statics) and PropertyDeclarationSyntax.cs (for instances). I dunno if that is right. Is there another place?

jaredpar · 2016-02-15T14:50:03Z

@lachbaer the one API issue I saw glancing through that needed to be updated was the removal of this call:

static Microsoft.CodeAnalysis.CSharp.SyntaxFactory.CatchClause() -> Microsoft.CodeAnalysis.CSharp.Syntax.CatchClauseSyntax

We need to maintain binary compat hence can't remove this. It's fine for this to be implemented by just chaining into the new version that you added though.

lachbaer · 2016-02-15T19:17:34Z

SyntaxFactory.CatchClause() changed, because I had to patch CSharpSyntaxGenerator to emit a proper AccessorDeclaration(SyntaxKind kind, BlockSyntax body = null) in Syntax.xml.Generated.cs. The patch had no impact on any other APIs or methods. But how can we define a method CatchClause() when the generated method already has an optional parameter?

…cExpression

…re for CatchClause()

lachbaer · 2016-02-16T11:40:28Z

I've fixed that issue with CatchClause().

alrz · 2016-02-16T11:45:59Z

@lachbaer Is there any ambiguity that => solves? I mean woudn't it be clear if there was no => and the expression just followed by get and set?

lachbaer · 2016-02-16T11:48:51Z

@alrz goto #7881 for discussion please. Answer: no, just syntacitc sugar, because set and get are actually method definitions and methods allow expression syntax, with =>

lachbaer · 2016-02-28T19:19:29Z

@davkean Do you still have this noted?

davkean · 2016-02-28T19:24:07Z

@gafter @jaredpar Do we have a sponsor for this issue in the Language Design Committee?

gafter · 2016-02-28T22:12:04Z

@davkean No, we do not have a sponsor for this, and it has not yet been discussed in the LDM. We have other more urgent issues.

This was discussed in the original draft design of => for properties for C# 6 and simplified to the current syntax (for getters only) by the LDM. I'm not saying it is unwanted, but there isn't anyone on the LDM clamoring for it. I will make sure we will consider it when time allows.

AlekseyTs · 2016-03-09T07:23:27Z

@lachbaer I am done with review pass. I haven't looked at test changes because it looks like some of the new or existing tests are failing.

lachbaer · 2016-03-09T09:10:18Z

@AlekseyTs Thank you very much! There must be a bug in git, because some of the changes above (the accidents) aren't in my repository, or are stashed. So I have no idea how they got pushed.
Well, I don't even see those diffs when I inspect my pushes on github?! Confused...
Found it, finally. But nevertheless I'm confused how they got pushed anyway.

lachbaer · 2016-03-09T09:34:33Z

The tests fail, because the feature must be enabled for them now.

…ession

lachbaer · 2016-03-09T12:06:18Z

@jaredpar @davkean @gafter @AlekseyTs
Do you also want this for anonymous methods?

Action a = delegate () { DoSomething(); };
Action a = delegate () => DoSomething();
Action a = () => DoSomething();

I know that this is somehow redundant to lambdas, but it would allow for "expression syntax everywhere", at least everywhere where a method block body is allowed, meaning that there will be no exception where this is not allowed.

AlekseyTs · 2016-03-09T17:36:07Z

@gafter Could you bring the question above to the next LDM?
CC @MadsTorgersen

agocke · 2016-03-09T17:39:11Z

src/Compilers/CSharp/Portable/Compilation/SyntaxTreeSemanticModel.cs

-                                GetOrAddModelIfContains(constructorDecl.Body, span);
+                                   GetOrAddModelIfContains(constructorDecl.Initializer, span) ??
+                                   GetOrAddModelIfContains(expressionBody, span) ??
+                                   GetOrAddModelIfContains(constructorDecl.Body, span);
                        }

                    case SyntaxKind.DestructorDeclaration:


Is the Destructor case now the same as the MethodDeclaration case?

agocke · 2016-03-09T18:06:29Z

@lachbaer @gafter @MadsTorgersen I don't think we should add expression bodies to the existing anonymous delegate syntax, but I think if we add a way to ignore parameters in lambda syntax there's no reason for anyone to ever use it again (and we can finally declare it "deprecated").

jaredpar · 2016-03-09T18:25:49Z

Agree with @agocke. The anonymous delegate syntax is a construct we don't want to invest in further. Would instead like to separately consider a way to make lambdas capable of having the ignore parameter list feature.

lachbaer · 2016-03-09T21:51:23Z

@agocke @jaredpar I think you are right. I had a look at it and I think that it is not worth the effort to put in it. Anonymous delegates don't have much similarity to the changes done here.

Regarding the ignore parameter list, do you intend to omit the () completeley and start with => directly? Otherwise I would suggest to accept (void) (as a stand-alone) in the parameter list. Same for methods also, to keep the look'n'feel of the language consistent. For parameter less lambdas it would look like this

Action act = 
  void => {
    DoSome();
    Things();
  };
// methods:
  public DateTime GetRandomDate(void) => _randomDate.Next();

In my opinion that looks much better than () => { /**/ } or just => { /**/ }!

…ged unshipped API Shortened SyntaxExtensions.cs

gafter · 2016-04-27T17:28:39Z

Giving to @jaredpar for reassignment

jaredpar · 2016-09-01T21:12:10Z

Hey @lachbaer,

First wanted to apologize for going silent on this PR after the work you put into it. The team ended up getting swamped on a number of issues and this fell off of my radar. Again my apologies.

On a better note though we have a bit more time in our schedule than originally anticipated. We’ve decided to allocate some of that time to getting this feature into C# 7.0. @gafter has already taken this PR, gotten it up to date with master and will be sending out a PR shortly to move it into a feature branch. The code needs to move there temporarily so we can finish off some of the IDE, ENC work involved before merging back into master. Hoping to complete this work in the next week in order to get it ready for the next preview.

Thanks again for your contribution!

gafter · 2016-09-01T23:16:13Z

@lachbaer Your rebased PR is now at #13543 .

gafter · 2016-09-07T03:44:27Z

This PR has been revised as #13543

gafter · 2016-09-07T03:45:13Z

@AndyGerlicher You have some automation that just removed a label from this PR. Please figure out what is doing that and cause it to stop.

lachbaer added 2 commits February 10, 2016 15:10

Bug Fixes for deadlock, better name for error constant, removed code …

c35bfb9

…redundancy

dnfclas added the cla-already-signed label Feb 11, 2016

davkean added Community The pull request was submitted by a contributor who is not a Microsoft employee. Area-Compilers labels Feb 11, 2016

lachbaer added 7 commits February 14, 2016 13:01

Fixed ParserErrorXXXTests to expect ERR_SemiOrLBraceOrArrowExpected i…

162317d

…nstead of ERR_SemiOrLBraceExpected

Merge branch 'future' of https://github.com/dotnet/roslyn into propac…

360eecb

…cExpression

Renamed test function CS7887ERR_SemiOrLBraceOrArrowExpected() to CS10…

bc9b6f2

…43ERR_SemiOrLBraceExpected()

Respect ERR_AccessorListAndExpressionBody again

62a7fcd

Parser has issues with syntacic errors

5f55d92

Merge branch 'future' into propaccExpression

a29e585

Cleaned code and added methods for API backwards compatibility

90023cb

jaredpar added the Area-Language Design label Feb 15, 2016

lachbaer added 2 commits February 16, 2016 09:30

Merge branch 'future' of https://github.com/dotnet/roslyn into propac…

5377b5a

…cExpression

Fixed CSharpSyntaxGenerator/SourceWriter.cs to create old API signatu…

d602f7e

…re for CatchClause()

gafter self-assigned this Mar 3, 2016

gafter added this to the 2.0 (RC) milestone Mar 3, 2016

Reverted accidents from commit 8d72a23

6f10a90

lachbaer added 2 commits March 9, 2016 11:11

Changes based on 2nd review

06259cf

Merge remote-tracking branch 'remotes/origin/future' into propaccExpr…

026b3c4

…ession

agocke reviewed Mar 9, 2016
View reviewed changes

Reverted CSharpSyntaxGenerator/SourceWriter.cs back to original, chan…

6a58991

…ged unshipped API Shortened SyntaxExtensions.cs

lachbaer mentioned this pull request Mar 17, 2016

Proposal: allow void keyword as parameter for methods and lambdas #9834

Closed

gafter mentioned this pull request Mar 31, 2016

Language Feature Status Document #10233

Merged

gafter assigned jaredpar and unassigned gafter Apr 27, 2016

alrz mentioned this pull request May 11, 2016

Proposal: Expression-bodied events #11198

Closed

jasonmalinowski removed the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Jun 6, 2016

gafter self-assigned this Aug 25, 2016

gafter closed this Sep 7, 2016

AndyGerlicher removed the 4 - In Review A fix for the issue is submitted for review. label Sep 7, 2016

gafter added the Feature - Expression-Bodied Ctor/Dtor/Accessor Expression-Bodied Members label Sep 8, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expression body syntax for accessors, ctor and finalize #8594

Expression body syntax for accessors, ctor and finalize #8594

lachbaer commented Feb 11, 2016

davkean commented Feb 12, 2016

davkean commented Feb 12, 2016

lachbaer commented Feb 14, 2016

davkean commented Feb 15, 2016

lachbaer commented Feb 15, 2016

jaredpar commented Feb 15, 2016

lachbaer commented Feb 15, 2016

lachbaer commented Feb 16, 2016

alrz commented Feb 16, 2016

lachbaer commented Feb 16, 2016

lachbaer commented Feb 28, 2016

davkean commented Feb 28, 2016

gafter commented Feb 28, 2016

AlekseyTs commented Mar 9, 2016

lachbaer commented Mar 9, 2016

lachbaer commented Mar 9, 2016

lachbaer commented Mar 9, 2016

AlekseyTs commented Mar 9, 2016

agocke Mar 9, 2016

agocke commented Mar 9, 2016

jaredpar commented Mar 9, 2016

lachbaer commented Mar 9, 2016

gafter commented Apr 27, 2016

jaredpar commented Sep 1, 2016

gafter commented Sep 1, 2016

gafter commented Sep 7, 2016

gafter commented Sep 7, 2016

Expression body syntax for accessors, ctor and finalize #8594

Expression body syntax for accessors, ctor and finalize #8594

Conversation

lachbaer commented Feb 11, 2016

davkean commented Feb 12, 2016

davkean commented Feb 12, 2016

lachbaer commented Feb 14, 2016

davkean commented Feb 15, 2016

lachbaer commented Feb 15, 2016

jaredpar commented Feb 15, 2016

lachbaer commented Feb 15, 2016

lachbaer commented Feb 16, 2016

alrz commented Feb 16, 2016

lachbaer commented Feb 16, 2016

lachbaer commented Feb 28, 2016

davkean commented Feb 28, 2016

gafter commented Feb 28, 2016

AlekseyTs commented Mar 9, 2016

lachbaer commented Mar 9, 2016

lachbaer commented Mar 9, 2016

lachbaer commented Mar 9, 2016

AlekseyTs commented Mar 9, 2016

agocke Mar 9, 2016

Choose a reason for hiding this comment

agocke commented Mar 9, 2016

jaredpar commented Mar 9, 2016

lachbaer commented Mar 9, 2016

gafter commented Apr 27, 2016

jaredpar commented Sep 1, 2016

gafter commented Sep 1, 2016

gafter commented Sep 7, 2016

gafter commented Sep 7, 2016