Skip to content

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jun 13, 2025

Fixes #141123.

The implementation and purpose of this new #[doc(attribute = "...")] attribute is very close to #[doc(keyword = "...")]. Which means that luckily for us, most of the code needed was already in place and @Noratrieb nicely wrote a first draft that helped me implement this new attribute very fast.

Now with all this said, there is one thing I didn't do yet: adding a rustdoc-js-std test. I added GUI tests with search results for attributes so should be fine but I still plan on adding one for it once documentation for builtin attributes will be written into the core/std libs.

You can test it here.

cc @Noratrieb @Veykril

@rustbot

This comment was marked as outdated.

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustdoc-json Area: Rustdoc JSON backend A-rustdoc-search Area: Rustdoc's search feature 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Jun 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 13, 2025

rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing src/librustdoc/json/conversions.rs; otherwise, make sure you bump the FORMAT_VERSION constant.

cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi

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

cc @jdonszelmann

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha, @lolbinarycat

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-attribute-attribute branch from 1ab0dc7 to e2fd7f7 Compare June 13, 2025 20:07
@rust-log-analyzer

This comment has been minimized.

@jdonszelmann
Copy link
Contributor

cc: @ehuss you might want to make sure the reference documents this too? Unsure whether this is interesting to you but just to be safe

if is_keyword { "keyword" } else { "attribute " }
}

let value = match meta.value_str() {
Copy link
Contributor

Choose a reason for hiding this comment

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

sigh, I'll put the doc attribute high up on my todo list to get a proper parser... (this is fine for now)

#![feature(rustdoc_internals)]
#![allow(internal_features)]

/// Some documentation about the attribute.
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we make sure this stays in sync with the compiler's view of attributes? Is the plan to keep this manual, or could we somehow generate this from for example rustc_attr_data_structures

Copy link
Member Author

Choose a reason for hiding this comment

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

Just like keywords: we can only check that the attribute is used on an existing one. If new ones are added, there is no check for that.

We could add a tidy check for it though.

Copy link
Contributor

Choose a reason for hiding this comment

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

that'd be nice

title.push_str(it.name.unwrap().as_str());
}
if !it.is_primitive() && !it.is_keyword() {
if !it.is_primitive() && !it.is_keyword() && !it.is_attribute() {
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth adding a clean::Item::is_std_synthetic method for all 3 of these cases (name heavily subject to bikeshedding).

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about the same but wasn't sure it was worth it. But since I'm not the only one thinking that, adding it.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-attribute-attribute branch from e2fd7f7 to a0fb9d0 Compare June 14, 2025 10:41
@rustbot
Copy link
Collaborator

rustbot commented Jun 14, 2025

These commits modify tests/rustdoc-json.
rustdoc-json is a public (but unstable) interface.

Please ensure that if you've changed the output:

  • It's intentional.
  • The FORMAT_VERSION in src/librustdoc-json-types is bumped if necessary.

cc @aDotInTheVoid, @obi1kenobi

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-attribute-attribute branch from a0fb9d0 to 14b0aa7 Compare June 14, 2025 23:27
@rust-log-analyzer

This comment has been minimized.

@bors

This comment was marked as resolved.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-attribute-attribute branch 2 times, most recently from 4a47724 to 094ed61 Compare June 15, 2025 10:25
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-attribute-attribute branch from 094ed61 to 10c8903 Compare June 15, 2025 10:33
@GuillaumeGomez
Copy link
Member Author

Ok, hopefully this time I didn't forget yet another different check I didn't use in years. T_T

@GuillaumeGomez
Copy link
Member Author

Yeay! \o/

@GuillaumeGomez
Copy link
Member Author

@notriddle It was an argument against having the search result link directly to the reference. I'm fine with having a link to an external resource in an item page as long as there is some content before it and not just a link.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-attribute-attribute branch from 4b3fb64 to 93fbd25 Compare August 28, 2025 14:04
@rustbot
Copy link
Collaborator

rustbot commented Aug 28, 2025

This PR was rebased onto a different master 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.

@GuillaumeGomez
Copy link
Member Author

Fixed merge conflicts. Since we got approval from the lang team, all that remains is to review I guess. :)

@fmease
Copy link
Member

fmease commented Aug 28, 2025

I have fully reviewed here: #142472 (review).

@GuillaumeGomez
Copy link
Member Author

I realized it just after commenting. ^^' I'm working on it. :)

@GuillaumeGomez
Copy link
Member Author

Applied @fmease's suggestions. :)

@@ -0,0 +1,5 @@
// This is currently not supported but should be!
Copy link
Member

@fmease fmease Aug 28, 2025

Choose a reason for hiding this comment

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

What happens tho if you add #![feature(rustdoc_internals)]? Does it error with unknown attribute or does it get accepted? If it gets accepted, is rustdoc-search able to find it when you search for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Arg, I tested with a file with all attributes and forgot to copy/paste this one. ><


### Document builtin attributes

This is for Rust compiler internal use only.
Copy link
Member

Choose a reason for hiding this comment

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

It's more "for internal use in the std library."; section Document keywords also has that incorrect statement.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then taking it as well.

@fmease
Copy link
Member

fmease commented Aug 28, 2025

@bors rollup (internal feature)

r=me with latest nits addressed in some way or another + with some of the smaller commits squashed where it makes sense.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-attribute-attribute branch from 65d329a to f3c0234 Compare August 28, 2025 16:25
@GuillaumeGomez
Copy link
Member Author

@bors r=fmease

