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..517b538d1bfd1 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,49 +98,63 @@ 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) + } + + 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: &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` - // (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) - }; + // 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(), 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 - && 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 }; @@ -149,7 +165,6 @@ impl SpanMapVisitor<'_> { self.matches .insert(path.span.into(), LinkFromSrc::Primitive(PrimitiveType::from(p))); } - Res::Err => {} _ => {} } } @@ -216,43 +231,6 @@ impl SpanMapVisitor<'_> { 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 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()); - // 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)) - } else { - LinkFromSrc::External(def_id) - }; - self.matches.insert(span, link); - } - } -} - -// 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> { @@ -262,7 +240,24 @@ 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_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; } @@ -272,25 +267,32 @@ 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 { - // We change the span to not include parens. - span: path.ident.span, - res: typeck_results.qpath_res(qpath, id), - segments: &[], - }; - self.handle_path(&path, false); - } - } else { - self.infer_id(path.hir_id, Some(id), path.ident.span.into()); + QPath::TypeRelative(qself, segment) => { + // 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), + segments: std::slice::from_ref(segment), + }; + self.handle_path(&path, false); } 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,23 +325,27 @@ 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()) - } - 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; + 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)); } } + // We don't want to go deeper into the macro. + _ if self.handle_macro(expr.span) => return, + _ => {} } intravisit::walk_expr(self, expr); } 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 +365,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/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) }>; +} 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; + } +}