Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Implement RFC 3631: add rustdoc doc_cfg features #138907

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

Open
GuillaumeGomez wants to merge 19 commits into rust-lang:master
base: master
Choose a base branch
Loading
from GuillaumeGomez:rfc-3631

Conversation

Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Mar 24, 2025 •
edited
Loading

Implementation of rust-lang/rfcs#3631.

This implementation actually resulted in a lot of simplifications:

  • All cfg computation is now done in one place: propagate_doc_cfg.rs. Because (trait) impls are not retrieved at the same time as the other items, we cannot perform this computation in the clean process, it needs to be after.
  • Because there is cfg inheritance, we can keep track of them in one place (in propagate_doc_cfg.rs), meaning we don't need to copy an item's attributes to its children anymore. Only exception: impl items. For them we clone only cfg attributes.
  • propagate_doc_cfg.rs is also now much simpler, much less need to keep track of parents, since everything we need is handled by the new CfgInfo type.
  • I also suspect that Cfg::simplify_with could either be removed or at least used directly into propagate_doc_cfg.rs when we compute cfgs. Considering how big the PR already is, I'll do it in a follow-up.

I didn't remove the doc_cfg* features in this PR because some dependencies used in rustc (like stdarch) are using it, so we need to have a nightly released with this PR before I can switch to the new feature.

r? ghost

@rustbot rustbot added A-attributes Area: Attributes (`#[...]`, `#![...]`) 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-libs Relevant to the library 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 Mar 24, 2025

This comment has been minimized.

Copy link
Contributor

What remains to be done:

I'd also want to block the stabilization on landing #138844 to avoid a stable rustdoc feature relying on externally observable hacks in rustc.
The crater run in #138844 returned mostly clean, so I expect it to land soon.

Copy link
Member Author

Noted! And that will be a nice improvement, thanks!

Just one thing left for the cfg expansion missing: #[cfg_attr(blabla, derive(Debug))]. In this case, the cfg is not kept in the generated derive items. It's been in my TODO list for a long time now. ^^'

Copy link
Contributor

Just one thing left for the cfg expansion missing: #[cfg_attr(blabla, derive(Debug))]

Do you mean like in #138515? :)

Copy link
Member Author

You're my hero! Gonna need to handle this new attribute then. :)

This comment has been minimized.

Copy link
Collaborator

bors commented Mar 25, 2025

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

@GuillaumeGomez GuillaumeGomez force-pushed the rfc-3631 branch 2 times, most recently from d88598f to db25eea Compare March 27, 2025 10:18
@rustbot rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Mar 27, 2025

This comment has been minimized.

Copy link
Collaborator

bors commented Mar 27, 2025

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

@GuillaumeGomez GuillaumeGomez force-pushed the rfc-3631 branch 2 times, most recently from b8cb424 to b581ce1 Compare March 28, 2025 23:54

This comment has been minimized.

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the rfc-3631 branch 4 times, most recently from 71369a1 to fbee8a9 Compare April 1, 2025 15:52

This comment has been minimized.

Copy link
Member Author

I don't know if we have written that down in our new Forge docs (I hope so) but a feature stabilization would of course require another FCP, the one from the RFC isn't sufficient for that. Moreover, not stabilizing here / now / immediately allows us to gather more user feedback (esp. since the changes made by this PR are non-trivial). Heck, we could even issue a "call for testing" on the official blog – anything to make the eventual stabilization run as smoothly as possible.

I was planning to keep it unstable for at least a few months and to enable it in some big crates to gather some usage and see if everything is working as expected.

And in any case: any stabilization PR must go through FCP (we should check if we mention that in the forge ^^').

Copy link
Member Author

Oh also: applied suggestions from @lolbinarycat. :)
lolbinarycat reacted with thumbs up emoji

Copy link
Collaborator

bors commented Aug 28, 2025

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

Copy link
Collaborator

rustbot commented Aug 29, 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.

Copy link
Collaborator

rustbot commented Aug 29, 2025

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

Copy link
Member Author

Fixed merge conflicts. :)
lolbinarycat reacted with thumbs up emoji

Copy link
Member

fmease commented Sep 4, 2025

I'll take another look later tonight then I'll r=lolbinarycat,fmease ig.
lolbinarycat reacted with thumbs up emoji

@fmease fmease assigned fmease and unassigned camelid Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Urgau Urgau Urgau left review comments

@lolbinarycat lolbinarycat lolbinarycat left review comments

@Manishearth Manishearth Manishearth approved these changes

@camelid camelid Awaiting requested review from camelid

Labels
A-attributes Area: Attributes (`#[...]`, `#![...]`) A-run-make Area: port run-make Makefiles to rmake.rs 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-libs Relevant to the library 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /