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.

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

[Umbrella] Improve EnC support for C#7 features #12379

Closed
6 of 13 tasks
tmat opened this issue Jul 7, 2016 · 5 comments
Closed
6 of 13 tasks

[Umbrella] Improve EnC support for C#7 features #12379

tmat opened this issue Jul 7, 2016 · 5 comments
Labels
Milestone

Comments

@tmat
Copy link
Member

tmat commented Jul 7, 2016

Scope 0:

Scope 1:

  • Reasonable diff function
  • Disable EnC in presence of v7 syntax (deconstruction, tuple, out var, ref locals, local functions) for active methods

Scope 2:

  • Distance computation for C# 7 nodes, tracking slots with newly introduced locals and temps
@gafter
Copy link
Member

gafter commented Jul 7, 2016

@jaredpar @AlekseyTs @agocke FYI

@DustinCampbell
Copy link
Member

FWIW, it'd be great to have a future work item to add EnC support here.

@gafter gafter modified the milestones: 2.0 (RC), 2.0 (RTM) Jul 18, 2016
@gafter gafter modified the milestones: 2.0 (RTM), 2.0 (RC) Aug 8, 2016
@gafter gafter changed the title Disable EnC when a body contains new C# 7 syntax nodes [Umbrella] Disable EnC when a body contains new C# 7 syntax nodes Aug 9, 2016
@gafter gafter modified the milestones: 2.0 (Preview 5), 2.0 (RC) Aug 9, 2016
@gafter
Copy link
Member

gafter commented Aug 9, 2016

In private email, I asked @tmat

what is the right way to make it so that changes to a method that uses certain new language features is a rude edit? Is this even possible?

He responded

It is certainly possible, on two levels. The big hammer is to disable analysis of a file containing any C# 7 syntax node. To do so a rude edit diagnostic should be added here (will need to add abstract method overridden by both C# and VB EnC analyzer). That will make sure we don’t even try to analyze any changes in that file, so the tree diffing code doesn’t need to be updated to account for new syntax nodes.

Alternatively we can add support for all new syntax nodes to the tree diffing code and only disallow editing active methods containing C# 7 features. That’d be less annoying for the user since they can at least step out of the method and edit it once it’s not active anymore (unless it’s an async or an interator method which we consider active all the time currently). To do so you can check out existing code in VB that’s reporting rude edits for active methods containing on-error-resume statements.

@gafter gafter modified the milestones: 2.0 (RC), 2.0 (Preview 5) Aug 9, 2016
@jcouv
Copy link
Member

jcouv commented Aug 29, 2016

Notes from our discussion on 2.0 scope:

  • Scope 0 (min bar): no edit on any method that includes any new syntax node or a v7 switch syntax node; handle tuples in method signature (some diff work). SWAG 3w: some ramp-up, 1w coding, 1w testing
  • Scope 1 (plan of record): no edit on active method that includes any new syntax node (or v7 switch). Edit on inactive methods with new nodes is allowed and we are able to do some reasonable diff function. SWAG: a couple of days to a week, depending on how fancy we get.
  • Scope 2: no edit on active statement, but edit on inactive statement is ok (even if new syntax node is involved). We can enable new syntax nodes one by one. Deconstruction probably easiest, then pattern and out var. Didn’t SWAG.

@gafter gafter assigned jaredpar and unassigned gafter Aug 31, 2016
@jcouv jcouv changed the title [Umbrella] Disable EnC when a body contains new C# 7 syntax nodes [Umbrella] Disable EnC when a body contains newly introduced nodes Sep 14, 2016
@jcouv jcouv changed the title [Umbrella] Disable EnC when a body contains newly introduced nodes [Umbrella] Improve EnC support for new features Sep 14, 2016
@jcouv jcouv modified the milestones: 2.0 (RC), 2.0 (Preview 5) Sep 14, 2016
@jcouv jcouv modified the milestones: 2.0 (RTM), 2.0 (RC) Oct 7, 2016
@jcouv jcouv modified the milestones: 2.1, 2.0 (RTM) Nov 16, 2016
@jcouv jcouv changed the title [Umbrella] Improve EnC support for new features [Umbrella] Improve EnC support for C#7 features Dec 18, 2016
@jcouv jcouv removed the 3 - Working label Jan 20, 2017
@jcouv jcouv modified the milestones: 2.1, 2.2 Mar 3, 2017
@jcouv jcouv removed their assignment Mar 8, 2017
@gafter gafter modified the milestones: 15.later, 15.2 Apr 24, 2017
@jcouv
Copy link
Member

jcouv commented May 28, 2017

I think this issue here is superceded by #17891 (and in fact most of the C# 7.0 features, except for local functions, now work with EnC). So I'll close this as duplicate.

@jcouv jcouv closed this as completed May 28, 2017
@jcouv jcouv added the Resolution-Duplicate The described behavior is tracked in another issue label May 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants