11

New Rust attribute to support embedding debugger visualizers by ridwanabdillahi...

 2 years ago
source link: https://github.com/rust-lang/rfcs/pull/3191
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

Copy link

Contributor

@ridwanabdillahi ridwanabdillahi commented on Nov 3, 2021

edited by ehuss

This RFC adds support for a new Rust attribute that will embed a debugger visualizer into a PDB/ELF.

Internals thread

Rendered

ChayimFriedman2, roblabla, Kixiron, xTachyon, undersquire, rainydio, yoshuawuyts, sabbyX, slanterns, AsyncVoid, and 7 more reacted with thumbs up emoji Kixiron, gilescope, smmalis37, nicomem, nico-abram, xTachyon, undersquire, yoshuawuyts, Jasper-Bekkers, sabbyX, and 5 more reacted with heart emoji

I can totally see people wanting to derive natvis files using (proc) macros.

ridwanabdillahi, weihanglo, bluebear94, runiq, and Connicpu reacted with thumbs up emoji

ehuss

added T-cargo Relevant to the cargo subteam, which will review and decide on the RFC.

T-compiler Relevant to the compiler subteam, which will review and decide on the RFC.

labels

on Nov 4, 2021

Copy link

Contributor

Diggsey commented on Nov 6, 2021

I'm a bit wary of adding super platform-specific stuff to Cargo.toml (and this is coming from a windows user...)

I think it would be useful to look at prior art on other platforms, to see if there's some common denominator there.

It looks like natvis is also supported with GDB in vscode, although I don't know if there's any way to embed natvis files into the debug info in that case.

Idk, quite often it’s one way for windows and one way for unix systems. What matters is being able to have a better debug experience. If that means we have to do two things to cover all platforms that’s not too arduous.

joshtriplett and stepancheg reacted with thumbs up emoji

Copy link

Contributor

Author

ridwanabdillahi commented on Nov 8, 2021

edited

Idk, quite often it’s one way for windows and one way for unix systems. What matters is being able to have a better debug experience. If that means we have to do two things to cover all platforms that’s not too arduous.

I agree with this as well. Other platforms do not have a way to embed this kind of debug information automatically. VSCode uses MIEngine which only has a subset of the Natvis framework ported to it. This applies natvis to types within the debugger but the natvis is not embedded into the debug info. This means having to find the natvis for a specific version of a crate while debugging to get the correct behavior. This would put onus on the Rust developer who is consuming said crate to find the correct version of natvis for the version of the crate they are using. I believe having this solution for Windows is a great first step since these platforms are extremely different in how they support debugger visualizations.

Copy link

Member

nrc commented on Nov 15, 2021

So, I think it is a great idea to support natvis in an ergonomic way. Even though natvis is specific to certain platforms/debuggers and so is intrinsically not cross-platform, I think it is important for this RFC to consider other platforms so that we are sure that it is possible for crates to support multiple formats for different platforms, ideally using a single mechanism or very similar mechanisms (not the same visualisation format, but the same mechanism for Cargo to discover the visualisations and notify them to rustc, maybe).

On a lower level, how much does rustc need to do? Is it literally just passing filenames to the linker? Or does it need to do some symbol (de-)mangling? Integration with debuginfo support? How exactly will natvis files be saved in the metadata?

Finally, I much prefer the auto-discovery alternative. It seems much more inline with Cargo's 'convention over configuration' philosophy - we do not list source files, for example.

lebensterben and kangalioo reacted with thumbs up emoji

Copy link

Contributor

sivadeilra commented on Nov 15, 2021

I think it is important for this RFC to consider other platforms so that we are sure that it is possible for crates to support multiple formats for different platforms

Hi, @nrc. I'm one of the co-authors of this RFC. Can you be more specific about this request? The other visualization formats supported by other debuggers are quite different in how they are implemented, packaged, and used. This RFC doesn't prevent implementing support for those debugger visualizations. In fact, it proposes a filesystem schema specifically for avoiding collisions, by using .natvis file extension that are based on the root module filename for a given crate. This works for the primary crate, unit test binary, integration test binaries, etc.

If there is something more to do here, we are certainly open to that. To me, it appears that the main requirement is not to interfere with implementing support for other debug formats, and I think this proposal meets that requirement.

Because different debugger visualizers are so different in how they work, I don't see much of an opportunity to "use a single mechanism or very similar mechanism". Again, I'm open to that, but I just don't see any reusability here.

Is it literally just passing filenames to the linker?

Yes, that's pretty much all there is to it. The only complexity here lies in the requirement to propagate these filenames across the dependency graph. That is, when you invoke the linker, you need to pass the linker options for all crates in the dependency graph, not just the current crate.

In my original prototype implementation, I did this purely in Cargo, because Cargo knows the entire dependency graph and so it could walk it and find all of the NatVis files. However, when I discussed this design with some folks on the Cargo channel on Zulip, they felt that this was not a good approach because it would not work correctly for build system that were directly driving rustc, rather than using Cargo. They suggested the approach of bundling the NatVis files into metadata fields, so that the final rustc invocation could just inspect metadata, pull out the NatVis files, and pass them to the linker. That's what we propose to do in this RFC.

We also favor auto-discovery. However, like many options in Cargo, I think the developer should still be able to override auto-discovery.

Copy link

Member

nrc commented on Nov 16, 2021

Hi, @nrc. I'm one of the co-authors of this RFC. Can you be more specific about this request? The other visualization formats supported by other debuggers are quite different in how they are implemented, packaged, and used. This RFC doesn't prevent implementing support for those debugger visualizations. In fact, it proposes a filesystem schema specifically for avoiding collisions, by using .natvis file extension that are based on the root module filename for a given crate. This works for the primary crate, unit test binary, integration test binaries, etc.

Hi, so I'm personally not familiar with how any debuggers support visualisations, so it would be useful for me to have a short survey of how that works so that I can evaluate the similarities/differences myself. I assume that is also the case for others who want to evaluate this RFC too. If they are so different, then that would be enough.

Since I don't know the other mechanisms, I'm making a complete strawman here, but suppose that GDB takes a python file as a visualisation input, then that suggests that the Cargo manifest syntax should be something like

[debug-visualisations]
natvis = [...]
gdb-py = [...]

(again, totally making up the keywords as I go along). In that case, I would suggest this RFC propose this syntax (without the gdb case, leaving that for future work), rather than having natvis as a top-level key in the TOML file.

If there is something more to do here, we are certainly open to that. To me, it appears that the main requirement is not to interfere with implementing support for other debug formats, and I think this proposal meets that requirement.

I would say that not only must we not interfere, we should leave scope for supporting other formats in a clean way.

Because different debugger visualizers are so different in how they work, I don't see much of an opportunity to "use a single mechanism or very similar mechanism". Again, I'm open to that, but I just don't see any reusability here.

I'm not sure if it makes sense, but I hope the hypothetical example above makes clear the extent of reusability I'm imagining, it's not much of an iteration over the current state of the RFC.

Yes, that's pretty much all there is to it. The only complexity here lies in the requirement to propagate these filenames across the dependency graph. That is, when you invoke the linker, you need to pass the linker options for all crates in the dependency graph, not just the current crate.

That sounds fine. I think it would be nice to specify this in the RFC in a bit of detail so that we can evaluate the scope of changes being proposed

We also favor auto-discovery. However, like many options in Cargo, I think the developer should still be able to override auto-discovery.

