Skip to content

[Default Interfaces] Edit and Continue #9601

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

Closed
MichalStrehovsky opened this issue Jan 25, 2018 · 11 comments
Closed

[Default Interfaces] Edit and Continue #9601

MichalStrehovsky opened this issue Jan 25, 2018 · 11 comments

Comments

@MichalStrehovsky
Copy link
Member

Not sure if any work is needed, so this tracks just checking whether it works for now.

Might want to specifically try shared methods on generic types.

@noahfalk
Copy link
Member

@tmat - Is there anything that needs to be done EnC-wise? I assume adding a default interface method should be classified as a rude edit and maybe a test case needs to be added/adjusted?

@tmat
Copy link
Member

tmat commented Jan 16, 2019

That's pretty much up to you - how much do you want to invest into EnC support? The more we invest the less the customer needs to restart their debugging session and thus the more productive they are. The more features that do not work the worse the overall experience is and the more dissatisfied customers we get.

With each feature we need to consider what operations are possible or can be made possible.

Re Default interfaces specifically, the operation would be "Adding a method with a body to an interface", correct?

If you can make that work in runtime then we will make it work in the compiler and we don't need a rude edit.

Other useful operations would be

  • Making a class implement an interface (adding entry to InterfaceImpl table)
  • Making a method implement an interface method (updating MethodImpl table)

@noahfalk
Copy link
Member

Adding support would be a non-trivial runtime change and to date I don't think we've supported any edits to interfaces in EnC right? I wasn't planning to change that level of support for default interfaces.

Thus far I had been expecting to get EnC related customer feedback primarily from the VS debugger team and hadn't heard anything to suggest it was a high priority issue. Customers that we reached out to independently had other diagnostic priorities (VS debugger was frequently a high-point in the overall experience). I'm happy to take a look at any other feedback you think is worthwhile though. Thanks @tmat!

@tmat
Copy link
Member

tmat commented Jan 17, 2019

Yes, we don't allow adding methods to interfaces currently. Adding a method without an implementation is more complicated than adding a method with an implementation, because you'd need to update all types that implement the interface at the same edit. I guess that would be also theoretically possible and perhaps it's the same complexity for runtime regardless of whether the method has implementation or not.

I'd think the feedback on EnC is split between debugger and Roslyn. From the customer perspective it's C#/VB debugging experience, so it gets filed on Roslyn. We have seen many complains that EnC doesn't work in various cases. Coincidentally I'm currently looking at some of them:

https://developercommunity.visualstudio.com/content/problem/107803/during-debugging-no-changes-are-allowed-if-edit-co.html
https://developercommunity.visualstudio.com/content/problem/119803/edit-and-continue-warns-about-changes-that-cannot.html
https://developercommunity.visualstudio.com/content/problem/241414/edit-and-continue-sees-error-in-line-that-no-longe.html
https://developercommunity.visualstudio.com/content/problem/244554/edit-and-continue-xunit-is-flakey.html
https://developercommunity.visualstudio.com/content/problem/322758/edit-and-continue-crash.html

I have also heard personally from our partners who have customers that would like to see EnC work in more scenarios.

BTW: You can also query dotnet/roslyn/issues for label "Interactive-EnC":
https://github.com/dotnet/roslyn/issues?q=is%3Aopen+is%3Aissue+label%3AInteractive-EnC

Specifically this customer puts it bluntly :)
dotnet/roslyn#27976

@noahfalk
Copy link
Member

Thanks @tmat! Yep dotnet/roslyn#27976 was blunt ; ) (but its good and fair feedback)

I opened dotnet/coreclr#22055 as implementing this may not be as bad as I expected. I didn't put it in 3.0, but I think its worth looking at in our next round of planning. I looked through the issues you posted and nothing specific to interface edits was apparent, but if I missed it or its referenced obliquely feel free to add notes to dotnet/coreclr#22055.

@tmat
Copy link
Member

tmat commented Jan 18, 2019

Right, we have no specific feedback for editing interfaces. Usually we won't get feedback about a specific kind of edit that was disallowed. Most customers do not realize the nuances why an edit can't be applied in this case but can't be in another case (e.g. can add a new method with body to an abstract class, but not to an interface). They just want the cases when debugging session needs to be interrupted to be rare. The more restrictions we have the worst experience it ends up being.

@noahfalk
Copy link
Member

Do we have VS telemetry about why edits were disallowed?

@tmat
Copy link
Member

tmat commented Jan 19, 2019

Yes. We prioritize based on that, although there hasn't been much time for EnC work past year anyways, so there has not been much progress. We did a lot of work enabling editing lambdas, LINQ, and async methods in past.

Most of the issues do not need CLR change. They can often be addressed in Roslyn. The one issue that is higher priority and needs work in the CLR and the debugger is editing in a generic context (generic methods, types). That's one of the reasons F# doesn't even try to implement EnC - the compiler produces a lot of generic types.

@noahfalk
Copy link
Member

Thanks good to know. Feel free to open an issue tracking EnC on generics. I don't have a good sense of what is involved, but if its important getting an idea of its cost during our next round of planning seems like a good idea.

@MichalStrehovsky
Copy link
Member Author

Interaction with default interface feature is tracked on the Roslyn side (we don't currently plan runtime work for this, so this is just about making sure rude edits work as expected). The rest of the discussed issues forked off to separate places.

@tmat
Copy link
Member

tmat commented Jan 23, 2019

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants