Skip to content
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

[EE] Prettify compiler-generated field names with IDkmClrFullNameProvider.GetClrMemberName #50931

Conversation

r-ramesh
Copy link
Contributor

@r-ramesh r-ramesh commented Feb 1, 2021

Issue:

  • The debugger's NullReferenceException analysis feature figures out what was null through metadata and calls into the EE to display the offender as an expression.
  • If a field reference was null, we end up calling into GetClrMemberName.
  • However, for hoisted locals, etc we end up with an ugly mangled name like <myVar>5__1 was null.

Changes:

  1. Formatter:
  • Tweak implementation to handle compiler generated names.
  1. CSharpFormatter:
  • Add implementation for C#. VB is still TODO.
  • Split up GeneratedNames so that it can be included in the CSharpResultProvider.

Issue:
- The debugger's NullReferenceException analysis feature figures out what was null through metadata and calls into the EE to display the offender as an expression.
- If a field reference was null, we end up calling into GetClrMemberName.
- However, for hoisted locals, etc we end up with an ugly mangled name like <myVar>5__1 was null.

Changes:
1. Formatter:
- Tweak implementation to handle compiler generated names.

2. CSharpFormatter:
- Add implementation for C#. VB is still TODO.
- Split up GeneratedNames so that it can be included in the CSharpResultProvider.
@r-ramesh r-ramesh requested a review from a team as a code owner February 1, 2021 08:49
@dnfadmin
Copy link

dnfadmin commented Feb 1, 2021

CLA assistant check
All CLA requirements met.

@r-ramesh r-ramesh changed the title [EE] Prettify compiler-generated field names. [EE] Prettify compiler-generated field names with IDkmClrFullNameProvider.GetClrMemberName Feb 1, 2021
@r-ramesh
Copy link
Contributor Author

r-ramesh commented Feb 1, 2021

@tmat @ivanbasov Let me know your suggestions..


index = -1;
return false;
}
Copy link
Member

@cston cston Feb 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to move these methods to a separate file? Moving the methods makes it difficult to track changes.

It looks like it is necessary to move the methods to allow including this file in ResultProvider.csproj.

Comparing this file with the previous version, it looks like the only change is including IndexOfBalancedParenthesis().

/// <summary>
/// Parses a compiler-generated name and returns the simpler user-visible name.
/// </summary>
internal override string PrettifyCompilerGeneratedName(string name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps SimplifyCompilerGeneratedName().

@r-ramesh r-ramesh closed this Feb 2, 2021
@r-ramesh
Copy link
Contributor Author

r-ramesh commented Feb 2, 2021

@cston Discussing this a bit further with the team I think it's better to have a new Concord API that is eventually implemented in Roslyn rather than piggybacking and overloading an existing one. I've added to you to the Concord PR..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants