Breakpoint and instrumentation support for new C# 7 language features #14535
Conversation
…or, dtor, and accessors Fixes dotnet#14438
gafter
added
Area-Compilers
PR For Personal Review Only
|
👍 so far. I suggest merging now and opening new PR for the rest of the changes. Although related they are of different nature. |
gafter
added
Feature Request
4 - In Review
|
@dotnet/roslyn-compiler @tmat @JohnHamby This PR adds sequence points, IDE breakpoint support, and instrumentation for all new language constructs. Please review. |
|
The changes for dynamic analysis look good to me. Thanks!!! |
|
@CyrusNajmabadi Can you please review this from an IDE perspective? |
| case SyntaxKind.DestructorDeclaration: | ||
| return TryCreateSpanForNode(((DestructorDeclarationSyntax)node).Body, position); | ||
| var methodDeclaration = (BaseMethodDeclarationSyntax)node; | ||
| return (methodDeclaration.Body != null) ? CreateSpanForBlock(methodDeclaration.Body, position) : methodDeclaration.ExpressionBody?.Expression.Span; |
There was a problem hiding this comment.
can you also add support for the expression body on a constructor?
| @@ -284,6 +291,12 @@ private static int GetEndPosition(SyntaxNodeOrToken nodeOrToken) | |||
| var groupClause = (GroupClauseSyntax)node; | |||
| return TryCreateSpanForNode(groupClause.GroupExpression, position); | |||
|
|
|||
| case SyntaxKind.LocalFunctionStatement: | |||
| var localFunction = (LocalFunctionStatementSyntax)node; | |||
| return (localFunction.Body != null) ? | |||
There was a problem hiding this comment.
- unnecessary parens.
- ? and : go on the next line.
| @@ -308,12 +321,18 @@ private static TextSpan CreateSpanForConstructorDeclaration(ConstructorDeclarati | |||
| return CreateSpanForConstructorInitializer(constructorSyntax.Initializer); | |||
| } | |||
|
|
|||
| if (constructorSyntax.ExpressionBody != null) | |||
|
minor nits. otherwise LGTM> |
|
LGTM |
|
@MattGertz @jaredpar I forgot to get approval for this RC change before integrating. This fixes sequence points, breakpoints, and instrumentation for all new language constructs. |
|
CC @Pilchie and @ManishJayaswal who are approvals today. |
|
Approved (after the fact). |
gafter
added
the
Resolution-Fixed
This is a work in progress. I will send it out for review when all of the relevant issues have been addressed. The issues that will be addressed in this PR (ideally one commit per issue) are: