Skip to content

Conversation

@GuillaumeGomez
Copy link
Member

Follow-up of #138907.

r? lolbinarycat

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Sep 29, 2025
@GuillaumeGomez GuillaumeGomez changed the title Doc propagation before stripping items [rustdoc] Move doc cfg propagation pass before items stripping passes Sep 29, 2025
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-propagation-before-stripping-items branch from 9157004 to a5068f7 Compare September 29, 2025 15:05
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the doc-propagation-before-stripping-items branch from a5068f7 to 9119eba Compare September 29, 2025 16:08
@lolbinarycat
Copy link
Contributor

This just made me realize that the way we print feature dependency info is often pretty bad. I originally read "Available on non-crate feature blob only" as referring to a "non-crate feature" (whatever that would mean) named "blob".

Anyways, that's an unrelated (and non-trivial) issue that should be addressed in a separate PR.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 29, 2025

📌 Commit 9119eba has been approved by lolbinarycat

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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 29, 2025
bors added a commit that referenced this pull request Sep 30, 2025
Rollup of 4 pull requests

Successful merges:

 - #145883 (Make macOS dist build configuration match where reasonable)
 - #146457 (Skip cleanups on unsupported targets)
 - #147152 (builtin `Fn`-trait impls: instantiate binder before the return type `Sized` check)
 - #147153 ([rustdoc] Move doc cfg propagation pass before items stripping passes)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 745b8f6 into rust-lang:master Sep 30, 2025
10 checks passed
rust-timer added a commit that referenced this pull request Sep 30, 2025
Rollup merge of #147153 - GuillaumeGomez:doc-propagation-before-stripping-items, r=lolbinarycat

[rustdoc] Move doc cfg propagation pass before items stripping passes

Follow-up of #138907.

r? lolbinarycat
@rustbot rustbot added this to the 1.92.0 milestone Sep 30, 2025
@GuillaumeGomez GuillaumeGomez deleted the doc-propagation-before-stripping-items branch September 30, 2025 08:35
@ojeda
Copy link
Contributor

ojeda commented Oct 13, 2025

I think this one implies that now intra-doc links are now checked in doc(hidden) items, right?

That is nice, since intra-doc links could be useful even if not rendered (e.g. for source view, IDEs, etc., plus to detect typos or if the docs get copied to public items, etc.), but I wanted to confirm whether that is expected behavior (because I need to clean up a couple cases in the kernel), and if so, whether we should add a test here on rustdoc about this.

Thanks!

ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden),
ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate),
ConditionalPass::new(STRIP_PRIV_IMPORTS, WhenDocumentPrivate),
ConditionalPass::always(COLLECT_INTRA_DOC_LINKS),
Copy link
Member

@fmease fmease Oct 13, 2025

Choose a reason for hiding this comment

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

Revisiting thus due to ojeda's comment. Moving the intra-doc link pass before the strippers seems very much like a mistake.

I believe this PR was a response to my review comment #138907 (comment) in which I requested to move the doc cfg propagation pass before the strippers.

I haven't tried it out yet but that review comment wasn't properly addressed then.

(You did also change the ordering in PASSES but that's only used in the --help usage output IIRC)

Copy link
Member

Choose a reason for hiding this comment

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

Did the added test also work before this PR?

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 17, 2025
… r=fmease

rustdoc: Fix passes order so intra-doc links are collected after stripping passes

Fixes regression I introduced in rust-lang#147153.

This PR puts back the intra-doc link collecting pass after the stripping items pass, preventing lints to be emitted on non-visible items.

Although, might be nice to add a way to change this behaviour. To be discussed later on.

cc `@ojeda`
r? `@fmease`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 17, 2025
… r=fmease

rustdoc: Fix passes order so intra-doc links are collected after stripping passes

Fixes regression I introduced in rust-lang#147153.

This PR puts back the intra-doc link collecting pass after the stripping items pass, preventing lints to be emitted on non-visible items.

Although, might be nice to add a way to change this behaviour. To be discussed later on.

cc `@ojeda`
r? `@fmease`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 18, 2025
… r=fmease

rustdoc: Fix passes order so intra-doc links are collected after stripping passes

Fixes regression I introduced in rust-lang#147153.

This PR puts back the intra-doc link collecting pass after the stripping items pass, preventing lints to be emitted on non-visible items.

Although, might be nice to add a way to change this behaviour. To be discussed later on.

cc ``@ojeda``
r? ``@fmease``
rust-timer added a commit that referenced this pull request Oct 18, 2025
Rollup merge of #147809 - GuillaumeGomez:fix-rustdoc-passes, r=fmease

rustdoc: Fix passes order so intra-doc links are collected after stripping passes

Fixes regression I introduced in #147153.

This PR puts back the intra-doc link collecting pass after the stripping items pass, preventing lints to be emitted on non-visible items.

Although, might be nice to add a way to change this behaviour. To be discussed later on.

cc ``@ojeda``
r? ``@fmease``
ojeda added a commit to ojeda/linux that referenced this pull request Oct 29, 2025
Starting with Rust 1.90.0 (expected 2025-12-11), Rust checks intra-doc
links in `doc(hidden)` items [1]:

    warning: unresolved link to `PinnedDrop`
       --> rust/pin-init/src/macros.rs:508:11
        |
    508 | /// See [`PinnedDrop`] for more information.
        |           ^^^^^^^^^^ no item named `PinnedDrop` in scope
        |
        = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
        = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

Thus fix it. Normally, we would fix the link, but there are other
`PinnedDrop` mentions in the file without the intra-doc link, so stay
consistent.

Link: rust-lang/rust#147153 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this pull request Nov 19, 2025
Rust 1.92.0 warns when building the documentation that [`PinnedDrop`] is
an invalid reference. This is correct and it's weird that it didn't warn
before, so fix the link.

[ The reason is that it is hidden -- I had asked about that in the
  upstream PR that changed the behavior because I wasn't sure it was
  intentional (and thus whether we needed to fix this and other cases):

      rust-lang/rust#147153 (comment)

  It turns out it was not, and it has been fixed for 1.92.0's upcoming
  release thanks to Guillaume and León. So we do not strictly need
  this patch and the other changes anymore:

      rust-lang/rust#147809

  However, checking hidden/private items or, even better, a runtime
  toggle to be able to see those on the fly, is something that I think
  would be quite nice so I have had it in our usual lists for a while.
  Guillaume is open to the idea and perhaps experimenting with an
  implementation on our side first -- he asked me to open issues
  upstream:

      rust-lang/rust#149105
      rust-lang/rust#149106

    - Miguel ]

Signed-off-by: Benno Lossin <lossin@kernel.org>
Link: https://patch.msgid.link/20251016211740.653599-1-lossin@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants