Port `doc` attributes to new attribute API #149645

GuillaumeGomez · 2025-12-04T14:56:14Z

This PR ports the doc attributes to the new attribute API. However, there are things that will need to be fixed in a follow-up:

Some part of cfg_old.rs are likely unused now, so they should be removed.
Not all error/lints are emitted at the same time anymore, making them kinda less useful considering that you need to run and fix rustc/rustdoc multiple times to get through all of them.
For coherency with the other attribute errors, I didn't modify the default output too much, meaning that we have some new messages now. I'll likely come back to that to check if the previous ones were better in a case-by-case approach.
doc(test(attr(...))) is handled in a horrifying manner currently. Until we can handle it correctly with the Attribute system, it'll remain that thing we're all very ashamed of. 😈
A type in rustdoc got its size increased, I'll check the impact on performance. But in any case, I plan to improve it in a follow-up so should be "ok".
Because of error reporting, some fields of Doc are suboptimal, like inline which instead of being an Option is a ThinVec because we report the error later on. Part of the things I'm not super happy about but can be postponed to future me.

And finally, once this PR is merged, I plan to finally stabilize doc_cfg feature. :)

cc @jdonszelmann
r? @JonathanBrouwer

rustbot · 2025-12-04T14:56:18Z

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann

GuillaumeGomez · 2025-12-04T14:57:13Z

Just to be safe:

@bors try @rust-timer queue

Port `doc` attributes to new attribute API

GuillaumeGomez · 2025-12-04T16:22:17Z

Ah right, forgot about clippy. Updating it as well.

rustbot · 2025-12-04T16:43:25Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

rust-bors · 2025-12-04T17:13:36Z

☀️ Try build successful (CI)
Build commit: 4f24013 (4f240130369441697d5ccde454291d18ea71990c, parent: 5372fc9cb790c112ce707991b1fcc025cec8fdbe)

rust-timer · 2025-12-04T23:06:53Z

Finished benchmarking commit (4f24013): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.0%	[0.2%, 3.1%]	30
Improvements ✅ (primary)	-0.5%	[-1.3%, -0.1%]	39
Improvements ✅ (secondary)	-0.6%	[-1.1%, -0.1%]	30
All ❌✅ (primary)	-0.5%	[-1.3%, -0.1%]	39

Max RSS (memory usage)

