Skip to content

Fix jump to def link generation on primitive type associated methods#156736

Open
GuillaumeGomez wants to merge 1 commit into
rust-lang:mainfrom
GuillaumeGomez:primitive-assoc-methods
Open

Fix jump to def link generation on primitive type associated methods#156736
GuillaumeGomez wants to merge 1 commit into
rust-lang:mainfrom
GuillaumeGomez:primitive-assoc-methods

Conversation

@GuillaumeGomez
Copy link
Copy Markdown
Member

@GuillaumeGomez GuillaumeGomez commented May 19, 2026

Fixes #156707.

Interestingly enough, the inference fails on primitive type, so instead I go around a bit by generating a PrimitiveType and then tweak a bit the href generation.

r? @fmease

@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 May 19, 2026
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the primitive-assoc-methods branch from c139ad7 to af73ce7 Compare May 19, 2026 17:08
@rust-bors

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the primitive-assoc-methods branch from af73ce7 to 260409a Compare May 19, 2026 21:21
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 19, 2026

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.

@GuillaumeGomez GuillaumeGomez force-pushed the primitive-assoc-methods branch from 260409a to d1bcc86 Compare May 22, 2026 10:29
Copy link
Copy Markdown
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Preliminary review

View changes since this review

Comment thread src/librustdoc/html/format.rs Outdated
Comment thread src/librustdoc/clean/types.rs Outdated
@GuillaumeGomez GuillaumeGomez force-pushed the primitive-assoc-methods branch from d1bcc86 to a1dba63 Compare May 22, 2026 12:42
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 22, 2026

The GCC codegen subtree was changed

cc @antoyo

@GuillaumeGomez GuillaumeGomez force-pushed the primitive-assoc-methods branch from a1dba63 to 300dc46 Compare May 22, 2026 12:43
@rust-log-analyzer

This comment has been minimized.

Comment thread src/librustdoc/html/span_map.rs Outdated
@GuillaumeGomez GuillaumeGomez force-pushed the primitive-assoc-methods branch from 300dc46 to 8951062 Compare May 22, 2026 12:52
if def_id != original_def_id && matches!(tcx.def_kind(def_id), DefKind::Impl { .. }) {
let infcx = tcx.infer_ctxt().build(TypingMode::non_body_analysis());
def_id = infcx
let ty = tcx.type_of(def_id);
Copy link
Copy Markdown
Member

@fmease fmease May 23, 2026

Choose a reason for hiding this comment

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

Could you use the normalized type for both the ADT check and the primitive one? So set ty to the normalized type? Something like the following (pseudo code):

let ty = tcx.type_of(def_id);
let ty =
    ...query_normalize(...ty...)
    .map(...resolve_vars_if_possible...)
    .unwrap_or(ty);

That's because I'm relatively sure we're currently failing at finding the definition if the impl contains free alias types (unlikely scenario but normalizing is simply the correct thing to do).

Consider:

  • rustc a.rs --crate-type=lib:
    #![feature(lazy_type_alias, rustc_attrs)]
    #![rustc_coherence_is_core]
    
    type Identity<T> = T;
    
    impl Identity<u8> {
        pub fn inherent(self) {}
    }
  • rustdoc b.rs -L. --generate-link-to-definition -Zunstable-options:
    fn scope() {
        extern crate a;
        0u8.inherent();
    }

So under your PR I presume inherent still 404s but please double-check that locally.

View changes since the review

Comment on lines +454 to +455
// If the parent is an impl of a primitive type, it'll fail because primitives aren't
// ADT, so instead we use `type_of` to get the actual primitive type.
Copy link
Copy Markdown
Member

@fmease fmease May 23, 2026

Choose a reason for hiding this comment

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

Suggested change
// If the parent is an impl of a primitive type, it'll fail because primitives aren't
// ADT, so instead we use `type_of` to get the actual primitive type.

I mean we're also using type_of in the ADT case. It's just that we first check if it's an ADT then if it's a primitive if it's not the former.

View changes since the review

Comment on lines +460 to +466
let (fqp, shortty): (Vec<Symbol>, ItemType) = match prim {
Some(prim) => (vec![crate_name, prim.as_sym()], ItemType::Primitive),
None => {
let relative = clean::inline::item_relative_path(tcx, def_id);
(once(crate_name).chain(relative).collect(), ItemType::from_def_id(def_id, tcx))
}
};
Copy link
Copy Markdown
Member

@fmease fmease May 23, 2026

Choose a reason for hiding this comment

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

Suggested change
let (fqp, shortty): (Vec<Symbol>, ItemType) = match prim {
Some(prim) => (vec![crate_name, prim.as_sym()], ItemType::Primitive),
None => {
let relative = clean::inline::item_relative_path(tcx, def_id);
(once(crate_name).chain(relative).collect(), ItemType::from_def_id(def_id, tcx))
}
};
let mut fqp = vec![crate_name];
let shortty = if let Some(prim) = prim {
fqp.push(prim.as_sym());
ItemType::Primitive
} else {
fqp.append(&mut clean::inline::item_relative_path(tcx, def_id));
ItemType::from_def_id(def_id, tcx)
};

View changes since the review

@fmease
Copy link
Copy Markdown
Member

fmease commented May 23, 2026

Then r=me, thanks!

@fmease fmease 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 May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

rustdoc generates broken link for inherent methods on primitive types on source code pages under --generate-link-to-definition

4 participants