matklad / once_cell

Rust library for single assignment cells and lazy statics without macros
Apache License 2.0
1.87k stars 109 forks source link

Add Natvis definitions and tests for `once_cell` types #196

Closed ridwanabdillahi closed 1 year ago

ridwanabdillahi commented 2 years ago

This change adds Natvis visualizations for types in the once_cell crate to help improve the debugging experience on Windows.

Natvis is a framework that can be used to specify how types should be viewed under a supported debugger, such as the Windows debugger (WinDbg) and the Visual Studio debugger.

The Rust compiler does have Natvis support for some types, but this is limited to some of the core libraries and not supported for external crates.

https://github.com/rust-lang/rfcs/pull/3191 proposes adding support for embedding debugging visualizations such as Natvis in a Rust crate. This RFC has been approved, merged and implemented.

This PR adds:

matklad commented 2 years ago

Hm, I am a bit torn here. On the one hand, yeah, giving nicer results in the debugger is obviously good.

On the other hand, it feels like this pushes a lot of accidental complexity onto the library: running a windows build, running a windows build with a specific debugger which is not a part of rust-distribution, testing this via a proc-macro, testing this via a test which requires special fiddling to actually run.

I don't feel like I will be able to maintain this infrastructure without investing some significant time setting up the windows dev-environment and what not.

Can this functionality be provided by some other crate? Like, once-cell-debugvis or what not? If it could, I'd rather prefer that. Of course, author of the once-cell-debugvis would be on the hook for keeping it up-to-date with impl details of once_cell, but, given how fiddly the debugger setup looks, I indeed intentionally don't want once_cell authors to be on the hook for maintaining debugger infrastructure!


What I would be much more on-board with is maintaining some pure-rust, debugger independent descriptors, and leave the task of mapping those descriptors to natvis/gdb to someone else. So, something like this (spitballing here):

/// The crates which maps debugger-independent view into particular debuggres,
/// it shields downstream crates (like once_cell) from caring about gdb vs windbg, etc. 
extern crate debugvis;

/// Implement some trait which gives a "schema" of the type to use by debuggers,
/// a-la serde
impl debugvis::DebugVis for OnecCell {
  fn schema() -> debugvis::Schema {
     debugvis::Schema::new()
       .display("value")
       .child("queue")
       .child("marker")
       .child("value")
       .build()
  }
}

/// A test which is *somehow* debugger independent.
/// I don't know how that would work, but presumably, if a debugger can
/// use the data to show stuff, the program can do it itself, by lookin
/// at its own dwarf or whatnot? 
#[test]
fn test_debug_vis() {
  let cell = OnceCell::new();
  cell.set(92);
  debugvis::assert_vis!(cell, "
    92
      - value: 92
      - queue: []
      - marker: marker
  ");
}

/// Expands to `#[debugger_visualizer]`
debugvis::bundle!(OnceCell);

/// A test to generate the xml file.
/// Ideally, this would be build.rs, but we need access to OnceCell type / DebugVis impl,
/// so this needs to be a *post-*build script. We don't have those, but a test can
/// play this role. 
#[test]
fn generate_debugvis() {
  debugvis::generate::<OnceCell>("./debug_metadata/once_cell.natvis");
}
matklad commented 2 years ago

To clarify, I don't think we should change anything about the design of https://github.com/rust-lang/rfcs/pull/3191; rather, I'd love to see a higher-level infrastructure which "desugars" to that.

ridwanabdillahi commented 2 years ago

@matklad

To clarify, I don't think we should change anything about the design of rust-lang/rfcs#3191; rather, I'd love to see a higher-level infrastructure which "desugars" to that.

I understand the hesitation as this would be something that the once_cell maintainers would be on the hook to maintain. I have added testing to the CI to ensure nothing breaks silently and rots but as you suggested that would mean taking the effort to set up a windows dev environment to work on.

This really comes down to the benefits of enhanced debugger visualizations for the once_cell crate vs the maintenance aspect of said visualizations.

What I would be much more on-board with is maintaining some pure-rust, debugger independent descriptors, and leave the task of mapping those descriptors to natvis/gdb to someone else.

I'm not entirely sure how this would work. Debugger visualizations are pretty debugger specific, even the infrastructure used vary widely leading to different outputs for each debugger, i.e. Natvis for windows debugging and pretty printers (python scripts) for gdb/lldb.

matklad commented 2 years ago

I'm not entirely sure how this would work.

Me neither! But in the end of the day, the visualizations do seem similar? rendering value to string + providing access to children, based primarily on the structure of the type. So it seems in theory possible to have some abstract description of a type, and generate debugger-specific infra from that.

Otherwise, it's an MxN problem where each library has to support each debugger, which is also exacebrated by the fact that the testing infra for each one is pretty heavy weight.

ridwanabdillahi commented 2 years ago

So it seems in theory possible to have some abstract description of a type, and generate debugger-specific infra from that.

I believe this could be feasible but it's not what I was trying to add for this change.

Otherwise, it's an MxN problem where each library has to support each debugger, which is also exacebrated by the fact that the testing infra for each one is pretty heavy weight.

Yes, depending on the number of debuggers a crate tried to add support for, this problem space could grow.

Seeing as how this change is not what you would expect, or want to maintain in this crate should I close out this PR then?

matklad commented 1 year ago

Let's close this. I understand that this solves a real problem, but I also don't think explicit support for each debugger from each library would add up to a reasonable ecosystem in the long run.