Results (primary -0.2%, secondary 1.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.3%	[0.8%, 1.5%]	3
Regressions ❌ (secondary)	3.6%	[1.4%, 6.4%]	11
Improvements ✅ (primary)	-1.0%	[-2.3%, -0.5%]	6
Improvements ✅ (secondary)	-1.4%	[-2.5%, -0.6%]	8
All ❌✅ (primary)	-0.2%	[-2.3%, 1.5%]	9

Cycles

Results (secondary 2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.1%	[2.2%, 9.8%]	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.8%	[-5.5%, -2.2%]	3
All ❌✅ (primary)	-	-	0

Binary size

Results (primary -0.5%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.3%]	23
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	9
Improvements ✅ (primary)	-0.9%	[-3.1%, -0.0%]	28
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.0%]	15
All ❌✅ (primary)	-0.5%	[-3.1%, 0.3%]	51

Bootstrap: 468.725s -> 473.041s (0.92%)
Artifact size: 386.77 MiB -> 389.06 MiB (0.59%)

GuillaumeGomez · 2025-12-04T23:17:42Z

Strange, one doc test has a regression but all others are faster.

bors · 2025-12-05T02:53:57Z

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

JonathanBrouwer · 2025-12-05T12:21:30Z

I'd say since perf looks very good on primary benchmarks and is mixed on secondary benchmarks, I'm happy with perf as-is. I am wondering what caused the regression on include-blob though, it doesn't even use any doc attributes
@rustbot label: +perf-regression-triaged

…tEmitter` instead of `NodeId`

…g::cfg_matches` API

rustbot · 2025-12-05T15:26:08Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

compiler/rustc_attr_parsing/src/lib.rs

JonathanBrouwer · 2025-12-05T15:42:39Z

compiler/rustc_attr_parsing/src/context.rs

        ConstStabilityParser,
+        DocParser,
        MacroUseParser,
+


nit: extra newline

JonathanBrouwer · 2025-12-05T15:43:07Z

compiler/rustc_attr_parsing/src/attributes/doc.rs

@@ -0,0 +1,571 @@
+// FIXME: to be removed
+#![allow(unused_imports)]


nit: Can you remove this one?

JonathanBrouwer · 2025-12-05T15:50:54Z

compiler/rustc_attr_parsing/src/attributes/doc.rs

+                // if self.attribute.$ident.is_some() {
+                //     cx.duplicate_key(path.span(), path.word_sym().unwrap());
+                //     return;
+                // }


Does leaving this out cause a regression? If so please fix, otherwise we can leave it like this

No, the opposite. If we uncomment it, bootstrap command will fail because rustdoc args are duplicated. Gonna need to fix that.

…lve`

rust-log-analyzer · 2025-12-05T23:30:27Z

The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)


---- [rustdoc-json] tests/rustdoc-json/keyword_private.rs stdout ----
------jsondocck stdout------------------------------

------jsondocck stderr------------------------------
/checkout/tests/rustdoc-json/keyword_private.rs:8, directive failed
matched to String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], keyword: \"match\", test_attrs: []})]") but want String("#[doc(keyword = \"match\")]")
/checkout/tests/rustdoc-json/keyword_private.rs:16, directive failed
matched to String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], keyword: \"break\", test_attrs: []})]") but want String("#[doc(keyword = \"break\")]")

------------------------------------------

error: jsondocck failed!
status: exit status: 1
command: "/checkout/obj/build/aarch64-unknown-linux-gnu/stage1-tools-bin/jsondocck" "--doc-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/rustdoc-json/keyword_private" "--template" "/checkout/tests/rustdoc-json/keyword_private.rs"
stdout: none
--- stderr -------------------------------
/checkout/tests/rustdoc-json/keyword_private.rs:8, directive failed
matched to String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], keyword: \"match\", test_attrs: []})]") but want String("#[doc(keyword = \"match\")]")
/checkout/tests/rustdoc-json/keyword_private.rs:16, directive failed
matched to String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], keyword: \"break\", test_attrs: []})]") but want String("#[doc(keyword = \"break\")]")
------------------------------------------

Rustdoc Output:
status: exit status: 0
command: cd "/checkout/obj/build/aarch64-unknown-linux-gnu/test/rustdoc-json/keyword_private" && env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/lib/rustlib/aarch64-unknown-linux-gnu/lib" "-L" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/rustdoc-json/keyword_private/auxiliary" "-o" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/rustdoc-json/keyword_private" "--deny" "warnings" "/checkout/tests/rustdoc-json/keyword_private.rs" "-A" "internal_features" "--document-private-items" "--output-format" "json" "-Zunstable-options"
stdout: none
stderr: none

---- [rustdoc-json] tests/rustdoc-json/keyword_private.rs stdout end ----
---- [rustdoc-json] tests/rustdoc-json/visibility/doc_hidden_documented.rs stdout ----
------jsondocck stdout------------------------------

------jsondocck stderr------------------------------
/checkout/tests/rustdoc-json/visibility/doc_hidden_documented.rs:4, directive failed
matched to Array [Object {"other": String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], test_attrs: []})]")}] but want Array [Object {"other": String("#[doc(hidden)]")}]
/checkout/tests/rustdoc-json/visibility/doc_hidden_documented.rs:8, directive failed
matched to Array [Object {"other": String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], test_attrs: []})]")}] but want Array [Object {"other": String("#[doc(hidden)]")}]
/checkout/tests/rustdoc-json/visibility/doc_hidden_documented.rs:12, directive failed
matched to Array [Object {"other": String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], test_attrs: []})]")}] but want Array [Object {"other": String("#[doc(hidden)]")}]

------------------------------------------

error: jsondocck failed!
status: exit status: 1
command: "/checkout/obj/build/aarch64-unknown-linux-gnu/stage1-tools-bin/jsondocck" "--doc-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/rustdoc-json/visibility/doc_hidden_documented" "--template" "/checkout/tests/rustdoc-json/visibility/doc_hidden_documented.rs"
stdout: none
--- stderr -------------------------------
/checkout/tests/rustdoc-json/visibility/doc_hidden_documented.rs:4, directive failed
matched to Array [Object {"other": String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], test_attrs: []})]")}] but want Array [Object {"other": String("#[doc(hidden)]")}]
/checkout/tests/rustdoc-json/visibility/doc_hidden_documented.rs:8, directive failed
matched to Array [Object {"other": String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], test_attrs: []})]")}] but want Array [Object {"other": String("#[doc(hidden)]")}]
/checkout/tests/rustdoc-json/visibility/doc_hidden_documented.rs:12, directive failed
matched to Array [Object {"other": String("#[attr = Doc(DocAttribute {aliases: [], inline: [], cfg: [], auto_cfg: [],\nauto_cfg_change: [], test_attrs: []})]")}] but want Array [Object {"other": String("#[doc(hidden)]")}]
------------------------------------------

Rustdoc Output:
status: exit status: 0
command: cd "/checkout/obj/build/aarch64-unknown-linux-gnu/test/rustdoc-json/visibility/doc_hidden_documented" && env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/lib/rustlib/aarch64-unknown-linux-gnu/lib" "-L" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/rustdoc-json/visibility/doc_hidden_documented/auxiliary" "-o" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/rustdoc-json/visibility/doc_hidden_documented" "--deny" "warnings" "/checkout/tests/rustdoc-json/visibility/doc_hidden_documented.rs" "-A" "internal_features" "--document-hidden-items" "--output-format" "json" "-Zunstable-options"
stdout: none
stderr: none

---- [rustdoc-json] tests/rustdoc-json/visibility/doc_hidden_documented.rs stdout end ----

JonathanBrouwer

I did a first pass through all the code changes (not the tests yet). Will probably do a second pass because I went quite quickly and might've missed things, but this should be most of it

View changes since this review

JonathanBrouwer · 2025-12-06T08:59:30Z

compiler/rustc_hir/src/attrs/data_structures.rs

+    pub no_crate_inject: Option<Span>,
+}
+
+impl Default for DocAttribute {


Is this different from a derive(Default)? If not, lets derive this

JonathanBrouwer · 2025-12-06T09:13:30Z

compiler/rustc_passes/src/check_attr.rs

+                    span,
                    errors::DocInlineOnlyUse {
-                        attr_span: meta.span(),
-                        item_span: (style == AttrStyle::Outer).then(|| self.tcx.hir_span(hir_id)),


I see a bunch of these checks got removed, why?

JonathanBrouwer · 2025-12-06T09:19:06Z

compiler/rustc_passes/src/check_attr.rs

-                                self.check_doc_search_unbox(meta, hir_id);
-                            }
-                        }
+    fn check_doc_attrs(&self, attr: &DocAttribute, hir_id: HirId, target: Target) {


Ooh I love how much nicer this file got, ty :3

JonathanBrouwer · 2025-12-06T09:58:27Z

src/librustdoc/clean/cfg.rs

+        | CfgEntry::NameValue { .. }
+        | CfgEntry::Not(..)
+        | CfgEntry::Version(..)
+        | CfgEntry::All(..) => true,


Should we recursively check for an Any inside the All here?

JonathanBrouwer · 2025-12-06T10:01:03Z

src/librustdoc/clean/cfg.rs

+            if cfgs.is_empty() { None } else { Some(CfgEntry::All(cfgs, DUMMY_SP)) }
+        }
+        CfgEntry::Version(..) => {
+            // FIXME: Should be handled.


Why is this not handled the way it is?

JonathanBrouwer · 2025-12-06T10:26:45Z

src/librustdoc/doctest/rust.rs

+                {
+                    let mut iter = stream.into_iter().peekable();
+                    while let Some(token) = iter.next() {
+                        if let TokenTree::Ident(i) = token {


This code silently fails if the span does not match the expected structure of an attribute. Is there anywhere we check that it does match it, and error otherwise?
Fine with this hack (honestly surprisingly elegant) but I'm anticipating this hack might be hard to fix and this code might be here for a while, wanna make sure it's not silently failing while it is

JonathanBrouwer · 2025-12-06T10:27:26Z

src/librustdoc/doctest/rust.rs

+                            let i = i.to_string();
+                            let peek = iter.peek();
+                            match peek {
+                                Some(TokenTree::Group(g)) => {


I don't fully understand the cases in this match here, could you add some clarifying comments?

JonathanBrouwer · 2025-12-06T10:32:07Z

src/librustdoc/passes/collect_trait_impls.rs

+                    Attribute::Parsed(AttributeKind::Doc(d)) if !d.cfg.is_empty() => {
+                        let mut new_attr = DocAttribute::default();
+                        new_attr.cfg = d.cfg.clone();
+                        Some(Attribute::Parsed(AttributeKind::Doc(Box::new(new_attr))))


Why are we doing this weird clone of only the cfg condition here? Performance?
(If so, please add a comment explaining it)

JonathanBrouwer · 2025-12-06T10:35:37Z

src/librustdoc/Cargo.toml

 indexmap = { version = "2", features = ["serde"] }
 itertools = "0.12"
 minifier = { version = "0.3.5", default-features = false }
+proc-macro2 = "1.0.103"


Could we use TokenStream from rustc_ast instead of proc-macro2? Is that better/worse?

JonathanBrouwer · 2025-12-06T10:36:58Z

src/librustdoc/lib.rs

+                    for lint in &delayed_lints.lints {
+                        match lint {
+                            DelayedLint::AttributeParsing(attribute_lint) => {
+                                rustc_attr_parsing::emit_attribute_lint(attribute_lint, tcx)


This will slightly change after you rebase, but should be easy enough to fix

bors · 2025-12-06T10:40:14Z

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

rustbot assigned JonathanBrouwer Dec 4, 2025

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Dec 4, 2025

Auto merge of #149645 - GuillaumeGomez:doc-attr-based, r=<try>

4f24013

Port `doc` attributes to new attribute API

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 4, 2025

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the doc-attr-based branch from fc40ba1 to b27592e Compare December 4, 2025 16:08

This comment has been minimized.

Sign in to view

rustbot added the T-clippy Relevant to the Clippy team. label Dec 4, 2025

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Dec 4, 2025

This comment has been minimized.

Sign in to view

GuillaumeGomez added 13 commits December 5, 2025 15:43

Correctly handle doc items inlining

c7f1cf4

Fix Cfg add/or operations

b4df7b5

Keep a list of CfgEntry instead of just one

f068b5f

Fix doc(auto_cfg) attribute parsing

a70412f

Correctly handle doc(test(attr(...)))

d4812b1

Update rustdoc unit tests

c85a80f

Fix warning messages

8d05e85

Make rustc_attr_parsing::eval_config_entry take `impl CfgMatchesLin…

6baaeb5

…tEmitter` instead of `NodeId`

Update check_doc_cfg pass in rustdoc, remove old `rustc_attr_parsin…

949209d

…g::cfg_matches` API

Finish fixing ui tests

e86947c

Sort fluent messages

1e85502

Update clippy code to new doc attribute API

3a69d06

Update rustc_resolve unit tests

81530be

GuillaumeGomez force-pushed the doc-attr-based branch from db01fba to fc00bb8 Compare December 5, 2025 15:26

This comment has been minimized.

Sign in to view

Fix ui tests

f657ca7

GuillaumeGomez force-pushed the doc-attr-based branch from fc00bb8 to f657ca7 Compare December 5, 2025 15:31

JonathanBrouwer requested changes Dec 5, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2025

This comment has been minimized.

Sign in to view

Correctly iterate doc comments in intra-doc resolution in `rustc_reso…

2493227

…lve`

This comment has been minimized.

Sign in to view

Clean up code

9d4ba5b

GuillaumeGomez force-pushed the doc-attr-based branch from 649f660 to 9d4ba5b Compare December 5, 2025 22:29

JonathanBrouwer requested changes Dec 6, 2025

View reviewed changes

fmease mentioned this pull request Dec 6, 2025

[DONT MERGE] Doc attr based on master #147097

Closed

		@@ -0,0 +1,571 @@
		// FIXME: to be removed
		#![allow(unused_imports)]

Port doc attributes to new attribute API #149645

Are you sure you want to change the base?

Port doc attributes to new attribute API #149645

Conversation

GuillaumeGomez commented Dec 4, 2025

Uh oh!

rustbot commented Dec 4, 2025

Uh oh!

GuillaumeGomez commented Dec 4, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

GuillaumeGomez commented Dec 4, 2025

Uh oh!

rustbot commented Dec 4, 2025

Uh oh!

rust-bors bot commented Dec 4, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rust-timer commented Dec 4, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

Uh oh!

GuillaumeGomez commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

bors commented Dec 5, 2025

Uh oh!

JonathanBrouwer commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 5, 2025

Uh oh!

This comment has been minimized.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-log-analyzer commented Dec 5, 2025

Uh oh!

JonathanBrouwer left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Dec 6, 2025

Uh oh!

Reviewers

Assignees

Port `doc` attributes to new attribute API #149645

Port `doc` attributes to new attribute API #149645

GuillaumeGomez commented Dec 4, 2025 •

edited

Loading

JonathanBrouwer commented Dec 5, 2025 •

edited

Loading

JonathanBrouwer left a comment •

edited by rustbot

Loading