Pretty-print `#[deprecated]` attribute in HIR. #138019

obi1kenobi · 2025-03-04T22:14:54Z

Pretty-print #[deprecated] attribute in a form closer to how it might appear in Rust source code, rather than using a Debug-like representation.

Consider the following Rust code:

#[deprecated]
pub struct PlainDeprecated;

#[deprecated = "here's why this is deprecated"]
pub struct DirectNote;

#[deprecated(since = "1.2.3", note = "here's why this is deprecated")]
pub struct SinceAndNote;

Here's the previous output:

#[attr="Deprecation{deprecation: Deprecation{since: Unspecifiednote:
suggestion: }span: }")]
struct PlainDeprecated;

#[attr="Deprecation{deprecation: Deprecation{since: Unspecifiednote:
here's why this is deprecatedsuggestion: }span: }")]
struct DirectNote;

#[attr="Deprecation{deprecation: Deprecation{since: NonStandard(1.2.3)note:
here's why this is deprecatedsuggestion: }span: }")]
struct SinceAndNote;

Here's the new output:

#[deprecated]
struct PlainDeprecated;

#[deprecated = "here's why this is deprecated"]
struct DirectNote;

#[deprecated(since = "1.2.3", note = "here's why this is deprecated"]
struct SinceAndNote;

Also includes a test for #[diagnostic::(..)] attributes, though their behavior is not changed here. I already wrote the test, so I figured it probably won't hurt to have it.

Related to discussion in #137645.

r? @jdonszelmann

rustbot · 2025-03-04T22:14:58Z

Failed to set assignee to jdonszelmann: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

aDotInTheVoid · 2025-03-04T23:04:15Z

This should have a test. See https://github.com/rust-lang/rust/blob/f9e0239a7bc813b4aceffc7f069f4797cde3175c/tests/ui/unpretty/debug-fmt-hir.rs (and https://github.com/rust-lang/rust/blob/master/tests/ui/unpretty/debug-fmt-hir.stdout) for an example.

obi1kenobi · 2025-03-05T01:50:30Z

I think I'll back out the diagnostic attribute changes. I wrote a test case that shows my code there isn't hit at all, and the diagnostic attributes are added to the unpretty output via some other mechanism.

The deprecated attribute change seems to work properly and I'm also adding a test that shows that.

Thanks for the pointer, @aDotInTheVoid!

obi1kenobi · 2025-03-05T01:56:28Z

Backed out the diagnostic attribute changes, added tests, and updated the PR description to match.

compiler-errors · 2025-03-05T02:34:26Z

compiler/rustc_hir_pretty/src/lib.rs

+                // If there's a `since` or `suggestion` value, we're definitely in form 1.
+                if matches!(
+                    deprecation.since,
+                    rustc_attr_parsing::DeprecatedSince::RustcVersion(..)


unrelated but it may be nice to import these directly, or at least via a shorter name like use rustc_attr_parsing as attr;

I have no opinion one way or the other, happy to make the change. My personal preference is also to import things, but I know some codebases are conservative with what gets imported so I was playing it safe :)

compiler-errors · 2025-03-05T02:35:10Z

LGTM, if jana wants any changes those can be done as a follow-up

r? compiler-errors @bors r+ rollup

bors · 2025-03-05T02:35:12Z

📌 Commit 55f8d3f has been approved by compiler-errors

It is now in the queue for this repository.

…kingjubilee Rollup of 15 pull requests Successful merges: - rust-lang#137829 (Stabilize [T]::split_off... methods) - rust-lang#137850 (Stabilize `box_uninit_write`) - rust-lang#137912 (Do not recover missing lifetime with random in-scope lifetime) - rust-lang#137913 (Allow struct field default values to reference struct's generics) - rust-lang#137923 (Simplify `<Postorder as Iterator>::size_hint`) - rust-lang#137949 (Update MSVC INSTALL.md instructions to recommend VS 2022 + recent Windows 10/11 SDK) - rust-lang#137963 (Add ``dyn`` keyword to `E0373` examples) - rust-lang#137975 (Remove unused `PpMode::needs_hir`) - rust-lang#137981 (rustdoc search: increase strictness of typechecking) - rust-lang#137986 (Fix some typos) - rust-lang#137991 (Add `avr-none` to SUMMARY.md and platform-support.md) - rust-lang#137993 (Remove obsolete comment from DeduceReadOnly) - rust-lang#137996 (Revert "compiler/rustc_data_structures/src/sync/worker_local.rs: delete "unsafe impl Sync"") - rust-lang#138019 (Pretty-print `#[deprecated]` attribute in HIR.) - rust-lang#138026 (Make CrateItem::body() function return an option) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138019 - obi1kenobi:pg/pretty-print-more-attrs, r=compiler-errors Pretty-print `#[deprecated]` attribute in HIR. Pretty-print `#[deprecated]` attribute in a form closer to how it might appear in Rust source code, rather than using a `Debug`-like representation. Consider the following Rust code: ```rust #[deprecated] pub struct PlainDeprecated; #[deprecated = "here's why this is deprecated"] pub struct DirectNote; #[deprecated(since = "1.2.3", note = "here's why this is deprecated")] pub struct SinceAndNote; ``` Here's the previous output: ``` #[attr="Deprecation{deprecation: Deprecation{since: Unspecifiednote: suggestion: }span: }")] struct PlainDeprecated; #[attr="Deprecation{deprecation: Deprecation{since: Unspecifiednote: here's why this is deprecatedsuggestion: }span: }")] struct DirectNote; #[attr="Deprecation{deprecation: Deprecation{since: NonStandard(1.2.3)note: here's why this is deprecatedsuggestion: }span: }")] struct SinceAndNote; ``` Here's the new output: ```rust #[deprecated] struct PlainDeprecated; #[deprecated = "here's why this is deprecated"] struct DirectNote; #[deprecated(since = "1.2.3", note = "here's why this is deprecated"] struct SinceAndNote; ``` Also includes a test for `#[diagnostic::(..)]` attributes, though their behavior is not changed here. I already wrote the test, so I figured it probably won't hurt to have it. Related to discussion in rust-lang#137645. r? `@jdonszelmann`

jdonszelmann · 2025-03-05T10:34:53Z

I honestly do not agree with this PR as is, and would have really liked to be able to give input on this. I probably wouldn't have approved it. I wanted to write a message explaining this longer but haven't had the chance yet. will still do later.

jdonszelmann · 2025-03-05T13:15:10Z

So if I were to have had a chance to review this, I would have said the following:

First of all, @aDotInTheVoid commented here that:

[pretty printing some attributes] Seems like a good approach for now, even if eventually we move to something more structured. It'd require careful explaining in the docs, but that's fine.

I agree with this, but don't see that addressed in this PR.

If anyone in the future would really like to parse hir pretty printing output (like cargo semver checks does now) they would probably much prefer the new output. I'm not saying that that is currently documented super well, and I'm working on changing that, but now we've just introduced an inconsistency, that though unstable, cargo semver checks will start relying on and which I'd like to remove again as soon as rustdoc json gains the ability to represent attributes correctly.

An alternative is of course that we do actually do pretty printing for all attributes, but then we'd probably need a bunch of helper methods around here because it's pretty clear that it'll be a bunch of code haha

What I would've proposed, is to make this change not here but only when we generate rustdoc json. In that way, normal hir pretty printing isn't affected and we don't need to revert anything. Then, when rustdoc json actually gains the ability to output attributes structurally only that codebase needs to be updated.

In that view, I'd actually say it'd be nice to undo these changes again and move some (similar) code into rustdoc json itself.

Finally, just a nitpick, I see that a test for diagnostic attrs stayed around even though the codebase doesn't touch that anymore. But I guess the extra test is always nice.

jdonszelmann · 2025-03-05T14:04:34Z

ok, talked some more with errs offline. Going to revert this. I think if we temporarly want this change, then it should happen in rustdoc json, not here. the pretty printing of deprecated does seem to contain bugs, but that's unrelated to the kind of pretty printing needed for rustdoc-json. I heard errs was looking into that one.

compiler-errors · 2025-03-05T14:06:18Z

Thanks for the context, @jdonszelmann. I was probably a bit too swift with approving this, so apologies from me :D

From my context, this is a fix for a somewhat major diagnostics regression (albeit in a perma-unstable nightly flag, -Zunpretty=hir) that should be fixed here somehow, but this probably requires way too much special casing if we are to maintain an approach like this for all parsed attrs. Generally unpretty=hir (and all pp_hir stuff) just needs to be readable, but not necessarily parser-roundtrippable (see how we render desugared ast for example).

Let's revert this and put up a more general but less pretty fix for the attr rendering itself (just so we're not dropping parentheses everywhere and stuff like that).

…ception in hir-pretty

obi1kenobi · 2025-03-05T16:18:36Z

I'd like to gently push back on this, because I think the distinction matters quite a bit:

parse hir pretty printing output (like cargo semver checks does now)

cargo-semver-checks does not parse HIR. cargo-semver-checks uses rustdoc JSON, which outputs an attrs list of attributes applied to each item. Under the hood, it appears rustdoc JSON was relying on HIR pretty-printing — but from cargo-semver-checks' point of view this is a rustdoc implementation detail.

cargo-semver-checks does not use -Zunpretty=hir or know anything at all about HIR. Its only touchpoint to HIR is via rustdoc JSON.

From my context, this is a fix for a somewhat major diagnostics regression (albeit in a perma-unstable nightly flag, -Zunpretty=hir) that should be fixed here somehow

This was also my reasoning, which is why I opened this PR. Completely ignoring cargo-semver-checks, I believe the new formatting output (despite the possible unescaped quote) to be massively better than the old one because it is more readable and more familiar.

Then, when rustdoc json actually gains the ability to output attributes structurally only that codebase needs to be updated.

As cargo-semver-checks maintainer, I'd love to have a more structured way to interact with attributes. In the case of #[deprecated], that structure actually does exist so in principle, we could even not fix how #[deprecated] appears in rustdoc JSON attrs.

But it isn't clear to me that rustdoc JSON is on a path to even have a design in mind for this anytime soon, let alone actually ship such an ability. Given the large number of projects finding value in cargo-semver-checks, I'm strongly concerned about any approach that requires breaking things for all those users today without clearly communicating a plan and timeline about fixing them.

To be clear, this isn't a HIR problem or a rustdoc problem or a cargo-semver-checks problem. It's an "all of us" problem, and a possible problem for everyone depending on cargo-semver-checks.

I'd like to work together on a way to minimize the disruption to all our users, because they are the same users :)

jdonszelmann · 2025-03-05T16:33:50Z

parse hir pretty printing output (like cargo semver checks does now)

I am aware cargo-semver checks doesn't parse hir pretty printed strings directly, but because that's what rustdoc json forwards you effectively are anyway.

That's exactly the reason why I'm saying what I'm saying. rustdoc-json shouldn't blindly rely on hir pretty printing and as such, I think the solution isn't to modify hir pretty printing. The temporary solution (for before rustdoc-json properly supports structured attributes) is to modify how rustdoc json prints these attributes. And that means that for some attributes rustdoc-json probably shouldn't directly call into hir pretty printing

obi1kenobi · 2025-03-05T16:45:53Z

100% agreed with that as the preferable resolution, apologies that wasn't clear from my previous message. If the snapshot tests from this PR are useful at least for purposes of recording the debug-style attribute printing in HIR, I'd be happy to pull them out into a separate PR without the behavior changes. That way it might be more obvious this format is intentional and not a bug or regression. Totally your call on it, I have no preference either way.

…

On Wed, Mar 5, 2025, 11:34 AM Jana Dönszelmann ***@***.***> wrote: parse hir pretty printing output (like cargo semver checks does now) I am aware cargo-semver checks doesn't parse hir pretty printed strings, but because that's what rustdoc json forwards you effectively are anyway. The only thing I'm saying is that I think that for that exact reason the solution isn't to modify hir pretty printing. The temporary solution (for before rustdoc-json *properly* supports structured attributes) is to modify how rustdoc json prints these attributes. And that means that for some attributes rustdoc-json probably shouldn't directly call into hir pretty printing — Reply to this email directly, view it on GitHub <#138019 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAR5MSV2H7PFFEPX5LUJ3Q32S4RQJAVCNFSM6AAAAABYKTH652VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMBRGQ3TANZVGM> . You are receiving this because you authored the thread.Message ID: ***@***.***> [image: jdonszelmann]*jdonszelmann* left a comment (rust-lang/rust#138019) <#138019 (comment)> parse hir pretty printing output (like cargo semver checks does now) I am aware cargo-semver checks doesn't parse hir pretty printed strings, but because that's what rustdoc json forwards you effectively are anyway. The only thing I'm saying is that I think that for that exact reason the solution isn't to modify hir pretty printing. The temporary solution (for before rustdoc-json *properly* supports structured attributes) is to modify how rustdoc json prints these attributes. And that means that for some attributes rustdoc-json probably shouldn't directly call into hir pretty printing — Reply to this email directly, view it on GitHub <#138019 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAR5MSV2H7PFFEPX5LUJ3Q32S4RQJAVCNFSM6AAAAABYKTH652VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMBRGQ3TANZVGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

