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.

Sign up for GitHub

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

Jump to bottom

Stabilize debugger_visualizer #108668

Merged
merged 4 commits into from
May 2, 2023
Merged

Stabilize debugger_visualizer #108668

merged 4 commits into from
May 2, 2023

Conversation

gibbyfree
Copy link

@gibbyfree gibbyfree commented Mar 2, 2023 •
edited by wesleywiser
Loading

This stabilizes the debugger_visualizer attribute (#95939).

  • Marks the debugger_visualizer feature as accepted.
  • Marks the debugger_visualizer attribute as ungated.
  • Deletes feature gate test, removes feature gate from other tests.

Closes #95939

@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 2, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Mar 2, 2023

Do we have any documentation for this feature yet?

The tracking issue links to https://rustc-dev-guide.rust-lang.org/stabilization_guide.html#documentation-prs for the next step.

cc @wesleywiser what's the status of this feature? Any unresolved questions still open?

@gibbyfree
Copy link
Author

Do we have any documentation for this feature yet?

I created a PR for documentation here: rust-lang/reference#1335 I'll be removing the 'draft' state shortly.

@gibbyfree gibbyfree marked this pull request as ready for review March 6, 2023 20:26
@gibbyfree gibbyfree mentioned this pull request Mar 6, 2023
Closed
5 tasks
@nyurik
Copy link
Contributor

nyurik commented Mar 24, 2023

I keep reading the RFC & the docs, and I keep thinking that we can do much better than natvis.

I have written extensive debugger visualizers back in my C# days, where the visualizations had to be done mostly as code. If we adapt natvis, many modern Rust developers will have a major XML cringe every time they would think about it - and that just means far fewer implementations.

I don't mind XML being stored inside PDBs - out of sight, out of mind, but the developers implementing visualizations should not be aware of it / write it / store it in a repo. Instead, I believe we should provide absolute minimum needed support in the compiler, and let crates define the needed attributes/macros to generate (natvis?) internally.

Or is this the intention all along?

@gibbyfree
Copy link
Author

@nyurik Do you see writing GDB pretty printers as a reasonable alternative to Natvis? This might be the preferred means of embedding visualizations for crate owners less familiar with Natvis.

The intention of this feature is to allow crate owners to specify their own visualizations (Natvis if they'd like to support WinDbg/Visual Studio users, Python if they'd like to support GDB users) to be embedded in their crate's debug binary. This allows crate consumers to visualize a crate's custom types without any manual configuration on their part. This achieves "out of sight, out of mind" from the end-user's perspective.

As far as supporting visualization development via macros or some other form of source embedding, I think this falls under "V2" of debugger_visualizer. This extension would probably require a new set of design discussions. (See rust-lang/rfcs#3191 (comment) for a bit more info.) By stabilizing the current state of debugger_visualizer, the feature can begin adding value to the community while possible extensions are specced out. We have already contributed Natvis definitions to some popular crates (servo/rust-smallvec#286 and Lokathor/tinyvec#167, for example).

@wesleywiser
Copy link
Member

Hi @gibbyfree, would you mind rebasing this PR and removing the merge commits? We prefer not to have merge commits in the PR history as it makes the final merge history messier.

Feel free to ping me if git rebase gives you trouble!

@gibbyfree gibbyfree force-pushed the stabilizedebuggervisualizer branch from 0b2cfc5 to c9653a6 Compare May 1, 2023 20:39
@wesleywiser
Copy link
Member

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented May 1, 2023

📌 Commit c9653a6 has been approved by wesleywiser

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 1, 2023
@matthiaskrgr
Rollup merge of rust-lang#108668 - gibbyfree:stabilizedebuggervisuali…
dcef3b7 
…zer, r=wesleywiser

Stabilize debugger_visualizer

This stabilizes the `debugger_visualizer` attribute (rust-lang#95939).

* Marks the `debugger_visualizer` feature as `accepted`.
* Marks the `debugger_visualizer` attribute as `ungated`.
* Deletes feature gate test, removes feature gate from other tests.

Closes rust-lang#95939
@matthiaskrgr matthiaskrgr mentioned this pull request May 1, 2023
Closed
@Dylan-DPC Dylan-DPC mentioned this pull request May 2, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2023
@bors
Auto merge of rust-lang#111089 - Dylan-DPC:rollup-b8oj6du, r=Dylan-DPC
7b99493 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#105076 (Refactor core::char::EscapeDefault and co. structures)
 - rust-lang#108161 (Add `ConstParamTy` trait)
 - rust-lang#108668 (Stabilize debugger_visualizer)
 - rust-lang#110512 (Fix elaboration with associated type bounds)
 - rust-lang#110895 (Remove `all` in target_thread_local cfg)
 - rust-lang#110955 (uplift `clippy::clone_double_ref` as `suspicious_double_ref_op`)
 - rust-lang#111048 (Mark`feature(return_position_impl_trait_in_trait)` and`feature(async_fn_in_trait)` as not incomplete)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f379a58 into rust-lang:master May 2, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 2, 2023
@wesleywiser wesleywiser added the relnotes Marks issues that should be documented in the release notes of the next release. label May 2, 2023
@cuviper cuviper mentioned this pull request Jun 27, 2023
Open
@emmanuel-ferdman emmanuel-ferdman mentioned this pull request Aug 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dpaoliello dpaoliello dpaoliello left review comments

Assignees

@oli-obk oli-obk

Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.

Tracking Issue for debugger_visualizer
7 participants
@gibbyfree @rustbot @oli-obk @nyurik @wesleywiser @bors @dpaoliello