From 2493ef150399adb8ad85234a5ae60de9b8ac2d5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 10 May 2026 15:59:37 +0200 Subject: [PATCH 1/6] rustdoc: `SpanMapVisitor`: Cache `TypeckResults` --- src/doc/rustdoc/src/unstable-features.md | 9 ++ src/librustdoc/html/render/span_map.rs | 122 +++++++++++------ .../jump-to-def/assoc-items-extra.rs | 20 +++ tests/rustdoc-html/jump-to-def/assoc-items.rs | 126 ++++++++++++------ tests/rustdoc-html/jump-to-def/assoc-types.rs | 20 --- .../rustdoc-html/jump-to-def/no-body-items.rs | 24 ---- .../items-nested-in-bodies.rs | 31 +++++ 7 files changed, 228 insertions(+), 124 deletions(-) create mode 100644 tests/rustdoc-html/jump-to-def/assoc-items-extra.rs delete mode 100644 tests/rustdoc-html/jump-to-def/assoc-types.rs delete mode 100644 tests/rustdoc-html/jump-to-def/no-body-items.rs create mode 100644 tests/rustdoc-ui/generate-link-to-definition/items-nested-in-bodies.rs diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index 1c91ad343b8c0..45f79f53f2c53 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -653,6 +653,15 @@ add the `--scrape-tests` flag. This flag enables the generation of links in the source code pages which allow the reader to jump to a type definition. +> [!WARNING] +> In very specific scenarios, enabling this feature may lead to your program getting rejected if you +> rely on rustdoc intentionally not running all semantic analysis passes on function bodies to aid +> with documenting `cfg`-conditional items. +> +> More concretely, rustdoc may choose to type-check bodies if they contain type-dependent paths +> including method calls. This may result in name resolution and type errors getting reported that +> rustdoc would usually suppress. + ### `--test-builder`: `rustc`-like program to build tests * Tracking issue: [#102981](https://github.com/rust-lang/rust/issues/102981) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index ff214bad59f1d..d314f2b55865e 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -1,12 +1,13 @@ use std::path::{Path, PathBuf}; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; +use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId}; +use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, Visitor, VisitorExt}; use rustc_hir::{ExprKind, HirId, Item, ItemKind, Mod, Node, QPath}; use rustc_middle::hir::nested_filter; -use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{self, TyCtxt}; use rustc_span::{BytePos, ExpnKind}; use crate::clean::{self, PrimitiveType, rustc_span}; @@ -82,7 +83,8 @@ pub(crate) fn collect_spans_and_sources( generate_link_to_definition: bool, ) -> (FxIndexMap, FxHashMap) { if include_sources { - let mut visitor = SpanMapVisitor { tcx, matches: FxHashMap::default() }; + let mut visitor = + SpanMapVisitor { tcx, maybe_typeck_results: None, matches: FxHashMap::default() }; if generate_link_to_definition { tcx.hir_walk_toplevel_module(&mut visitor); @@ -96,12 +98,34 @@ pub(crate) fn collect_spans_and_sources( struct SpanMapVisitor<'tcx> { pub(crate) tcx: TyCtxt<'tcx>, + pub(crate) maybe_typeck_results: Option>, pub(crate) matches: FxHashMap, } -impl SpanMapVisitor<'_> { +impl<'tcx> SpanMapVisitor<'tcx> { + /// Returns the typeck results of the current body if we're in one. + /// + /// This will typeck the body if it hasn't been already. Since rustdoc intentionally doesn't run + /// all semantic analysis passes on function bodies at the time of writing, this can lead to us + /// "suddenly" rejecting the user's code under `--generate-link-to-definition` while accepting + /// it if that flag isn't passed! So use this method sparingly and think about the consequences + /// including performance! + /// + /// This behavior is documented in the rustdoc book. Ideally, it wouldn't be that way but no + /// good solution has been found so far. Don't think about adding some sort of flag to rustc to + /// suppress diagnostic emission that would be unsound wrt. `ErrorGuaranteed`[^1] and generally + /// be quite hacky! + /// + /// [^1]: Historical context: + /// . + fn maybe_typeck_results(&mut self) -> Option<&'tcx ty::TypeckResults<'tcx>> { + let results = self.maybe_typeck_results.as_mut()?; + let results = results.cache.get_or_insert_with(|| self.tcx.typeck_body(results.body_id)); + Some(results) + } + /// This function is where we handle `hir::Path` elements and add them into the "span map". - fn handle_path(&mut self, path: &rustc_hir::Path<'_>, only_use_last_segment: bool) { + fn handle_path(&mut self, path: &hir::Path<'_>, only_use_last_segment: bool) { match path.res { // FIXME: For now, we handle `DefKind` if it's not a `DefKind::TyParam`. // Would be nice to support them too alongside the other `DefKind` @@ -218,18 +242,13 @@ impl SpanMapVisitor<'_> { } fn infer_id(&mut self, hir_id: HirId, expr_hir_id: Option, span: Span) { - let tcx = self.tcx; - let body_id = tcx.hir_enclosing_body_owner(hir_id); - // FIXME: this is showing error messages for parts of the code that are not - // compiled (because of cfg)! - // - // See discussion in https://github.com/rust-lang/rust/issues/69426#issuecomment-1019412352 - let typeck_results = tcx.typeck_body(tcx.hir_body_owned_by(body_id).id()); + let Some(typeck_results) = self.maybe_typeck_results() else { return }; + // Interestingly enough, for method calls, we need the whole expression whereas for static // method/function calls, we need the call expression specifically. if let Some(def_id) = typeck_results.type_dependent_def_id(expr_hir_id.unwrap_or(hir_id)) { let link = if def_id.as_local().is_some() { - LinkFromSrc::Local(rustc_span(def_id, tcx)) + LinkFromSrc::Local(rustc_span(def_id, self.tcx)) } else { LinkFromSrc::External(def_id) }; @@ -238,23 +257,6 @@ impl SpanMapVisitor<'_> { } } -// This is a reimplementation of `hir_enclosing_body_owner` which allows to fail without -// panicking. -fn hir_enclosing_body_owner(tcx: TyCtxt<'_>, hir_id: HirId) -> Option { - for (_, node) in tcx.hir_parent_iter(hir_id) { - // FIXME: associated type impl items don't have an associated body, so we don't handle - // them currently. - if let Node::ImplItem(impl_item) = node - && matches!(impl_item.kind, rustc_hir::ImplItemKind::Type(_)) - { - return None; - } else if let Some((def_id, _)) = node.associated_body() { - return Some(def_id); - } - } - None -} - impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { type NestedFilter = nested_filter::All; @@ -262,7 +264,14 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { self.tcx } - fn visit_path(&mut self, path: &rustc_hir::Path<'tcx>, _id: HirId) { + fn visit_nested_body(&mut self, body_id: hir::BodyId) -> Self::Result { + let maybe_typeck_results = + self.maybe_typeck_results.replace(LazyTypeckResults { body_id, cache: None }); + self.visit_body(self.tcx.hir_body(body_id)); + self.maybe_typeck_results = maybe_typeck_results; + } + + fn visit_path(&mut self, path: &hir::Path<'tcx>, _id: HirId) { if self.handle_macro(path.span) { return; } @@ -272,25 +281,37 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { fn visit_qpath(&mut self, qpath: &QPath<'tcx>, id: HirId, _span: rustc_span::Span) { match *qpath { - QPath::TypeRelative(qself, path) => { - if matches!(path.res, Res::Err) { - let tcx = self.tcx; - if let Some(body_id) = hir_enclosing_body_owner(tcx, id) { - let typeck_results = tcx.typeck_body(tcx.hir_body_owned_by(body_id).id()); - let path = rustc_hir::Path { + QPath::TypeRelative(qself, segment) => { + if let Res::Err = segment.res { + // FIXME: This doesn't work for paths in *types* since HIR ty lowering currently + // doesn't write back the resolution of type-relative paths. Updating it + // to do so should be a simple fix. + // FIXME: This obviously doesn't support item signatures / non-bodies. Sadly, + // rustc currently doesn't keep around that information & thus can't + // provide an API for it. + // `ItemCtxt`s would need a place to write back the resolution of type- + // dependent definitions. Ideally there was some sort of query keyed on + // the `LocalDefId` of the owning item that returns some table with which + // we can map the `HirId` to a `DefId`. + // Of course, we could re-HIR-ty-lower such paths *here* if we were to + // extend the public API of HIR analysis. However, I strongly advise + // against it as it would be too much of a hack. + if let Some(typeck_results) = self.maybe_typeck_results() { + let path = hir::Path { // We change the span to not include parens. - span: path.ident.span, + span: segment.ident.span, res: typeck_results.qpath_res(qpath, id), + // FIXME(fmease): Don't create a path with zero segments! segments: &[], }; self.handle_path(&path, false); } } else { - self.infer_id(path.hir_id, Some(id), path.ident.span.into()); + self.infer_id(segment.hir_id, Some(id), segment.ident.span.into()); } rustc_ast::visit::try_visit!(self.visit_ty_unambig(qself)); - self.visit_path_segment(path); + self.visit_path_segment(segment); } QPath::Resolved(maybe_qself, path) => { self.handle_path(path, true); @@ -323,11 +344,17 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { intravisit::walk_mod(self, m); } - fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) { + fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) { match expr.kind { ExprKind::MethodCall(segment, ..) => { self.infer_id(segment.hir_id, Some(expr.hir_id), segment.ident.span.into()) } + // FIXME(fmease): We needlessly request `TypeckResults` even if the callee isn't + // type-relative. In the majority of cases, it's just gonna be a + // `Resolved` path meaning we can end up unnecessarily + // `typeck`'ing the body which is super costly! + // Moreover, if it actually is a type-relative path, we end up + // "resolving" it twice (with slightly different spans). ExprKind::Call(call, ..) => self.infer_id(call.hir_id, None, call.span.into()), _ => { if self.handle_macro(expr.span) { @@ -340,6 +367,10 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { } fn visit_item(&mut self, item: &'tcx Item<'tcx>) { + // We're no longer in a body since we've crossed an item boundary. + // Temporarily take away the typeck results which are only valid in bodies. + let maybe_typeck_results = self.maybe_typeck_results.take(); + match item.kind { ItemKind::Static(..) | ItemKind::Const(..) @@ -359,6 +390,15 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { // We already have "visit_mod" above so no need to check it here. | ItemKind::Mod(..) => {} } + intravisit::walk_item(self, item); + + self.maybe_typeck_results = maybe_typeck_results; } } + +/// Lazily computed & cached [`ty::TypeckResults`]. +struct LazyTypeckResults<'tcx> { + body_id: hir::BodyId, + cache: Option<&'tcx ty::TypeckResults<'tcx>>, +} diff --git a/tests/rustdoc-html/jump-to-def/assoc-items-extra.rs b/tests/rustdoc-html/jump-to-def/assoc-items-extra.rs new file mode 100644 index 0000000000000..0e15f32048f6b --- /dev/null +++ b/tests/rustdoc-html/jump-to-def/assoc-items-extra.rs @@ -0,0 +1,20 @@ +// Like test `assoc-items.rs` but now utilizing unstable features. +// FIXME: Make use of (m)GCA assoc consts once they no longer ICE! +//@ compile-flags: -Zunstable-options --generate-link-to-definition +#![feature(return_type_notation)] + +//@ has 'src/assoc_items_extra/assoc-items-extra.rs.html' + +trait Trait0 { + fn fn0() -> impl Sized; + fn fn1() -> impl Sized; +} + +fn item() +where + //@ has - '//a[@href="#9"]' 'fn0' + ::fn0(..): Copy, // Item, AssocFn, Resolved + // FIXME: Support this: + //@ !has - '//a[@href="#10"]' 'fn1' + T::fn1(..): Copy, // Item, AssocFn, TypeRelative +{} diff --git a/tests/rustdoc-html/jump-to-def/assoc-items.rs b/tests/rustdoc-html/jump-to-def/assoc-items.rs index a434fa7e6053e..f176b39625605 100644 --- a/tests/rustdoc-html/jump-to-def/assoc-items.rs +++ b/tests/rustdoc-html/jump-to-def/assoc-items.rs @@ -1,58 +1,106 @@ -// This test ensures that patterns also get a link generated. - //@ compile-flags: -Zunstable-options --generate-link-to-definition -#![crate_name = "foo"] - -//@ has 'src/foo/assoc-items.rs.html' +//@ has 'src/assoc_items/assoc-items.rs.html' -pub trait Trait { - type T; +trait Trait0 { + fn fn0(); + fn fn1(); + fn fn2(); + fn fn3(); + const CT0: usize; + const CT1: usize; + type Ty0; + type Ty1; + type Ty2: Bound0; + type Ty3: Bound0; } -pub trait Another { - type T; - const X: u32; + +trait Bound0 { + fn fn0(); + const CT0: usize; } -pub struct Foo; +fn expr() { + //@ has - '//a[@href="#6"]' 'fn0' + let _ = ::fn0; // Expr, AssocFn, Resolved + //@ has - '//a[@href="#7"]' 'fn1' + let _ = T::fn1; // Expr, AssocFn, TypeRelative -impl Foo { - pub fn new() -> Self { Foo } -} + //@ has - '//a[@href="#8"]' 'fn2' + let _ = ::fn2(); // Expr, AssocFn, Resolved + //@ has - '//a[@href="#9"]' 'fn3' + let _ = T::fn3(); // Expr, AssocFn, TypeRelative -pub struct C; + //@ has - '//a[@href="#10"]' 'CT0' + let _ = ::CT0; // Expr, AssocConst, Resolved + //@ has - '//a[@href="#11"]' 'CT1' + let _ = ::CT1; // Expr, AssocConst, TypeRelative -impl C { - pub fn wat() {} -} + //@ has - '//a[@href="#12"]' 'Ty0' + let _: ::Ty0; // Expr, AssocTy, Resolved + // FIXME: Support this: + //@ !has - '//a[@href="#13"]' 'Ty1' + let _: T::Ty1; // Expr, AssocTy, TypeRelative -// These two links must not change and in particular must contain `/derive.`! -//@ has - '//a[@href="{{channel}}/core/fmt/macros/derive.Debug.html"]' 'Debug' -//@ has - '//a[@href="{{channel}}/core/cmp/derive.PartialEq.html"]' 'PartialEq' -#[derive(Debug, PartialEq)] -pub struct Bar; -impl Trait for Bar { - type T = Foo; + //@ has - '//a[@href="#14"]' 'Ty2' + //@ has - '//a[@href="#19"]' 'fn0' + let _ = ::Ty2::fn0(); + + // FIXME: Support this: + //@ !has - '//a[@href="#14"]' 'Ty3' + //@ has - '//a[@href="#20"]' 'CT0' + let _ = T::Ty2::CT0; } -impl Another for Bar { - type T = C; - const X: u32 = 12; + +trait Trait1 { + const CT0: usize; + const CT1: usize; + const CT2: usize; + + fn scope(); } -pub fn bar() { - //@ has - '//a[@href="#20"]' 'new' - ::T::new(); - //@ has - '//a[@href="#26"]' 'wat' - ::T::wat(); - match 12u32 { - //@ has - '//a[@href="#14"]' 'X' - ::X => {} +fn pat() { + //@ has - '//a[@href="#56"]' 'CT0' + if let <() as Trait1>::CT0 = 0 {} // Pat, AssocConst, Resolved + + match 0 { + //@ has - '//a[@href="#57"]' 'CT1' + <() as Trait1>::CT1 => {} // Pat, AssocConst, Resolved _ => {} } } -pub struct Far { - //@ has - '//a[@href="#10"]' 'T' - x: ::T, +impl Trait1 for () { + const CT0: usize = 0; + const CT1: usize = 1; + const CT2: usize = 2; + + fn scope() { + //@ has - '//a[@href="#58"]' 'CT2' + if let Self::CT2 = 0 {} // Pat, AssocConst, TypeRelative + } +} + +trait Trait2 { + const CT0: usize; + type Ty0; + type Ty1; +} + +impl Trait2 for () { + const CT0: usize = 0; + type Ty0 = (); + type Ty1 = (); +} + +struct Item { + //@ has - '//a[@href="#87"]' 'CT0' + f0: [(); <() as Trait2>::CT0], // Item, AssocConst, Resolved + //@ has - '//a[@href="#88"]' 'Ty0' + f1: ::Ty0, // Item, AssocTy, Resolved + // FIXME: Support this: + //@ !has - '//a[@href="#89"]' 'Ty1' + f2: T::Ty1, // Item, AssocTy, TypeRelative } diff --git a/tests/rustdoc-html/jump-to-def/assoc-types.rs b/tests/rustdoc-html/jump-to-def/assoc-types.rs deleted file mode 100644 index f430eaf16389a..0000000000000 --- a/tests/rustdoc-html/jump-to-def/assoc-types.rs +++ /dev/null @@ -1,20 +0,0 @@ -// This test ensures that associated types don't crash rustdoc jump to def. - -//@ compile-flags: -Zunstable-options --generate-link-to-definition - - -#![crate_name = "foo"] - -//@ has 'src/foo/assoc-types.rs.html' - -pub trait Trait { - type Node; -} - -pub fn y() { - struct X(G); - - impl Trait for X { - type Node = G::Node; - } -} diff --git a/tests/rustdoc-html/jump-to-def/no-body-items.rs b/tests/rustdoc-html/jump-to-def/no-body-items.rs deleted file mode 100644 index e74640072146e..0000000000000 --- a/tests/rustdoc-html/jump-to-def/no-body-items.rs +++ /dev/null @@ -1,24 +0,0 @@ -// This test ensures that items with no body don't panic when generating -// jump to def links. - -//@ compile-flags: -Zunstable-options --generate-link-to-definition - -#![crate_name = "foo"] - -//@ has 'src/foo/no-body-items.rs.html' - -pub trait A { - type T; - type U; -} - -impl A for () { - type T = Self::U; - type U = (); -} - -pub trait C { - type X; -} - -pub struct F(pub T::X); diff --git a/tests/rustdoc-ui/generate-link-to-definition/items-nested-in-bodies.rs b/tests/rustdoc-ui/generate-link-to-definition/items-nested-in-bodies.rs new file mode 100644 index 0000000000000..7d48e885c3f26 --- /dev/null +++ b/tests/rustdoc-ui/generate-link-to-definition/items-nested-in-bodies.rs @@ -0,0 +1,31 @@ +// When trying to resolve a type-relative path (e.g., `T::Item` where `T` is a type param) in an +// item that's nested inside of a body (e.g., of a function or constant), we once tried to look up +// the definition in the `TypeckResults` of the body owner which is wrong and led to compiler ICEs. +// +// We now make sure to invalidate the `TypeckResults` when crossing a body - item border. +// +// For additional context, `TypeckResults` as returned by queries like `typeck` store the typeck +// results of *bodies* only. In item signatures / non-bodies, there's no equivalent at the time of +// writing, so it's impossible to resolve HIR TypeRelative paths (identified by a `HirId`) to their +// definition (`DefId`) in other parts of the compiler / in tools. + +//@ compile-flags: -Zunstable-options --generate-link-to-definition +//@ check-pass +// issue: + +fn scope() { + struct X(T::Item); + + trait Trait { + type Ty; + + fn func() + where + T::Item: Copy + {} + } + + impl Trait for T { + type Ty = T::Item; + } +} From b0eb4ffb00008891ed797eccda532a4246fd9b55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 10 May 2026 16:26:46 +0200 Subject: [PATCH 2/6] Don't check if the resolution of `TypeRelative` paths is `Err` because it always is `rustc_resolve` doesn't resolve type-relative paths since that's the job of HIR ty lowering and HIR typeck. `segment.res` comes from `rustc_resolve` and is thus always `Res::Err`. So just try to obtain the `TypeckResults` immediately since they contain the actual resolution as deduced by HIR typeck. --- src/librustdoc/html/render/span_map.rs | 48 ++++++++++++-------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index d314f2b55865e..324c79ff2a43c 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -282,32 +282,28 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { fn visit_qpath(&mut self, qpath: &QPath<'tcx>, id: HirId, _span: rustc_span::Span) { match *qpath { QPath::TypeRelative(qself, segment) => { - if let Res::Err = segment.res { - // FIXME: This doesn't work for paths in *types* since HIR ty lowering currently - // doesn't write back the resolution of type-relative paths. Updating it - // to do so should be a simple fix. - // FIXME: This obviously doesn't support item signatures / non-bodies. Sadly, - // rustc currently doesn't keep around that information & thus can't - // provide an API for it. - // `ItemCtxt`s would need a place to write back the resolution of type- - // dependent definitions. Ideally there was some sort of query keyed on - // the `LocalDefId` of the owning item that returns some table with which - // we can map the `HirId` to a `DefId`. - // Of course, we could re-HIR-ty-lower such paths *here* if we were to - // extend the public API of HIR analysis. However, I strongly advise - // against it as it would be too much of a hack. - if let Some(typeck_results) = self.maybe_typeck_results() { - let path = hir::Path { - // We change the span to not include parens. - span: segment.ident.span, - res: typeck_results.qpath_res(qpath, id), - // FIXME(fmease): Don't create a path with zero segments! - segments: &[], - }; - self.handle_path(&path, false); - } - } else { - self.infer_id(segment.hir_id, Some(id), segment.ident.span.into()); + // FIXME: This doesn't work for paths in *types* since HIR ty lowering currently + // doesn't write back the resolution of type-relative paths. Updating it to + // do so should be a simple fix. + // FIXME: This obviously doesn't support item signatures / non-bodies. Sadly, rustc + // currently doesn't keep around that information & thus can't provide an API + // for it. + // `ItemCtxt`s would need a place to write back the resolution of type- + // dependent definitions. Ideally there was some sort of query keyed on the + // `LocalDefId` of the owning item that returns some table with which we can + // map the `HirId` to a `DefId`. + // Of course, we could re-HIR-ty-lower such paths *here* if we were to extend + // the public API of HIR analysis. However, I strongly advise against it as + // it would be too much of a hack. + if let Some(typeck_results) = self.maybe_typeck_results() { + let path = hir::Path { + // We change the span to not include parens. + span: segment.ident.span, + res: typeck_results.qpath_res(qpath, id), + // FIXME(fmease): Don't create a path with zero segments! + segments: &[], + }; + self.handle_path(&path, false); } rustc_ast::visit::try_visit!(self.visit_ty_unambig(qself)); From f5ad60c7daa7280f55cf9bc9eaee83de402bc946 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 10 May 2026 16:49:39 +0200 Subject: [PATCH 3/6] Inline non-self-descriptive fn `infer_id` --- src/librustdoc/html/render/span_map.rs | 53 +++++++++++--------------- 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 324c79ff2a43c..6c847effe0b39 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -124,6 +124,14 @@ impl<'tcx> SpanMapVisitor<'tcx> { Some(results) } + fn link_for_def(&self, def_id: DefId) -> LinkFromSrc { + if def_id.is_local() { + LinkFromSrc::Local(rustc_span(def_id, self.tcx)) + } else { + LinkFromSrc::External(def_id) + } + } + /// This function is where we handle `hir::Path` elements and add them into the "span map". fn handle_path(&mut self, path: &hir::Path<'_>, only_use_last_segment: bool) { match path.res { @@ -131,11 +139,6 @@ impl<'tcx> SpanMapVisitor<'tcx> { // Would be nice to support them too alongside the other `DefKind` // (such as primitive types!). Res::Def(kind, def_id) if kind != DefKind::TyParam => { - let link = if def_id.as_local().is_some() { - LinkFromSrc::Local(rustc_span(def_id, self.tcx)) - } else { - LinkFromSrc::External(def_id) - }; // In case the path ends with generics, we remove them from the span. let span = if only_use_last_segment && let Some(path_span) = path.segments.last().map(|segment| segment.ident.span) @@ -156,7 +159,7 @@ impl<'tcx> SpanMapVisitor<'tcx> { }) .unwrap_or(path.span) }; - self.matches.insert(span.into(), link); + self.matches.insert(span.into(), self.link_for_def(def_id)); } Res::Local(_) if let Some(span) = self.tcx.hir_res_span(path.res) => { let path_span = if only_use_last_segment @@ -240,21 +243,6 @@ impl<'tcx> SpanMapVisitor<'tcx> { self.matches.insert(new_span.into(), link_from_src); true } - - fn infer_id(&mut self, hir_id: HirId, expr_hir_id: Option, span: Span) { - let Some(typeck_results) = self.maybe_typeck_results() else { return }; - - // Interestingly enough, for method calls, we need the whole expression whereas for static - // method/function calls, we need the call expression specifically. - if let Some(def_id) = typeck_results.type_dependent_def_id(expr_hir_id.unwrap_or(hir_id)) { - let link = if def_id.as_local().is_some() { - LinkFromSrc::Local(rustc_span(def_id, self.tcx)) - } else { - LinkFromSrc::External(def_id) - }; - self.matches.insert(span, link); - } - } } impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { @@ -341,23 +329,26 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { } fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) { - match expr.kind { - ExprKind::MethodCall(segment, ..) => { - self.infer_id(segment.hir_id, Some(expr.hir_id), segment.ident.span.into()) + let mut handle_type_dependent_def = |hir_id: HirId, span: rustc_span::Span| { + // Exprs *have* to exist in a body, so typeck results should always be available. + let typeck_results = self.maybe_typeck_results().unwrap(); + if let Some(def_id) = typeck_results.type_dependent_def_id(hir_id) { + self.matches.insert(span.into(), self.link_for_def(def_id)); } + }; + + match expr.kind { + ExprKind::MethodCall(seg, ..) => handle_type_dependent_def(expr.hir_id, seg.ident.span), // FIXME(fmease): We needlessly request `TypeckResults` even if the callee isn't // type-relative. In the majority of cases, it's just gonna be a // `Resolved` path meaning we can end up unnecessarily // `typeck`'ing the body which is super costly! // Moreover, if it actually is a type-relative path, we end up // "resolving" it twice (with slightly different spans). - ExprKind::Call(call, ..) => self.infer_id(call.hir_id, None, call.span.into()), - _ => { - if self.handle_macro(expr.span) { - // We don't want to go deeper into the macro. - return; - } - } + ExprKind::Call(callee, ..) => handle_type_dependent_def(callee.hir_id, callee.span), + // We don't want to go deeper into the macro. + _ if self.handle_macro(expr.span) => return, + _ => {} } intravisit::walk_expr(self, expr); } From bfcaf513b4819cfd3dd2b3e930dc702db05e7f87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 10 May 2026 18:23:50 +0200 Subject: [PATCH 4/6] Don't create a path with zero segments --- src/librustdoc/html/render/span_map.rs | 43 +++++++++----------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 6c847effe0b39..824eb758937f6 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -135,37 +135,26 @@ impl<'tcx> SpanMapVisitor<'tcx> { /// This function is where we handle `hir::Path` elements and add them into the "span map". fn handle_path(&mut self, path: &hir::Path<'_>, only_use_last_segment: bool) { match path.res { - // FIXME: For now, we handle `DefKind` if it's not a `DefKind::TyParam`. - // Would be nice to support them too alongside the other `DefKind` - // (such as primitive types!). - Res::Def(kind, def_id) if kind != DefKind::TyParam => { + // FIXME: Properly support type parameters. Note they resolve just fine. The issue is + // that our highlighter would then also linkify their *definition site* for some reason + // linking them to themselves. Const parameters don't exhibit this issue. + Res::Def(DefKind::TyParam, _) => {} + Res::Def(_, def_id) => { + // The segments can be empty for `use *;` in a non-crate-root scope in Rust 2015. + let span = path.segments.last().map_or(path.span, |seg| seg.ident.span); // In case the path ends with generics, we remove them from the span. - let span = if only_use_last_segment - && let Some(path_span) = path.segments.last().map(|segment| segment.ident.span) - { - path_span + let span = if only_use_last_segment { + span } else { - path.segments - .last() - .map(|last| { - // In `use` statements, the included item is not in the path segments. - // However, it doesn't matter because you can't have generics on `use` - // statements. - if path.span.contains(last.ident.span) { - path.span.with_hi(last.ident.span.hi()) - } else { - path.span - } - }) - .unwrap_or(path.span) + // In `use` statements, the included item is not in the path segments. However, + // it doesn't matter because you can't have generics on `use` statements. + if path.span.contains(span) { path.span.with_hi(span.hi()) } else { path.span } }; self.matches.insert(span.into(), self.link_for_def(def_id)); } Res::Local(_) if let Some(span) = self.tcx.hir_res_span(path.res) => { - let path_span = if only_use_last_segment - && let Some(path_span) = path.segments.last().map(|segment| segment.ident.span) - { - path_span + let path_span = if only_use_last_segment { + path.segments.last().unwrap().ident.span } else { path.span }; @@ -176,7 +165,6 @@ impl<'tcx> SpanMapVisitor<'tcx> { self.matches .insert(path.span.into(), LinkFromSrc::Primitive(PrimitiveType::from(p))); } - Res::Err => {} _ => {} } } @@ -288,8 +276,7 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { // We change the span to not include parens. span: segment.ident.span, res: typeck_results.qpath_res(qpath, id), - // FIXME(fmease): Don't create a path with zero segments! - segments: &[], + segments: std::slice::from_ref(segment), }; self.handle_path(&path, false); } From efaf441d7b37a28462fd006d8ca64271c8cbb0a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 10 May 2026 19:55:04 +0200 Subject: [PATCH 5/6] Don't special-case `ExprKind::Call` Don't unnecessarily try to obtain the type-dependent definition of callees in `visit_expr`, just let `visit_qpath` handle callees. This means that for callees that are * `Resolved` paths (the majority of callees) we don't try to `typeck` the enclosing body which should improve perf if the body doesn't contain any type-dependent definitions. * actually `TypeRelative` paths we don't resolve them twice (with slightly different spans) --- src/librustdoc/html/render/span_map.rs | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 824eb758937f6..863ce3a79f4e6 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -316,23 +316,14 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { } fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) { - let mut handle_type_dependent_def = |hir_id: HirId, span: rustc_span::Span| { - // Exprs *have* to exist in a body, so typeck results should always be available. - let typeck_results = self.maybe_typeck_results().unwrap(); - if let Some(def_id) = typeck_results.type_dependent_def_id(hir_id) { - self.matches.insert(span.into(), self.link_for_def(def_id)); - } - }; - match expr.kind { - ExprKind::MethodCall(seg, ..) => handle_type_dependent_def(expr.hir_id, seg.ident.span), - // FIXME(fmease): We needlessly request `TypeckResults` even if the callee isn't - // type-relative. In the majority of cases, it's just gonna be a - // `Resolved` path meaning we can end up unnecessarily - // `typeck`'ing the body which is super costly! - // Moreover, if it actually is a type-relative path, we end up - // "resolving" it twice (with slightly different spans). - ExprKind::Call(callee, ..) => handle_type_dependent_def(callee.hir_id, callee.span), + ExprKind::MethodCall(segment, ..) => { + // Exprs *have* to exist in a body, so typeck results should always be available. + let typeck_results = self.maybe_typeck_results().unwrap(); + if let Some(def_id) = typeck_results.type_dependent_def_id(expr.hir_id) { + self.matches.insert(segment.ident.span.into(), self.link_for_def(def_id)); + } + } // We don't want to go deeper into the macro. _ if self.handle_macro(expr.span) => return, _ => {} From e205784d77f5e84e5505b2b77536ad2d89961ff6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 13 May 2026 03:50:04 +0200 Subject: [PATCH 6/6] Don't try to resolve type-dependent paths in anon consts --- src/librustdoc/html/render/span_map.rs | 16 +++++++++++++--- .../generate-link-to-definition/anon-consts.rs | 11 +++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 tests/rustdoc-ui/generate-link-to-definition/anon-consts.rs diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 863ce3a79f4e6..517b538d1bfd1 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -247,6 +247,16 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { self.maybe_typeck_results = maybe_typeck_results; } + fn visit_anon_const(&mut self, ct: &'tcx hir::AnonConst) { + // FIXME: Typeck'ing anon consts leads to ICEs in rustc if the parent body wasn't typeck'ed + // yet. See #156418. Figure out what the best and proper solution for this is. Until + // then, let's prevent `typeck` from being called on anon consts by not setting + // `maybe_typeck_results` to `Some(_)`. + let maybe_typeck_results = self.maybe_typeck_results.take(); + self.visit_body(self.tcx.hir_body(ct.body)); + self.maybe_typeck_results = maybe_typeck_results; + } + fn visit_path(&mut self, path: &hir::Path<'tcx>, _id: HirId) { if self.handle_macro(path.span) { return; @@ -318,9 +328,9 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) { match expr.kind { ExprKind::MethodCall(segment, ..) => { - // Exprs *have* to exist in a body, so typeck results should always be available. - let typeck_results = self.maybe_typeck_results().unwrap(); - if let Some(def_id) = typeck_results.type_dependent_def_id(expr.hir_id) { + if let Some(typeck_results) = self.maybe_typeck_results() + && let Some(def_id) = typeck_results.type_dependent_def_id(expr.hir_id) + { self.matches.insert(segment.ident.span.into(), self.link_for_def(def_id)); } } diff --git a/tests/rustdoc-ui/generate-link-to-definition/anon-consts.rs b/tests/rustdoc-ui/generate-link-to-definition/anon-consts.rs new file mode 100644 index 0000000000000..3a774b0d9ca99 --- /dev/null +++ b/tests/rustdoc-ui/generate-link-to-definition/anon-consts.rs @@ -0,0 +1,11 @@ +// Ensure that we don't crash on anonymous constants. +// FIXME: Ideally, we would actually support linkifying type-dependent paths in +// anon consts but for now that's disabled until we figure out what the +// proper solution is implementation-wise. +// issue: +//@ check-pass + +fn scope() { + struct Hold; + let _ = X::<{ 0usize.saturating_add(1) }>; +}