Update ICSharpCode.Decompiler to 4.0-beta1 #29029
Conversation
|
|
||
| // Initialize a decompiler with default settings. | ||
| var decompiler = new CSharpDecompiler(assemblyDefinition.MainModule, new DecompilerSettings()); | ||
| // TODO: Use language version currently used by the project. |
There was a problem hiding this comment.
📝 This won't be necessary. As #24349 progresses and we favor the original source over decompiled source in more cases, the language version shown for these files will naturally vary from the current project. I don't want to define the Navigate to Decompiled Source in such a way that users expect the language version to match the current project because it will lead to questions/bugs about mismatched versions down the road.
There was a problem hiding this comment.
That's fine... it was just an idea I had this morning... 😃 I will remove this comment.
| @@ -224,10 +225,13 @@ private string GetRootPathWithGuid_NoLock() | |||
| } | |||
|
|
|||
| // Load the assembly. | |||
| var assemblyDefinition = AssemblyDefinition.ReadAssembly(assemblyLocation, new ReaderParameters() { AssemblyResolver = new RoslynAssemblyResolver(compilation) }); | |||
| // TODO: Use a different PEFile ctor overload that allows us to reuse the PEReader or Stream already created by Roslyn. | |||
There was a problem hiding this comment.
❗️ These comments are best removed before merge, and follow-up issues filed for them. The follow-up issues can be created before or after merging the pull request, but when you remove the TODO comments you should either create the issues directly or add comments to the pull request with information necessary to create the issues (so we don't lose the info). 😄
There was a problem hiding this comment.
I think we will have to always load the assembly being decompiled from disk, because Roslyn will load them in "metadata-only mode" I guess.
As for the other references, could you tell me how to access the PEReader or Stream used by Roslyn? Thanks!
I would be glad to implement this correctly right from the start.
|
@siegfriedpammer It looks like the Mono.Cecil dependency needs to be removed at the same time due to the build validation. Do you mind if I add a commit to your pull request branch with the necessary change? |
|
I don't mind at all! 😄 Please go ahead. Thanks! |
|
@siegfriedpammer Done! Hopefully that fixes the build. |
|
I removed all TODO comments and added a summary to the PR description. Is there anything else to do? |
sharwell
added
Area-IDE
Community
|
|
||
| // Initialize a decompiler with default settings. | ||
| var decompiler = new CSharpDecompiler(assemblyDefinition.MainModule, new DecompilerSettings()); | ||
| var settings = new DecompilerSettings(LanguageVersion.Latest); | ||
| var decompiler = new CSharpDecompiler(pefile, new RoslynAssemblyResolver(compilation), settings); | ||
| // Escape invalid identifiers to prevent Roslyn from failing to parse the generated code. |
There was a problem hiding this comment.
Blank line between comment and the var decompiler = … line.
This pull request updates the ILSpy decompiler integration to version 4.0-beta1, which uses
System.Reflection.Metadatainstead ofMono.Cecilfor reading .NET metadata tables.It also includes a fix for bad decompilation results in .NET Standard/Core projects (You can see the effects of missing references in this video).
Points open for discussion (maybe create issues?):
StreamorPEReaderinstances. We should pass either theStreamor SRMPEReaderused by Roslyn to the decompiler to avoid loading referenced assemblies again just for decompilation.@sharwell feel free to rebase the pull request to any other branch you think this change should go to. 😃