dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.06k stars 4.04k forks source link

Questions about SemanticEdit related public API. #59078

Open viewizard opened 2 years ago

viewizard commented 2 years ago

Hello,

I am investigating how we could implement EnC feature in our "Visual Studio Tools for Tizen" extention and debugger (https://github.com/Samsung/netcoredbg) and found, that previously in 2016, Josh already opened issue https://github.com/dotnet/roslyn/issues/8962 about public interface for simplify IL/metadata/PDB deltas generation, that could be used in managed debugger with ICorDebugModule2::ApplyChanges, since Roslyn already have all utility code inside. I did try to find by myself in Roslyn sources possible public API in order to generate array of SemanticEdit for changed source/project/solution that could be used with Compilation.EmitDifference method, but fail. Is the any related public API were added since 2016 in Roslyn?

CC @alpencolt

tmat commented 2 years ago

We currently do not have public APIs that you could use to generate semantic edits for EmitDifference and do not have plans for adding it.

I'm not sure I understand your scenario though. Are you adding a support for a platform (Tizen) that's currently not supported by VS debugger? Does VS not have appropriate debugger protocol extension points where you can plug your debugger without needing to interface with Roslyn language services?

viewizard commented 2 years ago

We have our own managed debugger for Tizen (https://github.com/Samsung/netcoredbg), that use MI protocol to communicate with VS Tizen extension that use MIEngine, as I know. We don't use VS debugger (not sure it could be used on Tizen x86 or arm32 at all).

For now we have our extension in VS, that start our debugger on Tizen device remotely and use MI protocol for interaction. But in case EnC, VS Tizen extension should generate IL/metadata/PDB deltas and send it to our debugger, so, I investigate in what way we could have this deltas.

gregg-miskelly commented 2 years ago

Hi @viewizard,

I think the right way to integrate netcoredbg with EnC in Visual Studio would be to have Tizen's MIEngine implement our 'IManagedEditAndContinueEngineService'. It is currently internal, but I am not aware of any reason we can't make it public in a future VS version.

Do you want to open a VS suggestion ticket (Help->Send Feedback->Suggest a Feature) so that you can more easily track when we complete this work?

viewizard commented 2 years ago

Hi @gregg-miskelly,

please, correct me if I am wrong, you can make 'IManagedEditAndContinueEngineService' public only in future VS version, I mean, in some new major VS version (not in VS 2022) right? But next release could be in 2024 or even 2025. The point is, we might need add EnC support for Tizen debugger for VS 2022 Tizen extension. Probably, our VS extension team will need write 'IManagedEditAndContinueEngineService' implementation then.

Do you want to open a VS suggestion ticket (Help->Send Feedback->Suggest a Feature) so that you can more easily track when we complete this work?

Please note, my primary task now only investigate EnC and provide some "proof of concept" I could made. I believe, VS extension team (I work in another team) should decide what exactly they will do and open a VS suggestion ticket.

gregg-miskelly commented 2 years ago

you can make 'IManagedEditAndContinueEngineService' public only in future VS version, I mean, in some new major VS version (not in VS 2022) right?

No, this change could be done in any minor release of VS 2022 (example: 17.3).

Please note, my primary task now only investigate EnC and provide some "proof of concept" I could made. I believe, VS extension team (I work in another team) should decide what exactly they will do and open a VS suggestion ticket.

Got it. Well, please ask the VS extension team to let us know as soon as they can if this is something they want since it will take some time between when the feature is requested and when it ships.

alpencolt commented 2 years ago

Hi @gregg-miskelly, Could you tell more about IManagedEditAndContinueEngineService? Does it notify debugger about all changes like EditAndContinueWorkspaceService or work another way? What information it provides?

sangwook96 commented 2 years ago

Hi, @gregg-miskelly, Could you tell me more about IManagedEditAndContinueEngineService? Does it notify the debugger about all changes like EditAndContinueWorkspaceService or work another way? What information did it provide? And when will you make it or did you already complete to implement it? We are waiting for it because we want to apply for ENC on Tizen, too. Thanks

gregg-miskelly commented 2 years ago

Hi @sangwook96 and @alpencolt sorry for missing your question the first time.

IManagedEditAndContinueEngineService is an interface that an engine can implement to participate in Managed EnC/Hot Reload. Among other things, it has an ApplyCodeChangesAsync method that is given a set of updates to apply to the target process:

        /// <summary>
        /// Apply code <paramref name="updates"/> for the current debugging session.
        /// </summary>
        /// <param name="updates">Collection of updates for modules loaded in the debugging session.</param>
        /// <param name="cancellationToken">Cancellation token.</param>
        Task ApplyCodeUpdatesAsync(ImmutableArray<ManagedModuleUpdate> updates, CancellationToken cancellationToken);

IManagedEditAndContinueEngineService is defined in Microsoft.VisualStudio.Debugger.Contracts if you want to look more at the details.

As for when we will make it public: we have a bit of a chicken and egg situation here -- there is some amount of work to make it public, so we would want a commitment from some company to consume it before we make it public. If Samsung is ready to comit to consuming it, I would be happy to discuss scheduling more.

sangwook96 commented 2 years ago

@gregg-miskelly, Hi, How are you? I am happy to see your feedback here. yes, we need to have the changed code. We tried to apply hotreload feature in Tizen SDK till now from this year. If you possible, once completion to make it, please open the source codes to the public, for Samsung or Other Partners. Actually, the expected date is delayed and we are waiting for your guide and reference codes.

I would appreciate your big support for us.

sangwook96 commented 2 years ago

Hi, @gregg-miskelly Hi, "IManagedEditAndContinueEngineService is defined in Microsoft.VisualStudio.Debugger.Contracts if you want to look more at the details." Then, May I see the IManagedEditAndContinueEngineService? I go to https://www.nuget.org/packages/Microsoft.VisualStudio.Debugger.Contracts and I go to the "Source repository" but I can not access it, just I can see a 401 error. How can I see IManagedEditAndContinueEngineService? If we see it, we try to apply it to our customer debugger, too. Thanks

kusin10 commented 2 years ago

Hi @gregg-miskelly We are not able to access source repository for Microsoft.VisualStudio.Debugger.Contracts at the moment. We are getting the same 401 error. Thanks

gregg-miskelly commented 2 years ago

The fact that the "Source Repository" link is available is a bug in the nuspec for the package -- this package isn't open source and isn't intended to be. But you can see the definitions in ildasm/ILSpy.

I am hoping we will be able to work on opening up these interfaces fairly soon. The developer who will work on this has some other things she needs to finish up first, but this is close to the top of her plate.

sangwook96 commented 2 years ago

@gregg-miskelly Hi, Thank you very much for updating your feedback here. Okay. let us try to use your tools like ildasm/ILSpy (decompiler tool), but I am not sure if we take what we want to know As you mentioned, we need to get a public API or interface If you have any progress, please inform us here again Thanks

gregg-miskelly commented 2 years ago

To be clear: I am just suggesting using ildasm/ILSpy so that your team can look at the API and see if they see any issues implementing it. You could also potentially prototype implementing an API with a similar shape, but without actually getting called. You will need a new public version to implement it for real.

sangwook96 commented 2 years ago

@gregg-miskelly Hi Thank you very much for suggesting your idea. And then, we are trying to use ILSpy tool. But what is the project to reverse with ILSPY? If you know that, could you tell us and could you tell us where we can take it, too? Thanks

sangwook96 commented 2 years ago

@gregg-miskelly , Hi, I got what you say. We can reverse Microsoft.VisualStudio.Debugger.Contracts with ILSPY Thank you very much for sharing it

sangwook96 commented 2 years ago

@gregg-miskelly, HI, you can make 'IManagedEditAndContinueEngineService' public only in future VS versions, I mean, in some new major VS version (not in VS 2022) right?

No, this change could be done in any minor release of VS 2022 (example: 17.3). => When will you open it in the public, too?

Thanks

gregg-miskelly commented 2 years ago

Assuming you don't find anything that needs to change, my hope would be to make the types public in 17.4.

sangwook96 commented 2 years ago

@gregg-miskelly, HI, Thank you very much for updating your comment. Okay, It's great good news for us. Excuse me, but when will 17.4 version open to the public? like July, August ?? Thanks

sangwook96 commented 2 years ago

@gregg-miskelly, Hi, How are you? Thank you very much for updating your feedback here. I tracked the previous release history. 17.0 - 2021/11 17.1 - 2022/02 17.2 - 2022/05 17.3 - 2022/08(??) 17.4 - 2022/11(??) I know you and MS are too busy due to lots of requests from others. But in case of this issue, could you apply it to the 17.3 version? Thanks!!!

sangwook96 commented 2 years ago

Assuming you don't find anything that needs to change, my hope would be to make the types public in 17.4.

Hi In 17.4, does it make delta files, too? If 17.4 make deltas, where does it put the delta files on the host pc? like "C:\Users[UserID]\AppData\Local\Temp Because we need to push the deltas to the Tizen target. Thanks

gregg-miskelly commented 2 years ago

When will 17.4 version open to the public? like July, August ??

I am not authorized to comment on release dates. But if you are blocked waiting for work, we may be able to send you private binaries for testing/development purposes only that would function on top of 17.3.

But in case of this issue, could you apply it to the 17.3 version?

We are past the point where 17.3 features can be checked in.

If 17.4 make deltas, where does it put the delta files on the host pc?

Delta are not dropped to disk at all. Your implementation could choose to do so if that made sense for you. The deltas are stored in the ManagedHotReloadUpdate that would be sent to your implementation. Here is the relevant documentation in case it makes it more clear:

        /// <summary>
        /// Collection of IL deltas affected by the update. Required by the CLR.
        /// Empty if no update action is intended.
        /// </summary>
        [Key("ilDelta")]
        public ImmutableArray<byte> ILDelta { get; }

        /// <summary>
        /// Collection of metadata deltas affected by the update. Required by the CLR.
        /// Empty if no update action is intended.
        /// </summary>
        [Key("metadataDelta")]
        public ImmutableArray<byte> MetadataDelta { get; }

        /// <summary>
        /// Collection of PDB deltas regarding the symbol information affected by the update.
        /// </summary>
        [Key("pdbDelta")]
        public ImmutableArray<byte> PdbDelta { get; }

        /// <summary>
        /// List of TypeDefs that have been modified during an edit.
        /// This is passed on to the CLR as an event on each Edit and Continue update.
        /// </summary>
        [Key("updatedTypes")]
        public ImmutableArray<int> UpdatedTypes { get; }
sangwook96 commented 2 years ago

@gregg-miskelly, Hello, Could you share your private binaries with us? How can I take it from you via email or anything? It's really helpful for us.

Thank you very much.

gregg-miskelly commented 2 years ago

To be clear: we haven't yet done any work on this, so we don't have bits to share. Also, my hope is that there will not be a significant delay between when we do the work and when there is a VS preview build available so that privates will not be necessary. But we are happy to explore that path if it proves useful. I am happy to continue this conversation over email. My account is 'greggm' on the microsoft.com email server.

sangwook96 commented 2 years ago

@gregg-miskelly, Thank you very much for your hard work on our issue. Yes, If you are ready or have anything, please share it with us again. In fact, we really appreciate it.

sangwook96 commented 2 years ago

@gregg-miskelly, Hi, Today, an MS engineer updated his answer on https://developercommunity.visualstudio.com/t/ENC0097-Applying-source-changes-while-th/10069184?entry=myfeedback&ref=native&refTime=1656587508959&refUserId=6acb2e52-33c7-6524-8970-ceab0fd4e368&viewtype=all. Could you discuss with him our issues? Because you and he may have another solution. I appreciate it.

@kusin10, I always thank you very much

gregg-miskelly commented 2 years ago

@sangwook96 Thanks for letting me know about the feedback ticket. I resolved it so that Zoe doesn't spend any more time investigating something that isn't supposed to work yet.

sangwook96 commented 2 years ago

@gregg-miskelly, Thank you very much for updating your feedback here.

kusin10 commented 2 years ago

@gregg-miskelly Thanks alot for the update. I would like to ask, that by implementing IManagedEditAndContinueService will fix this ENC0097 error or some more changes be required in MIEngine for that?

sangwook96 commented 2 years ago

@kusin10, Hi, Thank you very much for asking @gregg-miskelly.

@gregg-miskelly, Thank you very much for hard-working for Tizen and other partners about the issue. As you mentioned, you can fix it by yourself and we just need to wait for your result. Am I right to understand? Now, we do not investigate it and just are waiting for your feedback now. In addition, have you reproduced the enc error on our vsix (vs for tizen)? It always occurs when we use hot-reload. Thanks

gregg-miskelly commented 2 years ago

By implementing IManagedEditAndContinueService will fix this ENC0097 error or some more changes be required in MIEngine for that?

That should be all that is needed.

As you mentioned, you can fix it by yourself and we just need to wait for your result. Am I right to understand?

No. VS will enable engines to implement IManagedEditAndContinueService. When an implementation of IManagedEditAndContinueService for Tizen is contributed the issue will go away.

In addition, have you reproduced the enc error on our vsix (vs for tizen)?

Since the behavior you are seeing is expected, I didn't attempt to reproduce it.

It always occurs when we use hot-reload.

The naming of edit-and-continue and hot reload is confusing. IManagedEditAndContinueService is used to apply changes to a process while debugging regardless as to if the process is stopped or not.

If you also want to support hot reload while NOT debugging, you want to also implement IManagedHotReloadAgent and have your project system's debug launcher (the component that knows how to launch projects on Tizen) call IHotReloadAgentManagerClient.AgentStartedAsync. HotReloadAgentManagerClient can be imported via MEF. These interfaces are also in the debugger contracts assembly.

sangwook96 commented 2 years ago

@gregg-miskelly, Hi, Thank you very much for updating your comment.

comment. The naming of edit-and-continue and hot reload is confusing. IManagedEditAndContinueService is used to apply changes to a process while debugging regardless as to if the process is stopped or not.

=> It occurs like that

  1. Check the checkbox, Debug->Options->Debugging->.NET / C++ Hot Reload : Apply Hot Reload on File Save
  2. Set the Breakpoint and start F5 (Debugging mode)
  3. Just Edit the line on *.cs file.
  4. ENC007 Error happens.

I think that the error step(behavior) is simple.

Thank you very much

sangwook96 commented 2 years ago

@gregg-miskelly, Hi how are you doing? do you have any progress? we see the new version on https://docs.microsoft.com/en-us/visualstudio/releases/2022/release-notes-preview#17.3.0-pre.3.0, Is there any updated patchset for our issue, on the new version? Thanks

sangwook96 commented 2 years ago

@gregg-miskelly, Hi, How are you? [Question] By implementing IManagedEditAndContinueService will fix this ENC0097 error or some more changes be required in MIEngine for that? [Your answer] That should be all that is needed.

[Question] As you mentioned, you can fix it by yourself and we just need to wait for your result. Am I right to understand?

[Your answer] No. VS will enable engines to implement IManagedEditAndContinueService. When an implementation of IManagedEditAndContinueService for Tizen is contributed the issue will go away.

Do you have any progress? When will you have a plan to open it to the public? 17.4 preivew1.0 version was already released to the public. But we can not see it anywhere. Could you share your plan, too? Thanks

HI, I have updated one more query for you. When I change a source code in debug mode, I can see errors like the below that

Exception thrown: 'System.ArgumentException' in Microsoft.VisualStudio.Platform.VSEditor.dll Exception thrown: 'System.ArgumentException' in Microsoft.VisualStudio.Platform.VSEditor.dll

Could we ignore the errors?