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

Add Natvis definitions and tests for once_cell types #196

Closed
wants to merge 1 commit into from

Conversation

ridwanabdillahi
Copy link

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.

rust-lang/rfcs#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:

  • Natvis visualizations for once_cell types.
  • Tests for testing visualizers embedded in the once_cell crate.
  • Updates to the CI pipeline to ensure tests for visualizers are run so they do not break silently.
  • Add testing against Windows using the nightly toolchain
  • A new debugger_visualizer feature for the once_cell crate to enable the unstable debugger_visualizer Rust feature.

@matklad
Copy link
Owner

matklad commented Sep 15, 2022

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
Copy link
Owner

matklad commented Sep 15, 2022

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.

@ridwanabdillahi
Copy link
Author

@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
Copy link
Owner

matklad commented Sep 16, 2022

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
Copy link
Author

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
Copy link
Owner

matklad commented Jun 4, 2023

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.

@matklad matklad closed this Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants