Skip to content
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

rustdoc regression: intra-doc link links to wrong item #136777

Open
tchebb opened this issue Feb 9, 2025 · 3 comments
Open

rustdoc regression: intra-doc link links to wrong item #136777

tchebb opened this issue Feb 9, 2025 · 3 comments
Assignees
Labels
C-bug Category: This is a bug. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc I-slow Issue: Problems and improvements with respect to performance of generated code. regression-from-stable-to-beta Performance or correctness regression from stable to beta. 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.

Comments

@tchebb
Copy link

tchebb commented Feb 9, 2025

Reproduction steps

I tried to document autocxx::subclass::is_subclass() from the autocxx crate (docs.rs page), which has the following doc comment:

/// Deprecated - use [`subclass`] instead.
#[deprecated]
pub use autocxx_macro::subclass as is_subclass;

I expected to see this happen: the subclass link points to the documentation for the subclass item defined in the same file. This is indeed what happens on stable, rustdoc 1.84.1 (e71f9a9a9 2025-01-27).

Instead, this happened: the subclass link points to the documentation for the is_subclass item—the same page I'm already on! This happens on rustdoc 1.86.0-nightly (ad211ced8 2025-01-07), currently in use by docs.rs, as well as today's beta (rustdoc 1.85.0-beta.8 (38213856a 2025-02-06)) and today's nightly (rustdoc 1.86.0-nightly (942db6782 2025-02-06)).

Sorry this example is using a real crate rather than a minimized test case. I haven't taken the time to dig any deeper than what I've written here, but I can if it'd be helpful.

Version it worked on

It most recently worked on: rustdoc 1.84.1

Version with regression

rustdoc --version --verbose:

rustdoc 1.85.0-beta.8 (38213856a 2025-02-06)
binary: rustdoc
commit-hash: 38213856a8a3a6d49a234e0d95a722a4f28a2b18
commit-date: 2025-02-06
host: x86_64-unknown-linux-gnu
release: 1.85.0-beta.8
LLVM version: 19.1.7

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

@tchebb tchebb added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Feb 9, 2025
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels Feb 9, 2025
@tchebb tchebb changed the title rustdoc regression: intra-doc links link to wrong item rustdoc regression: intra-doc link links to wrong item Feb 9, 2025
@jieyouxu jieyouxu added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Feb 9, 2025
@Shunpoco
Copy link
Contributor

I reproduced this regression in the master branch:

tests/rustdoc/auxiliary/macro-bar.rs

//@ force-host
//@ no-prefer-dynamic
//@ compile-flags: --crate-type proc-macro

#![crate_type="proc-macro"]
#![crate_name = "bar"]

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn a1(_attr: TokenStream, item: TokenStream) -> TokenStream {
    item
}

tests/rustdoc/issue-136777.rs

//@ aux-build:macro-bar.rs

#![crate_name = "foo"]

extern crate bar;

//@ has foo/attr.new_a1.html '//a/@href' 'attr.a1.html'

/// Link to [`a1`].
pub use bar::a1 as new_a1;

pub use bar::a1;
error code % ./x test tests/rustdoc/issue-136777.rs ... running 1 tests

[rustdoc] tests/rustdoc/issue-136777.rs ... F

failures:
---- [rustdoc] tests/rustdoc/issue-136777.rs stdout ----

error: htmldocck failed!
status: exit status: 1
...
stdout: none
--- stderr -------------------------------
7: has check failed
XPATH PATTERN did not match
//@ has foo/attr.new_a1.html '//a/@href' 'attr.a1.html'

Encountered 1 errors

@rustbot claim

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Feb 10, 2025
@Shunpoco
Copy link
Contributor

Shunpoco commented Feb 13, 2025

After investigation, I found that this is a corner case of making internal link:

Bisect

After #134806 is merged, this problem is occurred.

Background: internal behavior of making a link

Rustdoc creates a link from [some_item_name] using a cache. The cache keeps the pair of item_def_id and a tuple of (item_path, item_type). item_path is a vector like ["autocxx", "subclass", "is_subclass"], and the item_type is trait, proc-macro and so on. Items which you re-exported from one item have the same item_def_id (is_subclass and subclass have the same id, in your case). One cache record contains only one value (item_path, item_type), so Rustdoc may build one shared link for multiple items.

Behavior in the current stable

  • The last item_path in the module (or a file) is used. In your case, "suclass" is the last item.
  • In my case below, all 3 links points to d3. This is not an ideal behavior I think. User expects link to d2 should points to d2.
/// Link to [`d2`].
pub use std::debug_assert as d1;

/// Link to [`d1`].
pub use std::debug_assert as d2;

/// Link to [`d2`].
pub use std::debug_assert as d3;

Image
Image
Image

Behavior after the PR above

  • The shortest item_path is used. If there is multiple shortest items, the first one is used.
    • ["std", "collections", "HashMap"] is shorter than ["std", "collections", "some", "HashMap"]. And in your case, ["subclass", "is_subclass"] and ["subclass", "subclass"] are the same length so is_subclass is used.
  • In my case, all 3 links points to d1. This is also not an ideal behavior.
    Image
    Image
    Image

So if you change the order of your re-export items, I think the link will be fixed (of course this is not a fundamental solution).

@Shunpoco
Copy link
Contributor

Shunpoco commented Feb 13, 2025

As I wrote above, both the current master and the stable have the issue. Ideally we should modify it, but we need to change the internal structure of Rustdoc (for example, Cache may need to have multiple values for one item_def_id. But it may break other part). Since this is a corner case, I think 1) deciding what the current proper behavior (first win or last win) of it, then 2) adding an unit test to ensure the decision, is good for a temporary solution. Since I have limited knowledge around Rustdoc, I want to ask someone in Rustdoc team if there is a better way (probably @GuillaumeGomez ?)

@Noratrieb Noratrieb added I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc I-slow Issue: Problems and improvements with respect to performance of generated code. regression-from-stable-to-beta Performance or correctness regression from stable to beta. 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.
Projects
None yet
Development

No branches or pull requests

6 participants