vadimcn / codelldb

A native debugger extension for VSCode based on LLDB
https://marketplace.visualstudio.com/items?itemName=vadimcn.vscode-lldb
MIT License
2.42k stars 237 forks source link

Use the Rust project's existing formatter #997

Open madsmtm opened 9 months ago

madsmtm commented 9 months ago

Rust already has built-in support for creating and enabling a "Rust" category with LLDB, see rust-lldb.

I think it'd make sense for CodeLLDB to defer to that in some way, both reduce the maintenance burden on your project, and to increase the quality of both rust-lldb and CodeLLDB. I can think of two ways to do so:

  1. Run rustc --print sysroot and find the LLDB files, like rust-lldb is doing now
  2. Link to the files from the Rust repo (a git SHA and a download script would be enough), and update them periodically

Option 1 would tie the debugging symbols to the current Rust version that the user is using, which is kinda nice since we'd automatically support all Rust versions, but also a bit troublesome since it's not ensured that makes the fact that python files are present is a part of Rust's stability promise. Also might be difficult to actually determine the Rust version that the user's binary was compiled with, not sure it's worth it.

Option 2 would be the easiest.


Note: I'll admit that Rust's builtin formatter is a bit lacking compared to your formatter, I'd volunteer to do the work of consolidating them if you would be on board with this?

vadimcn commented 9 months ago

Yeah, I've considered doing this... However, up until this got merged, vanilla lldb was lacking support for Rust enums, so I had to carry Rust-specific patches in my fork of lldb, and this was not compatible with rust-lldb formatters.

I'd volunteer to do the work of consolidating them if you would be on board with this?

I gotta warn you that I'm rather fond of my own implementation, so if by consolidating you mean basing implementation on rust-lldb formatters, I'll probably decline that patch.

Going in the other direction might also be met with pushback: the rust-lldb formatters do not handle peculiarities of the -windows-msvc debug info, since Rust devs expect those users to be using a debugger based on WinDbg.

davidbarsky commented 6 months ago

Vadim: I was speaking with Vladimir Makaev (the person who landed Rust enum support in lldb) and some folks who work on LLDB (Greg Clayton) about upstreaming your formatters into either lldb itself or rust-lldb. Two questions:

VladimirMakaev commented 6 months ago

@davidbarsky I'm happy to help implementing decoding for enums @vadimcn I was wondering is there any special Rust handling in your DAP server that is not provided by lldb-vscode?

vadimcn commented 6 months ago

@VladimirMakaev At the moment, CodeLLDB ships a custom version of liblldb, which includes Tom Tromey's implementation of TypeSystem for Rust. This allows LLDB to understand DWARF discriminated unions natively.

However, in the face of constant TypeSystem API changes, porting this patch forward has proven quite time-consuming, so I am considering dropping it in favor of just using your patch (with appropriate synthetic provider), even though this causes some regressions compared to the previous implementation (e.g. i32 type appearing in Rust code as "int", etc).

BTW, I had a thought in the process of implementing the new enum formatter: it'd be nice if Rust enum types were tagged with a specific keyword - this would allow matching them with a regex, instead of attaching a "wildcard" synth provider to all types and determining whether it's one of the enum types dynamically. Rustc does this for msvc ABI by changing type names to enum$<OriginalTypeName>. What do you think?

@davidbarsky I am not opposed in principle, but I am afraid you will find that my formatters use infrastructure that is quite different from the official one. This is in part due to the desire to share implementation with msvc formatters, as well as an attempt to reduce the overhead of Python code.

davidbarsky commented 6 months ago

@vadimcn

At the moment, CodeLLDB ships a custom version of liblldb, which includes Tom Tromey's implementation of TypeSystem for Rust.

Am I correct in thinking that this is where the bulk of the TypeSystem for Rust that you mentioned lives? If so, I recall @clayborg being pretty receptive to (and was, in fact, asking!) for a Rust TypeSystem implementation to live in-tree with LLDB. If you don't want to do it, I'm pretty sure that I (or @VladimirMakaev, if he'd like to!) would be happy to try porting over the TypeSystemRust.cpp to live in LLDB itself, which might reduce how much stuff you'd need to maintain and reduce the cost of API evolution, if refactors can be made inside of LLDB itself. Same offer is extended for the expression parser.

I am not opposed in principle, but I am afraid you will find that my formatters use infrastructure that is quite different from the official one.

Ah, gotcha. I didn't realize the scope of the changes, since I was able to successfully load of some of your synthetic formatters (e.g., for Path) into stock LLDB. On an aside, I didn't realize that a TypeSystem for Rust existed, which is really cool!

vadimcn commented 6 months ago

Am I correct in thinking that this is where the bulk of the TypeSystem for Rust that you mentioned lives?

Yeah, that's the one.

f so, I recall @clayborg being pretty receptive to (and was, in fact, asking!) for a Rust TypeSystem implementation to live in-tree with LLDB.

IIRC, he also wanted a firm promise to maintain and improve it afterwards. Personally, I don't have the bandwidth to take upon such a commitment, but if you feel like it, you're welcome to have a go.