Do you have an example of something you are following with this design? My intuition is that natvis files should be treated like source files, for which there is no mechanism to list them (not sure about overriding auto-discovery for source files, there is a mechanism in Rust source, but I'm not aware of a mechanism in Cargo).

Are there typically many natvis files in a project, or usually just one (or one per target)? My preference would be not to keep metadata in the src directory and thus have a separate directory for such files, but I guess I would feel less strongly if it were just one file. I'm not sure of any precedent for keeping metadata in src vs in a separate directory, but it would be good to know about any if there is any.

Copy link

Contributor

Author

ridwanabdillahi commented on Nov 16, 2021

In that case, I would suggest this RFC propose this syntax (without the gdb case, leaving that for future work), rather than having natvis as a top-level key in the TOML file.

This RFC initially suggested having the natvis key under the [package] section of the TOML file, which was one of the unresolved questions I had. Creating a new section for debugger visualizations sounds fair to me and would probably be more inclusive of future work to support other formats.

That sounds fine. I think it would be nice to specify this in the RFC in a bit of detail so that we can evaluate the scope of changes being proposed

I've explained at a fairly high level in the reference level explanation the scope of changes this RFC introduces in terms of new toml syntax and rustc flags as well as needing to make changes to the crate metadata. I will go into more detail in this section to show what specific changes need to be made and what fields need to be added to a CrateMetadata object.

Are there typically many natvis files in a project, or usually just one (or one per target)?

There is typically one Natvis file per library i.e. one file for a static lib and a separate for a shared DLL. This helps to break down where Natvis visualizations are defined for types. In the case of a Rust crate, you could imagine each crate having a single Natvis file that defines the debugger visualizations for types within that crate.

I'm not sure of any precedent for keeping metadata in src vs in a separate directory, but it would be good to know about any if there is any.

There is no reason as to why this would need to be kept in the src directory. Natvis files can be stored anywhere but they must be published as part of the crate. This way other crates that consume said crate would be able to pass the .natvis files into the linker.

Copy link

Contributor

sivadeilra commented on Nov 18, 2021

@nrc I like your suggested TOML structure, of:

[debug-visualisations]
natvis = [...]
gdb-py = [...]

About the question of where to put the files, and how many files there are. For typical C++ libraries, there is often just a single NatVis file, but it certainly does happen that a library is large enough to warrant breaking NatVis into more than one file, for the benefit of developers organizing things. So my expectation is that your typical crate will have just one, or a small number, of NatVis files.

When I wrote the first draft of this spec, I thought just having a ${crate_root}/dbg/natvis/*.natvis rule would be sufficient. However, if we want to allow a crate to have different sets of NatVis files for each unit that it produces (primary library, binaries, integration test binaries, etc.), then I think placing the NatVis files into the src tree makes sense. It also makes sense just for keeping the NatVis visualizations close to the module trees that they describe.

Or, we could simply say that the scenario of having different NatVis for different units is overkill, and that the same set of NatVis files should be used for all units in a given crate. That way, we just put them into ${crate_root}/dbg/natvis/*.natvis and be done with it. I like that approach, because it is simple and easy to remember and easy to implement, and because I can't actually think of a scenario where I would want different NatVis in different units.

Hi, so I'm personally not familiar with how any debuggers support visualisations, so it would be useful for me to have a short survey of how that works so that I can evaluate the similarities/differences myself. I assume that is also the case for others who want to evaluate this RFC too. If they are so different, then that would be enough.

I think that's a reasonable request, mainly because it will illustrate just how different they are. For GDB, it's a set of Python scripts (I think? might be wrong). For others (WinDbg), it's a native, compiled DLL that gets loaded into a debugger. WinDbg also supports JavaScript-based extension, too. Some of these can be linked into target binaries, or into their debug symbols (PDBs), but others have no convenient way to package the debugger extensions in a way that is discoverable by the debugger -- you just have to know how to find it.

Copy link

Member

nrc commented on Nov 18, 2021

However, if we want to allow a crate to have different sets of NatVis files for each unit that it produces (primary library, binaries, integration test binaries, etc.), ... and because I can't actually think of a scenario where I would want different NatVis in different units.

It sounds like it is worth digging in to this requirement. Is there an equivalent thing for C++/VS projects?

I know some projects have extensive support for testing infrastructure, and that might warrant separate debug metadata. However, one could also use a separate crate in workspace for this. I can't imagine want separate visualisations for most tests?

I can imagine a crate has both a library and binary targets and the latter has some data types (with visualisations) which are not in the library. Could this be handled with a single natvis file? Relatedly, if a crate has datatypes which are present or not depending on cfgs (e.g., Tokio), how would this be handled with natvis and this proposal?

Copy link

Contributor

sivadeilra commented on Nov 19, 2021

Is there an equivalent thing for C++/VS projects?

No, because in VS a single project produces a single output (EXE, DLL, LIB). There's no concept of "units".

On balance, I think we should start with the simpler model: For a given crate, the user defines a set of NatVis files. Those NatVis files are compiled into the primary crate unit (exe or rlib), and into the unit test crate unit. For benchmarks, integration tests, and bins, we don't include the NatVis files, because those compilations already reference the primary crate, which already contains the NatVis files.

I think this is best because:

  • It's simple for the user to reason about.
  • It still allows sophisticated developers to include definitions that are specific to their testing code. Yes, those definitions will be placed in the PDB file for the primary crate, which does mean that there's some test-only information in a non-test PDB. But I think that's OK -- it doesn't affect the primary crate itself, only its debugging info. It will be harmless, and will be minuscule in size, compared to the PDB itself.
  • It lets us use the simpler filesystem layout, which will be easy for developers to learn and use.

Relatedly, if a crate has datatypes which are present or not depending on cfgs (e.g., Tokio), how would this be handled with natvis and this proposal?

I think, for this V1, the NatVis file would contain definitions for the union of all features. As long as features are additive (and they are supposed to be), that should work fine. It means that some definitions may be ignored or useless when their corresponding features are not activated. I think that's OK.

When I originally drafted this RFC, I included two means to package NatVis into crates. The first is the simple "whole file" method, which is what is described here. The second was to allow users to include NatVis fragments directly into Rust source code, such as:

natvis!(r#"
    <Type Name="my_crate::MyFancyType">
      <DisplayString> ... </DisplayString>
      <Expand> ... </Expand>
      ...
    </Type>
"#);

Or, to provide it as an attribute on a type, so that the compiler could generate exactly the right type string, for matching descriptions to mangled types. This would free the developer from needing to write the <Type Name="..."> element. (Also, NatVis supports generic types, to an extent. Writing the <Type> parameters for generics is a little more tedious, so handling that for the developer would be a win.) Example:

#[natvis(xml = r#"
  <DisplayString> ... </DisplayString>
  <Expand> ... </Expand>
")]
pub struct MyFancyType { ... }

Rustc would then find all of these and assemble them into a single XML document, and bundle that into the PDB.

If we allow users to embed NatVis fragments directly into source code, then we can allow #[cfg] to control them, or to construct them using proc macros or macro_rules. This has lots of advantages, but we wanted to start with the simplest V1 that we could, because we felt that it would take a much longer time to come to consensus on what the V2 would look like.

So our plan is, stabilize the V1 support, and then to submit PRs to various crates, so that we can improve the debug experience of those crates. Then, when the value of this becomes apparent, we (with the community) can concurrently be working on spec'ing V2, with direct source embedding.

I posted one comment, but otherwise this looks good to me.

I think it's completely reasonable to do this in two phases, with the first requiring separate files and the second allowing inline attributes that get extracted.

Copy link

Contributor

Author

ridwanabdillahi commented on Nov 23, 2021

Hi @joshtriplett thanks for taking the time to review this RFC. The goal of this RFC is to see stabilization of this feature at some point in time. As such, I was wondering if the flags I have suggested in the RFC are appropriate. There's a couple of ways this could be achieved.

  1. As described in the RFC, keep the -Z natvis={comma-separated list} flag at which point it would eventually get turned into a -C flag for stabilization.
  2. Create a -C natvis={comma-separated list} flag with a -Z enable-natvis flag as well which would guard against using the -C natvis flag in stable Rust.

Thoughts?

@ridwanabdillahi I personally would propose starting with -C natvis, and gating it with the existing -Z unstable-options; that would make a future stabilization easier. But there may be a reason I'm not aware of for why that isn't a good idea.

Copy link

Contributor

Author

ridwanabdillahi commented on Nov 24, 2021

I personally would propose starting with -C natvis, and gating it with the existing -Z unstable-options;

@joshtriplett that sounds good to me. I'll update the RFC to reflect as such.

Copy link

Contributor

Author

ridwanabdillahi commented on Dec 8, 2021

I believe I've responded to all of the comments, is there anything else that needs to be addressed in this RFC?

nrc reacted with eyes emoji

Copy link

Member

@nrc nrc left a comment

Looks good! Thanks for addressing all my earlier comments. I have a few more inline, but nothing huge.

The one 'big' thing is that I still think the "Auto-discover Natvis XML files" option is more idiomatic for Cargo then explicitly listing all the natvis files, but we can leave that up to Cargo team review.

text/0000-natvis.md

Outdated Show resolved

text/0000-natvis.md

Outdated Show resolved

text/0000-natvis.md

Outdated Show resolved

text/0000-natvis.md

Outdated Show resolved

text/0000-natvis.md

Outdated Show resolved

Copy link

Member

nrc commented on Dec 9, 2021

In response to #3191 (comment) (and just general pondering). I wonder if it is worth thinking a bit about integration with Cargo features at least (and perhaps other sources of cfg input such as the target platform). I don't want to get too much into discussion of what v2 would look like, but I have a strong preference to avoid inlining the natvis info and keeping it in separate files. Given this possibility, I want to make sure there is scope in the proposal to extend it to including different natvis depending on features, etc. We don't have to go into too much detail, obviously.

Could we imagine extending the Cargo manifest entry to include features, etc. in the same way as dependencies? I think it should be easy enough some how, and perhaps this is a good justification for keeping the explicit filename list rather than auto-discovery?

Copy link

Member

nrc commented on Dec 9, 2021

One more thought, on toolchains which don't support adding natvis into the PDB during linking, should Cargo or rustc emit all the specified natvis files into a location in the target directory so that they can be easily accessed by the user?

Copy link

Contributor

Author

ridwanabdillahi commented on Mar 14

That's incorrect in general case, and quite different from how mod items are loaded.

I see. I currently have a draft PR out for an implementation of this RFC rust-lang/rust#91779. I'll reach out to you via zulip to better understand the extent of this problem.

Copy link

Contributor

petrochenkov commented on Mar 15

I'll reach out to you via zulip to better understand the extent of this problem.

The conclusion was that it's some small scale implementation annoyance due to resolving filesystem paths from attributes being a novel mechanism previously unused in the language, but it's not a blocker.

Copy link

Contributor

nikomatsakis commented 26 days ago

@rfcbot fcp reviewed

rfcbot

added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised.

and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period.

labels

25 days ago

Copy link

rfcbot commented 25 days ago

bellThis is now entering its final comment period, as per the review above. bell

Copy link

Member

nagisa commented 25 days ago

These nitpicks from mainly an implementer's perspective.


Through the use of a new Rust attribute, #[debugger_visualizer], the compiler will
encode the contents of the .natvis file in the crate metadata if the target is an rlib. If the target is a dll or exe, the /NATVIS MSVC linker flag is set for each .natvis file which will embed the Natvis visualizations into the PDB.

the compiler will encode the contents of the .py file in the crate metadata if the target is an rlib

The Rust compiler will serialize the contents of the file specified via the #[debugger_visualizer] attribute and store it in the crate metadata.

Returning to the metadata encoding within rlibs discussion. I don't believe metadata is a great place to put bundled files. rlib is already an archive that bundles a number of files, and placing the natviz files into the archive itself would make much more sense to me. At the very least from the perspective of it enabling use of standard tooling to inspect the contents.

As far as the pretty-printers are concerned the toolchain might as well avoid the work of moving the data around across compilations all the time by producing an object with an concatenated .debug_gdb_scripts section in the first place. Once that's done rustc no longer needs to think about the upstream pretty printers at all, anymore.

I guess overall my impression is that this wording is over-constraining the implementation-time decisions here. None of these guarantees for where data is placed are required for an implementation of this feature, nor are these the guarantees we want to make about the structure of the rlibs, I feel. As long as rustc can guarantee that the contents of these are bundled and preserved throughout compilation, where exactly within the rlib these contents are stored shouldn't matter.


To provide Natvis files, developers create a file with the .natvis file extension.

To provide pretty printers, developers create a file with the .py file extension

Why the extension restriction? The type of the file is already specified by the attribute. If there's a reason the file extension must be exactly natvis or py, the RFC should specify what happens (error, warning, ignore the attribute?) when it is not (IIRC it does not).


This attribute will directly target modules which means the syntax #![debugger_visualizer] is also valid when placed at the module level.

AFAIK crate is a distinct kind of entity and is not a module item. This wording in turn disallows application of the #![debugger_visualizer] to the crate itself, which is something the examples do.

More generally I would like a separate paragraph listing the exact list of the kinds of entities that this attribute is applicable to (crate, module item, anything else?) and a specification of what happens when this attribute is applied to anything else (error, warning, ignore?)

Finally, hand-waving a bit, but I'm not entirely sure if allowing this attribute at a module level today is the best idea. At the very least allowing something like this now immediately blocks any potential for future improvements to the attribute's behaviour. For example, I could see rustc automatically doing the work to scope the activation of the rules within the natviz/pretty-printer to the types contained within the module in question or something.


Sorry if I missed answers to any of these in prior discussion. I did try grepping through the page, but the discussion is definitely too long for me to catch up on properly.

scottmcm reacted with thumbs up emoji scottmcm reacted with heart emoji

Copy link

Contributor

Author

ridwanabdillahi commented 23 days ago

edited

@nagisa Thank you so much for taking the time to provide feedback.

As far as the pretty-printers are concerned the toolchain might as well avoid the work of moving the data around across compilations all the time by producing an object with an concatenated .debug_gdb_scripts section in the first place. Once that's done rustc no longer needs to think about the upstream pretty printers at all, anymore.

This is very interesting, in terms of the RFC I will remove any specifics as to how or when rustc will package the pretty printers into the .debug_gdb_scripts section. This will allow for flexibility in terms of the implementation of this feature.

I guess overall my impression is that this wording is over-constraining the implementation-time decisions here. None of these guarantees for where data is placed are required for an implementation of this feature, nor are these the guarantees we want to make about the structure of the rlibs, I feel. As long as rustc can guarantee that the contents of these are bundled and preserved throughout compilation, where exactly within the rlib these contents are stored shouldn't matter.

With this being my first RFC I believe I was too specific in the implementation details. I'll peel back a bit in terms of the implementation-time decisions and keep it at more of a high level.

Why the extension restriction? The type of the file is already specified by the attribute. If there's a reason the file extension must be exactly natvis or py, the RFC should specify what happens (error, warning, ignore the attribute?) when it is not (IIRC it does not).

I can't speak to any extension restriction for pretty printers, but in terms of Natvis, the MSVC linker flag /NATVIS: currently accepts a file with any file extension but the debuggers (WinDbg, VS Debugger) are unable to load these files into the debugging session automatically when the file extension is not .natvis. However, WinDbg does support directly loading a natvis file with any extension with the .nvload command inside the debugger. This sadly takes away from the point of this RFC that Rust developers shouldn't need to manuallly load anything for Natvis visualizations to apply. This is only mentioned in the RFC as an example for how to create these files, there is no error or warning if the files have a different extension since neither the linker or the debuggers throw an error.

More generally I would like a separate paragraph listing the exact list of the kinds of entities that this attribute is applicable to (crate, module item, anything else?) and a specification of what happens when this attribute is applied to anything else (error, warning, ignore?)

Currently this attribute is only applicable to module items and at the crate-level. I will update this so that this attribute is only applicable at the crate-level and will throw an error if used any where else.

Finally, hand-waving a bit, but I'm not entirely sure if allowing this attribute at a module level today is the best idea. At the very least allowing something like this now immediately blocks any potential for future improvements to the attribute's behaviour. For example, I could see rustc automatically doing the work to scope the activation of the rules within the natviz/pretty-printer to the types contained within the module in question or something.

Currently the RFC mentions that this attribute can be allowed to target module items as well as being a crate-level attribute which I understand are a distinct kind of entity in terms of how they are applied. @petrochenkov also suggested making this attribute specifically a crate-level attribute which sounds like the right direction to go. I think this will resolve the concern you have mentioned here in terms of this attribute targeting module items.

Copy link

Contributor

Author

ridwanabdillahi commented 23 days ago

edited

@nagisa

Why the extension restriction? The type of the file is already specified by the attribute. If there's a reason the file extension must be exactly natvis or py, the RFC should specify what happens (error, warning, ignore the attribute?) when it is not (IIRC it does not).

The more I think about it, this issue is moot. Since the contents of this file is embedded in the crate metadata and written out to a temp file with the correct .natvis extension in the case of a natvis file, there should not be any issues with loading these debugger visualizer files in either WinDbg or the VS debugger. Thank you for catching this, I'll update the RFC to remove any reference to specific file extensions.

Copy link

Contributor

Author

ridwanabdillahi commented 23 days ago

I'm worried that moving in the direction of limiting the #[debugger_visualizer] attribute to the crate-level would only make it more difficult for use by Rust developers. In the case of a crate with multiple modules, having Natvis files next to the .rs file which defines the type that the visualizations work with would be extremely valuable. This would also help with avoiding Natvis files from becoming stale when types deep within the module system are changed/updated. Being able to add the #[debugger visualizer] attribute at the module level would extremely simplify the paths specified on the attributes themselves as opposed to forcing users to place this attribute solely at the crate-level.

This also would work well with crates that turn off entire modules depending on which features have been activated. If a cfg feature removes a module from being active, then any of the #![debugger_visualizer] attributes used within this module can be dropped which means none of the debugger visualizers would be embedded into the binary. I think this would be great that way we aren't adding debugger visualizers for types that aren't going to exist anyway.

For example, I could see rustc automatically doing the work to scope the activation of the rules within the natviz/pretty-printer to the types contained within the module in question or something.

I'm not sure I understand why this would need to happen? If a debugger visualization exists for a given type, it would be applied once that type is used. I don't see why we would want to enable/disable a visualization for a type especially since the debugger visualizations are directly embedded into a binary.

Copy link

Member

nagisa commented 23 days ago

edited

I'm not sure I understand why this would need to happen? If a debugger visualization exists for a given type, it would be applied once that type is used. I don't see why we would want to enable/disable a visualization for a type especially since the debugger visualizations are directly embedded into a binary.

Well, the thing is that there can be multiple types that share the same name, so it seems quite natural to me that if I have something along the lines of…

mod a {
    #![debugger_visualizer="a.natviz"]
    struct Mango { ... };
}

mod b {
    #![debugger_visualizer="b.natviz"]
    struct Mango { ... };
}

then I can write the natviz like

<!-- a.natviz -->
<?xml version="1.0" encoding="utf-8"?>
<AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
  <Type Name="Mango">
    .
    .
  </Type>

</AutoVisualizer>

<!-- b.natviz -->
<?xml version="1.0" encoding="utf-8"?>
<AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
  <Type Name="Mango">
    .
    .
  </Type>

</AutoVisualizer>

and not have to worry about which visualization would end up getting applied where.

This is also a concern between crates, of course, but it feels to me like “use paths reachable from crate root” is a quite straightforward rule to follow, and everything more than that is, honestly, sounds like a significant burden by itself – a much bigger one than having to specify slightly longer paths.

I'm not necessarily saying that some sort of automatic processing of the files is the best approach. But I would at least like to see an explicit decision that this is something we definitely do not want to see now or in the future. And if we aren't willing to make such call today, we should stay a little conservative as to where we allow the attribute to appear for the time being.

Does that help to clarify my line of thought?

Copy link

Contributor

Author

ridwanabdillahi commented 22 days ago

Well, the thing is that there can be multiple types that share the same name, so it seems quite natural to me that if I have something along the lines of…

Thank you for clarifying your line of thought. In the case mentioned above this actually would not be an issue since the names generated for those types would actually be distinct. For the example above, the fully qualified names for these types for some crate foo would be foo::a::Mango and foo::b::Mango. The Natvis visualization would match against the fully qualified type names and therefore they would be unique and would not clash. The same would be true for types with the same name in different crates.

But I would at least like to see an explicit decision that this is something we definitely do not want to see now or in the future.

I believe for this specific attribute this is not something that we want to allow. In the future goals section, there is a mention about having support for defining Natvis visualizations or pretty printers "inline". Inline meaning an attribute or a proc-macro would take a snippet of a visualization schema and create the boilerplate needed around it and embed this in the binary as well.

This could potentially use the same attribute which would mean updating the attribute to allow for targeting types as well and using a different meta-item to switch on which target is allowed. Is this enough of an explicit decision for what is acceptable now and what can be added later?

In either case we would not want any automatic processing of the files by rustc.

Copy link

Contributor

tmandry commented 16 days ago

How do the embedding protocols provide for their own versioning and evolution?

For example, what if GDB came out with a new major version of its scripting interface, and your library crate wanted to provide scripts for v1 and v2 respectively. How would you go about this? cargo doesn't support mutually exclusive features, nor does the build necessarily have control over what debugger the binary gets run in, so it seems like we would have to provide for embedding both and annotate with a version somehow.

Related question: These are considered debug sections and would get stripped out when the strip option is enabled, correct?

rfcbot

added finished-final-comment-period The final comment period is finished for this RFC.

to-announce

and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised.

labels

15 days ago

Copy link

rfcbot commented 15 days ago

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

Copy link

Contributor

pickfire commented 13 days ago

edited

I am concern about having needed to maintain multiple separate implementation for debug, say if I use one platform and some contributors contributed another platform debugging visualizers, then I had to maintain both even though I don't have the access to it, when there are breaking changes, chances is that it might be dropped or broken due to limited maintenance capability on a small crate.

Copy link

Contributor

Author

ridwanabdillahi commented 13 days ago

@tmandry

How do the embedding protocols provide for their own versioning and evolution?

If this case does arise, which is totally plausible, then the attribute can always be updated to include a version meta-item or something similar to ensure the correct visualizer is added but that is not a goal of this RFC.

Related question: These are considered debug sections and would get stripped out when the strip option is enabled, correct?

Yes, this is correct. This attribute will work in the same manner as how the compiler handles debugger visualizers today. For both the Natvis files and GDB scripts, if the strip option is enabled then the visualizers will not be embedded in the generated PDB/ELF.

@pickfire

I am concern about having needed to maintain multiple separate implementation for debug, say if I use one platform and some contributors contributed another platform debugging visualizers, then I had to maintain both even though I don't have the access to it, when there are breaking changes, chances is that it might be dropped or broken due to limited maintenance capability on a small crate.

As with any crate, the maintainer of the crate would be responsible for ensuring the visualizers do not become stale. Currently the Rust compiler has tests which test the visualizers for parts of the standard library such as std, core and alloc against the debuggers that those visualizers target. For example: numeric-types.rs and pretty-huge-vec.rs

For Windows debuggers at least, the raw visualization for types can be displayed in the case of a broken Natvis visualization as a workaround.

pickfire reacted with eyes emoji

Copy link

Member

wesleywiser commented 6 days ago

The Compiler and Lang Teams have accepted this RFC! tada

Please see the tracking issue for further discussion: rust-lang/rust#95939

Copy link

lostmsu commented 3 days ago

Just noticed this RFC. Does it support using a custom function declared on the type to be called to produce the value? The scenario is a Rust type, that wraps a handle + group of accessor functions. E.g.

struct File {
  file_handle: isize,
}

impl Tensor {
  pub fn name(&self) -> String {
     let raw_name = winapi.GetFileName(self.file_handle);
     ...
}

Copy link

Contributor

Author

ridwanabdillahi commented 3 days ago

@lostmsu

Just noticed this RFC. Does it support using a custom function declared on the type to be called to produce the value?

No this scenario is not supported. Currently the supported scenario is having standalone visualizer files, Natvis or pretty printers, and using the attribute proposed to include those files directly. It does not support dynamically generating the visualization descriptions.


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK