Automatically update local variable debug info #676
Conversation
When manipulating the local variables collection on a method body, automatically update the debug info for all affected local variables. When removing a local variable, also remove the debug info for that local variable from the scopes. When removing or inserting update the indeces of local variable debug infos which are afected by the action. Note the local variable debug info either holds just a backpointer to the local variable in which case it doesn't store the index at all (and thus nothing to do), or it stores the index explicitly in which case it needs to be updated. Added tests for both insert and remove cases.
Mono.Cecil.Cil/MethodBody.cs
Outdated
| } | ||
|
|
||
| foreach (var variable in variables) { | ||
| if (variable.index.index.HasValue && variable.index.index.Value >= startIndex) { |
There was a problem hiding this comment.
Because this looks a bit odd, I think we want to comment as to why we're doing this. In that case my understanding is that you want to update the indexes of variables that have not been resolved into a VariableDefinition, while a VariableIndex can represent both the resolved and not resolved case.
There was a problem hiding this comment.
Hopefully the new version is more readable (and I also added a comment) - and it now also handles removal of resolved variable debug infos.
|
Thanks, this looks good! |
|
Looking at this PR and the other one, one thing that would be neat is to add a internal property to InstructionOffset and VariableIndex, like |
…pulate it. Reworked the `UpdateVariableIndeces` to only loop over variables once and also to handle removal of resolved variable debug infos.
|
I ended up adding two new internal properties (to both
|
|
@jbevain could you please let me know if the latest state of this PR and the friend in #677 is such that it will get merged sometime soon-ish? I'm trying to decide if we need to port it over to mono/cecil and use the fork there to ship the linker for .NET 5, or if we should keep using the cecil from this repo. (Without these fixes we are having trouble using the linker in the dotnet/runtime repo to build the libraries for .NET 5) |
Mono.Cecil.Cil/MethodBody.cs
Outdated
| return; | ||
|
|
||
| foreach (var scope in debug_info.GetScopes ()) { | ||
| if (scope.HasVariables) { |
There was a problem hiding this comment.
Please transform this condition into an early continue if there's no variable to reduce nesting.
|
@vitek-karas yes, please just address the nitpick and this is good to go. Thanks for the contribution! |
|
Thank you! |
When manipulating the local variables collection on a method body, automatically update the debug info for all affected local variables.
When removing a local variable, also remove the debug info for that local variable from the scopes.
When removing or inserting update the indices of local variable debug infos which are afected by the action. Note the local variable debug info either holds just a backpointer to the local variable in which case it doesn't store the index at all (and thus nothing to do), or it stores the index explicitly in which case it needs to be updated.
Added tests for both insert and remove cases.
Fixes #674
/cc @marek-safar