-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Require AddExplicitInterfaceImplementation for adding a type that implements member explicitly #73206
Conversation
…lements member explicitly
src/Features/Core/Portable/EditAndContinue/EditAndContinueCapabilitiesGrantor.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/EditAndContinue/Utilities/Extensions.cs
Outdated
Show resolved
Hide resolved
@davidwengier ptal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the number of tests that have the new capability, I'm surprised this hasn't come up in the past. Or maybe it has :)
@davidwengier The tests do not actually apply the change. Even if they did, the AV is from .NET Framework and it does not occur if the change is the first applied change. |
Yeah, I just took the number of tests as an indication of the number of scenarios that could cause an AV. If the ordering is also important, then that could explain it for sure. |
Waiting for more customer validation from 17.11 previews before merging. |
Is there a corresponding runtime change or a GH issue to light this up for CoreCLR and Mono? /cc @mikelle-rogers |
/cc @isadorasophia |
Adding a new row to InterfaceImpl table may case AV on .NET Framework runtime. Report rude edit to prevent that from happening.
The rude edit is reported when
AddExplicitInterfaceImplementation
runtime capability is not present.Contributes to fix of https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1452066.
Debugger part: https://devdiv.visualstudio.com/DevDiv/_git/Concord/pullrequest/546202