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 support for embedding pretty printers via #[debugger_visualizer] attribute #97028

Merged
merged 3 commits into from
May 29, 2022

Conversation

ridwanabdillahi
Copy link
Contributor

Initial support for RFC 3191 in PR #91779 was scoped to supporting embedding NatVis files using a new attribute. This PR implements the pretty printer support as stated in the RFC mentioned above.

This change includes embedding pretty printers in the .debug_gdb_scripts just as the pretty printers for rustc are embedded today. Also added additional tests for embedded pretty printers. Additionally cleaned up error checking so all error checking is done up front regardless of the current target.

RFC: rust-lang/rfcs#3191

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 13, 2022
@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2022
@rust-log-analyzer

This comment has been minimized.

compiler/rustc_codegen_ssa/src/base.rs Outdated Show resolved Hide resolved
compiler/rustc_passes/src/check_attr.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/base.rs Outdated Show resolved Hide resolved
@givebk-bot
Copy link

Hey! Looks like someone invited me to this issue.

@antl3x
Copy link

antl3x commented May 14, 2022

@givebk-bot !give @ridwanabdillahi $1

thanks for your work!

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 17, 2022

☔ The latest upstream changes (presumably #96825) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@michaelwoerister michaelwoerister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @ridwanabdillahi! It looks very good already (it's great that it extends testing), but we'll need to resolve the issue around the linkage of __rustc_debug_gdb_scripts_section__ somehow (see below).

compiler/rustc_codegen_ssa/src/base.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/base.rs Outdated Show resolved Hide resolved
compiler/rustc_passes/src/check_attr.rs Show resolved Hide resolved
compiler/rustc_passes/src/check_attr.rs Outdated Show resolved Hide resolved
compiler/rustc_span/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/base.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

@michaelwoerister
Copy link
Member

That last commit still looks a bit funny 😀 Maybe the rebase didn't quite work.

…zer]` attribute. Add tests for embedding pretty printers and update documentation.

Ensure all error checking for `#[debugger_visualizer]` is done up front and not when the `debugger_visualizer` query is run.

Clean up potential ODR violations when embedding pretty printers into the `__rustc_debug_gdb_scripts_section__` section.

Respond to PR comments and update documentation.
Copy link
Member

@michaelwoerister michaelwoerister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @ridwanabdillahi, looks great!

Please address the error message issue mentioned below, then this should be ready to go.

compiler/rustc_passes/src/debugger_visualizer.rs Outdated Show resolved Hide resolved
@michaelwoerister
Copy link
Member

Thanks, @ridwanabdillahi! This looks good to me now.

@bors r+

@bors
Copy link
Contributor

bors commented May 27, 2022

📌 Commit 7ac62ce has been approved by michaelwoerister

@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 27, 2022
@ridwanabdillahi
Copy link
Contributor Author

@rustbot label +F-debugger_visualizer

@rustbot rustbot added the F-debugger_visualizer `#![feature(debugger_visualizer)]` label May 27, 2022
@ridwanabdillahi
Copy link
Contributor Author

Tracking issue for the feature being implemented by this PR: rust-lang/rust#95939

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 28, 2022
…ichaelwoerister

Add support for embedding pretty printers via `#[debugger_visualizer]` attribute

Initial support for [RFC 3191](rust-lang/rfcs#3191) in PR rust-lang#91779 was scoped to supporting embedding NatVis files using a new attribute. This PR implements the pretty printer support as stated in the RFC mentioned above.

This change includes embedding pretty printers in the `.debug_gdb_scripts` just as the pretty printers for rustc are embedded today. Also added additional tests for embedded pretty printers. Additionally cleaned up error checking so all error checking is done up front regardless of the current target.

RFC: rust-lang/rfcs#3191
bors added a commit to rust-lang-ci/rust that referenced this pull request May 29, 2022
…laumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#96950 (Add regression test for rust-lang#96395)
 - rust-lang#97028 (Add support for embedding pretty printers via `#[debugger_visualizer]` attribute)
 - rust-lang#97478 (Remove FIXME on `ExtCtxt::fn_decl()`)
 - rust-lang#97479 (Make some tests check-pass)
 - rust-lang#97482 (ptr::invalid is not equivalent to a int2ptr cast)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 239287f into rust-lang:master May 29, 2022
@rustbot rustbot added this to the 1.63.0 milestone May 29, 2022
@ridwanabdillahi ridwanabdillahi deleted the pretty-printer branch August 10, 2022 16:20
@givebk-bot
Copy link

givebk-bot commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-debugger_visualizer `#![feature(debugger_visualizer)]` 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
Development

Successfully merging this pull request may close these issues.

9 participants