@bors
Copy link
Collaborator

bors commented Aug 28, 2025

📌 Commit f3c0234 has been approved by fmease

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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Aug 28, 2025
bors added a commit that referenced this pull request Aug 28, 2025
Rollup of 6 pull requests

Successful merges:

 - #142472 (Add new `doc(attribute = "...")` attribute)
 - #145368 (CFI: Make `lto` and `linker-plugin-lto` work the same for `compiler_builtins`)
 - #145853 (Improve error messages around invalid literals in attribute arguments)
 - #145920 (bootstrap: Explicitly mark the end of a failed test's captured output)
 - #145937 (add doc-hidden to exports in attribute prelude)
 - #145965 (Move exporting of profiler and sanitizer symbols to the LLVM backend)

r? `@ghost`
`@rustbot` modify labels: rollup
@traviscross
Copy link
Contributor

For multiple reasons:

  1. It is not item doc oriented but item kind oriented, meaning that one chapter will cover all of the items of the same kind (so for example attributes), making it nice when you want to go through all items, but definitely not when you're looking for one in particular and want a lot of information on this in particular.
  2. It is not so much about explaining but about describing. Which is not bad or whatever, just not aimed at the same target audience.
  3. It uses a lot of terms that a rust beginner would have never heard of.
  4. There is a lot of visual information, like the anchors on the side of each item. Again not an issue, but very different to what someone coming from rust docs would expect and definitely need an extra time to adapt (this point is honestly pretty low on the issues, just being exhaustive).

So overall, the target audience is the main issue imo and why on top of the previously mentioned reasons we should not just generate links to the reference but instead add some extra context around it (context that doesn't need to be big or anything, but that might allow to give the information the user is looking for more quickly).

Thanks for these details. You're not wrong about any of this. These are valid reasons to want to write and maintain a separate set of documentation for attributes and keywords. It's just a lot of work to do well.

@Mark-Simulacrum
Copy link
Member

That would also mean that we would end up in another page that wouldn't work with no internet connection and it would be bad UX to directly lead the users in a resource that isn't available.

FWIW, I'm pretty sure we ship the reference (and all other books) in the same component, rust-docs. So as long as our URLs are relative and the HTML loads on file:// URLs, I would expect that to work locally (offline) + in doc.r-l.o just fine.

@bors bors merged commit f948c79 into rust-lang:master Aug 28, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 28, 2025
rust-timer added a commit that referenced this pull request Aug 28, 2025
Rollup merge of #142472 - GuillaumeGomez:doc-attribute-attribute, r=fmease

Add new `doc(attribute = "...")` attribute

Fixes #141123.

The implementation and purpose of this new `#[doc(attribute = "...")]` attribute is very close to `#[doc(keyword = "...")]`. Which means that luckily for us, most of the code needed was already in place and `@Noratrieb` nicely wrote a first draft that helped me implement this new attribute very fast.

Now with all this said, there is one thing I didn't do yet: adding a `rustdoc-js-std` test. I added GUI tests with search results for attributes so should be fine but I still plan on adding one for it once documentation for builtin attributes will be written into the core/std libs.

You can test it [here](https://rustdoc.crud.net/imperio/doc-attribute-attribute/foo/index.html).

cc `@Noratrieb` `@Veykril`
@fmease
Copy link
Member

fmease commented Aug 29, 2025

Thanks for these details. You're not wrong about any of this. These are valid reasons to want to write and maintain a separate set of documentation for attributes and keywords. It's just a lot of work to do well.

Guillaume and I have sort of decided to go with a one-sentence synopsis + one example maximum + a link to the Reference. Nothing is set in stone here of course. However, having some sort of synopsis in rustdoc-generated pages (as opposed to linking or redirecting to the Reference) would "basically automatically" allow rust-analyzer to show docs for built-in attrs on LSP-hover which was actually the original motivation for this feature as stated in the linked issue #141123.

Another reason for a proper page over a link would be the hope that these new attribute docs would be more accessible, not just phrasing-wise (as previously mentioned) but also presentation-wise (re. the latter, there's no "context switch" necessary for the brain because the page layout stays the same which is good for various obvious reasons).

Of course, there are downsides to this approach. E.g., more experienced users will probably care less about these "stubs" and want to jump to the Reference directly in which case two clicks over one click could become increasingly annoying (T-lang's stance IIUC) and could also "hurt line of thought"1.

Footnotes

  1. At the other end of the spectrum however, we have actual power users who use parametrized browser keywords to jump to various documents like the Reference with a given query param.

@GuillaumeGomez GuillaumeGomez deleted the doc-attribute-attribute branch August 29, 2025 11:24
@GuillaumeGomez
Copy link
Member Author

That would also mean that we would end up in another page that wouldn't work with no internet connection and it would be bad UX to directly lead the users in a resource that isn't available.

FWIW, I'm pretty sure we ship the reference (and all other books) in the same component, rust-docs. So as long as our URLs are relative and the HTML loads on file:// URLs, I would expect that to work locally (offline) + in doc.r-l.o just fine.

Nice, didn't know about that. We can make relative links then. :)

It's just a lot of work to do well.

Indeed. In this case, I'd recommend opening an issue with the list of attributes so that the work can be done in parallel, allowing to potentially greatly reduce the total time required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustdoc-json Area: Rustdoc JSON backend A-rustdoc-search Area: Rustdoc's search feature I-lang-radar Items that are on lang's radar and will need eventual work or consideration. 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document builtin attributes (like no_mangle, repr, ...) in the standard library docs