StackTrace Explorer Implementation

[ryzngard]

Implementation for StackTrace Explorer for 17.1

Originally requested by #41557

Scope of this current PR

• Underlying parsing logic for StackTraces that we will support, including tests
• Addition of View > Other Windows > Stack Trace Explorer
• Add shortcut for Stack Trace Explorer, ctrl+E, ctr+S
• Add tool window initially with tabbing
• Add logic for navigating to source using a mix of FAR and GoToDef. FAR to find the symbol, GoToDef to navigate to it

Things not in scope:

• Full review of UI. Subject to another PR as we finalize details with UX
• Accessibility pass
• Determination for adding a columnal view of the data
• Generated name parsing (may already be handled, but is not verified)

#41557 will track remaining issues for completion and is not completed solely by this PR

DotNet StackFrames

Copied From Debugger (With Added Lines)

ActivityLog.xml

[tmat]

How do you handle stack frames from local functions, lambdas, async/iterator methods, etc.?

[ryzngard]

How do you handle stack frames from local functions, lambdas, async/iterator methods, etc.?

All good questions, and worth adding tests cases for. I think as long as the symbol finder can find the symbol based on the name output we should be fine. I'll add some instances of these and make sure they are handled gracefully.

[tmat]

I think as long as the symbol finder can find the symbol based on the name output we should be fine

It won't find the name since the names are mangled.

[ryzngard]

I think as long as the symbol finder can find the symbol based on the name output we should be fine

It won't find the name since the names are mangled.

Ah. Then there will be extra work needed to support that. Is it possible at all? Looking at sample output from local function it looks like we might could do it, but I'm not sure about everywhere.

at ConsoleApp4.MyOtherClass.<ThrowInLocalFunction>g__DoAThing|4_0() in C:\repos\ConsoleApp4\ConsoleApp4\Program.cs:line 71
  at ConsoleApp4.MyOtherClass.ThrowInLocalFunction() in C:\repos\ConsoleApp4\ConsoleApp4\Program.cs:line 67
  at ConsoleApp4.Program.Main(String[] args) in C:\repos\ConsoleApp4\ConsoleApp4\Program.cs:line 21

We could probably get close with:

Class: ConsoleApp4.MyOtherClass.
Method: <ThrowInLocalFunction>
Ignore: g__
LocalFunction: DoAThing
Ignore: |4_0()

Assuming the mangling is deterministic. Right now at least it does fail gracefully for these cases and doesn't show them as navigable.

[tmat]

Yes, it should be possible. We can internally link GeneratedNameParser.cs from the compiler and use the methods there to parse the names. We should definitely not implement a custom parsing logic of the synthesized names in the IDE or elsewhere.

[tmat]

We might need to make some changes to VB though - since we can't link GeneratedNameParser.vb to C# code. Perhaps the best would be if both of these parsers were moved to Microsoft.CodeAnalysis and written in C#.

[ryzngard]

We might need to make some changes to VB though - since we can't link GeneratedNameParser.vb to C# code. Perhaps the best would be if both of these parsers were moved to Microsoft.CodeAnalysis and written in C#.

Yea. Worst case we end up exposing something with MEF and link appropriately to each. It seems doable though, and I'm glad we have GeneratedNameParser to handle this

[tmat]

Worst case we end up exposing something with MEF and link appropriately to each

Good idea, I think that would work fine.

[jmarolf]

We might need to make some changes to VB though - since we can't link GeneratedNameParser.vb to C# code. Perhaps the best would be if both of these parsers were moved to Microsoft.CodeAnalysis and written in C#.

Yea. Worst case we end up exposing something with MEF and link appropriately to each. It seems doable though, and I'm glad we have GeneratedNameParser to handle this

This would be my preferred approach. expose a service and export it for both languages.

[Cosifne]

Retarget this to main because 17.1 branch is not used and main is 17.1 now

[davidwengier]

I'm clicking approve because I don't see why this can't progress forward as a POC or similar, and I don't want to block you.

At the same time, I would like to see more tests for some false positives (eg This,that.A,,,,,,,,,b() currently will successfully match the regex in TryParseMethodSignature) to ensure that the approach is as cautious and error resiliant as possible. Basically I worry that a big "Paste random stuff here" box in VS is going to be exception city :)

[davidwengier]

This is an odd method name, that implies a format (ie, type and file are always next to each other with nothing important in between).

Would it make more sense to have a collection of spans available (from the base class potentially?), in order, with an indication of what type of span it is (file, class, method, text, line number etc.)

This might be moot if this is changing to a grid in future or something.

[ryzngard]

Would it make more sense to have a collection of spans available (from the base class potentially?), in order, with an indication of what type of span it is (file, class, method, text, line number etc.)

I like this idea. Kind of like enumerating "classified" text instead of providing individual values. I'll think on this more...

The spans are already exposed, and if we add a grid I think we'll still want a pure text view and swap between the two

[ryzngard]

With the lexer/parser idea I think this becomes an even better argument to have. Instead of exposing spans it will likely expose concrete types that represent the part of the stack (fully qualified type, method, arguments, etc) and then map those to classification that we use in inlines.

[CyrusNajmabadi]

seems like TryParseMethodSignature could just return the frame if it succeeeded.

[CyrusNajmabadi]

so this is a major feature. i would strongly consider breaking this up into a feature branch, and hten breaking out components of that feature branch. i'm particularly interested in teh parsing portion (hardening and testing it thoroughly) and we could make that into it's own, easy-to-review PR indepednent of all the rest.

[CyrusNajmabadi]

Agreed. I would actually do this pr (or feature branch) in multiple parts. Everything related to parsing can be pulled out info another pr altogether. It will make things much easier to review as well.

[ryzngard]

Agreed. I would actually do this pr (or feature branch) in multiple parts. Everything related to parsing can be pulled out info another pr altogether. It will make things much easier to review as well.

Since it's an isolated feature I didn't do it in a separate branch. It allows for quick usage of the UI for cases that currently work while we improve upon the feature as a whole. What I'm hoping from this PR is that I've set up a good basis for people to get a broad scope idea of what the feature will be. The next PR will be the lexer/parser built for this (I assume that still belongs in embedded languages incase we ever want to highlight stack traces in code), symbol resolution improvements, and then more UI improvements (error messages, what to do if multiple symbols are found, etc).

[CyrusNajmabadi]

That's fine with me