jdonszelmann · 2025-03-05T16:46:42Z

Already pulled those out as part of the revert (#138060)

jdonszelmann · 2025-03-05T16:47:37Z

note, just to be clear, that that means that I'm on board with changes effectively equivalent to the ones from this PR. Just in a different place

…iler-errors Revert rust-lang#138019 after further discussion about how hir-pretty printing should work After some more discussion, rust-lang#138019 was probably merged a little fast. Though there probably is a real bug in pretty printing, it is not feasible to add similar pretty printing routines for all attributes, and making this specific exception is likely not desired either. For more context, see post-merge comments on rust-lang#138019 I kept the tests around, but reverted the hir-pretty change. r? `@compiler-errors`

…iler-errors Revert rust-lang#138019 after further discussion about how hir-pretty printing should work After some more discussion, rust-lang#138019 was probably merged a little fast. Though there probably is a real bug in pretty printing, it is not feasible to add similar pretty printing routines for all attributes, and making this specific exception is likely not desired either. For more context, see post-merge comments on rust-lang#138019 I kept the tests around, but reverted the hir-pretty change. r? ``@compiler-errors``

…iler-errors Revert rust-lang#138019 after further discussion about how hir-pretty printing should work After some more discussion, rust-lang#138019 was probably merged a little fast. Though there probably is a real bug in pretty printing, it is not feasible to add similar pretty printing routines for all attributes, and making this specific exception is likely not desired either. For more context, see post-merge comments on rust-lang#138019 I kept the tests around, but reverted the hir-pretty change. r? ```@compiler-errors```

…ception in hir-pretty

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#137674 (Enable `f16` for LoongArch) - rust-lang#138034 (library: Use `size_of` from the prelude instead of imported) - rust-lang#138060 (Revert rust-lang#138019 after further discussion about how hir-pretty printing should work) - rust-lang#138073 (Break critical edges in inline asm before code generation) - rust-lang#138107 (`librustdoc`: clippy fixes) - rust-lang#138111 (Use `default_field_values` for `rustc_errors::Context`, `rustc_session::config::NextSolverConfig` and `rustc_session::config::ErrorOutputType`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138060 - jdonszelmann:revert-138019, r=compiler-errors Revert rust-lang#138019 after further discussion about how hir-pretty printing should work After some more discussion, rust-lang#138019 was probably merged a little fast. Though there probably is a real bug in pretty printing, it is not feasible to add similar pretty printing routines for all attributes, and making this specific exception is likely not desired either. For more context, see post-merge comments on rust-lang#138019 I kept the tests around, but reverted the hir-pretty change. r? ```@compiler-errors```

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 4, 2025

obi1kenobi mentioned this pull request Mar 4, 2025

rustdoc-json: Improve Item::attrs to not use debug representation. #137645

Open

WaffleLapkin assigned jdonszelmann Mar 4, 2025

aDotInTheVoid added A-pretty Area: Pretty printing (including `-Z unpretty`) A-attributes Area: Attributes (`#[…]`, `#![…]`) labels Mar 4, 2025

Pretty-print #[deprecated] attribute in HIR.

55f8d3f

obi1kenobi force-pushed the pg/pretty-print-more-attrs branch from 3896004 to 55f8d3f Compare March 5, 2025 01:55

obi1kenobi changed the title ~~Pretty-print #[deprecated] and #[diagnostic::(..)] attributes.~~ Pretty-print #[deprecated] attribute in HIR. Mar 5, 2025

obi1kenobi marked this pull request as ready for review March 5, 2025 01:56

compiler-errors approved these changes Mar 5, 2025

View reviewed changes

compiler-errors reviewed Mar 5, 2025

View reviewed changes

rustbot assigned compiler-errors and unassigned jdonszelmann Mar 5, 2025

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 Mar 5, 2025

workingjubilee mentioned this pull request Mar 5, 2025

Rollup of 15 pull requests #138031

Merged

bors merged commit a69982e into rust-lang:master Mar 5, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 5, 2025

jdonszelmann added a commit to jdonszelmann/rust that referenced this pull request Mar 5, 2025

Revert rust-lang#138019 after further discussion about adding this ex…

4200f8e

…ception in hir-pretty

jdonszelmann mentioned this pull request Mar 5, 2025

Revert #138019 after further discussion about how hir-pretty printing should work #138060

Merged

jdonszelmann added a commit to jdonszelmann/rust that referenced this pull request Mar 5, 2025

Revert rust-lang#138019 after further discussion about adding this ex…

8391c08

…ception in hir-pretty

obi1kenobi deleted the pg/pretty-print-more-attrs branch March 5, 2025 16:18

jhpratt mentioned this pull request Mar 7, 2025

Rollup of 13 pull requests #138152

Closed

matthiaskrgr mentioned this pull request Mar 7, 2025

Rollup of 7 pull requests #138153

Closed

matthiaskrgr mentioned this pull request Mar 7, 2025

Rollup of 6 pull requests #138155

Merged

jdonszelmann added a commit to jdonszelmann/rust that referenced this pull request Mar 7, 2025

Revert rust-lang#138019 after further discussion about adding this ex…

1ba63c7

…ception in hir-pretty

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pretty-print `#[deprecated]` attribute in HIR. #138019

Pretty-print `#[deprecated]` attribute in HIR. #138019

obi1kenobi commented Mar 4, 2025 •

edited

Loading

rustbot commented Mar 4, 2025

aDotInTheVoid commented Mar 4, 2025

obi1kenobi commented Mar 5, 2025

obi1kenobi commented Mar 5, 2025

compiler-errors Mar 5, 2025

obi1kenobi Mar 5, 2025

compiler-errors commented Mar 5, 2025

bors commented Mar 5, 2025

jdonszelmann commented Mar 5, 2025

jdonszelmann commented Mar 5, 2025 •

edited

Loading

jdonszelmann commented Mar 5, 2025

compiler-errors commented Mar 5, 2025 •

edited

Loading

obi1kenobi commented Mar 5, 2025

jdonszelmann commented Mar 5, 2025 •

edited

Loading

obi1kenobi commented Mar 5, 2025 via email

jdonszelmann commented Mar 5, 2025 •

edited

Loading

jdonszelmann commented Mar 5, 2025

Pretty-print #[deprecated] attribute in HIR. #138019

Pretty-print #[deprecated] attribute in HIR. #138019

Conversation

obi1kenobi commented Mar 4, 2025 • edited Loading

rustbot commented Mar 4, 2025

aDotInTheVoid commented Mar 4, 2025

obi1kenobi commented Mar 5, 2025

obi1kenobi commented Mar 5, 2025

compiler-errors Mar 5, 2025

Choose a reason for hiding this comment

obi1kenobi Mar 5, 2025

Choose a reason for hiding this comment

compiler-errors commented Mar 5, 2025

bors commented Mar 5, 2025

jdonszelmann commented Mar 5, 2025

jdonszelmann commented Mar 5, 2025 • edited Loading

jdonszelmann commented Mar 5, 2025

compiler-errors commented Mar 5, 2025 • edited Loading

obi1kenobi commented Mar 5, 2025

jdonszelmann commented Mar 5, 2025 • edited Loading

obi1kenobi commented Mar 5, 2025 via email

jdonszelmann commented Mar 5, 2025 • edited Loading

jdonszelmann commented Mar 5, 2025

Pretty-print `#[deprecated]` attribute in HIR. #138019

Pretty-print `#[deprecated]` attribute in HIR. #138019

obi1kenobi commented Mar 4, 2025 •

edited

Loading

jdonszelmann commented Mar 5, 2025 •

edited

Loading

compiler-errors commented Mar 5, 2025 •

edited

Loading

jdonszelmann commented Mar 5, 2025 •

edited

Loading

jdonszelmann commented Mar 5, 2025 •

edited

Loading