From 55bc8e93c8cb2124f5bbe8aac3c6a50cb4879c38 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 7 Mar 2024 09:30:55 +0000 Subject: [PATCH] Don't let rustdoc source code urls look at bodies It is too fragile (keeps ICEing) and will only ever work for a single cfg, but rustdoc looks at all cfgs at the same time --- src/librustdoc/html/render/span_map.rs | 49 +------------------ .../check-source-code-urls-to-def-std.rs | 5 -- .../rustdoc/check-source-code-urls-to-def.rs | 10 ++-- tests/rustdoc/jump-to-def-doc-links-calls.rs | 27 ---------- tests/rustdoc/jump-to-non-local-method.rs | 48 ------------------ 5 files changed, 6 insertions(+), 133 deletions(-) delete mode 100644 tests/rustdoc/jump-to-def-doc-links-calls.rs delete mode 100644 tests/rustdoc/jump-to-non-local-method.rs diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 8ee35db56f8dd..78b0c9ee5a41d 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -5,8 +5,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::{ExprKind, HirId, Item, ItemKind, Mod, Node}; -use rustc_middle::hir::nested_filter; +use rustc_hir::{HirId, Item, ItemKind, Mod, Node}; use rustc_middle::ty::TyCtxt; use rustc_span::hygiene::MacroKind; use rustc_span::{BytePos, ExpnKind, Span}; @@ -49,7 +48,7 @@ pub(crate) fn collect_spans_and_sources( if include_sources { if generate_link_to_definition { - tcx.hir().walk_toplevel_module(&mut visitor); + tcx.hir().visit_all_item_likes_in_crate(&mut visitor); } let sources = sources::collect_local_sources(tcx, src_root, krate); (sources, visitor.matches) @@ -154,37 +153,9 @@ impl<'tcx> SpanMapVisitor<'tcx> { self.matches.insert(new_span, link_from_src); true } - - fn handle_call(&mut self, hir_id: HirId, expr_hir_id: Option, span: Span) { - let hir = self.tcx.hir(); - let body_id = 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 = self - .tcx - .typeck_body(hir.maybe_body_owned_by(body_id).expect("a body which isn't a body")); - // 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> { - type NestedFilter = nested_filter::All; - - fn nested_visit_map(&mut self) -> Self::Map { - self.tcx.hir() - } - fn visit_path(&mut self, path: &rustc_hir::Path<'tcx>, _id: HirId) { if self.handle_macro(path.span) { return; @@ -212,22 +183,6 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { intravisit::walk_mod(self, m, id); } - fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) { - match expr.kind { - ExprKind::MethodCall(segment, ..) => { - self.handle_call(segment.hir_id, Some(expr.hir_id), segment.ident.span) - } - ExprKind::Call(call, ..) => self.handle_call(call.hir_id, None, call.span), - _ => { - if self.handle_macro(expr.span) { - // We don't want to go deeper into the macro. - return; - } - } - } - intravisit::walk_expr(self, expr); - } - fn visit_item(&mut self, item: &'tcx Item<'tcx>) { match item.kind { ItemKind::Static(_, _, _) diff --git a/tests/rustdoc/check-source-code-urls-to-def-std.rs b/tests/rustdoc/check-source-code-urls-to-def-std.rs index fac2a94b81507..18f6344bf7a3a 100644 --- a/tests/rustdoc/check-source-code-urls-to-def-std.rs +++ b/tests/rustdoc/check-source-code-urls-to-def-std.rs @@ -8,8 +8,6 @@ fn babar() {} // @has - '//a[@href="{{channel}}/std/primitive.u32.html"]' 'u32' // @has - '//a[@href="{{channel}}/std/primitive.str.html"]' 'str' -// @has - '//a[@href="{{channel}}/std/primitive.bool.html"]' 'bool' -// @has - '//a[@href="#7"]' 'babar' pub fn foo(a: u32, b: &str, c: String) { let x = 12; let y: bool = true; @@ -33,10 +31,7 @@ pub fn another_foo() { // can't find any item or anything that could tell us that it comes from expansion. // @!has - '//a[@href="#19"]' 'yolo!' yolo!(); - // @has - '//a[@href="{{channel}}/std/macro.eprintln.html"]' 'eprintln!' eprintln!(); - // @has - '//a[@href="#27-29"]' 'data!' let x = data!(4); - // @has - '//a[@href="#23-25"]' 'bar!' bar!(x); } diff --git a/tests/rustdoc/check-source-code-urls-to-def.rs b/tests/rustdoc/check-source-code-urls-to-def.rs index 30638992ac71a..d7510c5be79bd 100644 --- a/tests/rustdoc/check-source-code-urls-to-def.rs +++ b/tests/rustdoc/check-source-code-urls-to-def.rs @@ -14,7 +14,7 @@ extern crate source_code; #[path = "auxiliary/source-code-bar.rs"] pub mod bar; -// @count - '//pre[@class="rust"]//a[@href="auxiliary/source-code-bar.rs.html#5-7"]' 4 +// @count - '//pre[@class="rust"]//a[@href="auxiliary/source-code-bar.rs.html#5-7"]' 2 use bar::Bar; // @has - '//pre[@class="rust"]//a[@href="auxiliary/source-code-bar.rs.html#13-17"]' 'self' // @has - '//pre[@class="rust"]//a[@href="auxiliary/source-code-bar.rs.html#14-16"]' 'Trait' @@ -31,7 +31,7 @@ fn babar() {} // @has - '//pre[@class="rust"]//a/@href' '/struct.String.html' // @has - '//pre[@class="rust"]//a/@href' '/primitive.u32.html' // @has - '//pre[@class="rust"]//a/@href' '/primitive.str.html' -// @count - '//pre[@class="rust"]//a[@href="#23"]' 5 +// @count - '//pre[@class="rust"]//a[@href="#23"]' 2 // @has - '//pre[@class="rust"]//a[@href="../../source_code/struct.SourceCode.html"]' \ // 'source_code::SourceCode' pub fn foo(a: u32, b: &str, c: String, d: Foo, e: bar::Bar, f: source_code::SourceCode) { @@ -39,7 +39,6 @@ pub fn foo(a: u32, b: &str, c: String, d: Foo, e: bar::Bar, f: source_code::Sour let y: Foo = Foo; let z: Bar = bar::Bar { field: Foo }; babar(); - // @has - '//pre[@class="rust"]//a[@href="#26"]' 'hello' y.hello(); } @@ -50,8 +49,8 @@ pub fn foo2(t: &T, v: &V, b: bool) {} pub trait AnotherTrait {} pub trait WhyNot {} -// @has - '//pre[@class="rust"]//a[@href="#50"]' 'AnotherTrait' -// @has - '//pre[@class="rust"]//a[@href="#51"]' 'WhyNot' +// @has - '//pre[@class="rust"]//a[@href="#49"]' 'AnotherTrait' +// @has - '//pre[@class="rust"]//a[@href="#50"]' 'WhyNot' pub fn foo3(t: &T, v: &V) where T: AnotherTrait, @@ -60,7 +59,6 @@ where pub trait AnotherTrait2 {} -// @has - '//pre[@class="rust"]//a[@href="#61"]' 'AnotherTrait2' pub fn foo4() { let x: Vec<&dyn AnotherTrait2> = Vec::new(); } diff --git a/tests/rustdoc/jump-to-def-doc-links-calls.rs b/tests/rustdoc/jump-to-def-doc-links-calls.rs deleted file mode 100644 index 4101058edbf4f..0000000000000 --- a/tests/rustdoc/jump-to-def-doc-links-calls.rs +++ /dev/null @@ -1,27 +0,0 @@ -//@ compile-flags: -Zunstable-options --generate-link-to-definition - -#![crate_name = "foo"] - -// @has 'src/foo/jump-to-def-doc-links-calls.rs.html' - -// @has - '//a[@href="../../foo/struct.Bar.html"]' 'Bar' -pub struct Bar; - -impl std::default::Default for Bar { - // @has - '//a[@href="#20-22"]' 'Self::new' - fn default() -> Self { - Self::new() - } -} - -// @has - '//a[@href="#8"]' 'Bar' -impl Bar { - // @has - '//a[@href="#24-26"]' 'Self::bar' - pub fn new()-> Self { - Self::bar() - } - - pub fn bar() -> Self { - Self - } -} diff --git a/tests/rustdoc/jump-to-non-local-method.rs b/tests/rustdoc/jump-to-non-local-method.rs deleted file mode 100644 index bc44d9a97088f..0000000000000 --- a/tests/rustdoc/jump-to-non-local-method.rs +++ /dev/null @@ -1,48 +0,0 @@ -//@ compile-flags: -Zunstable-options --generate-link-to-definition - -#![crate_name = "foo"] - -// @has 'src/foo/jump-to-non-local-method.rs.html' - -// @has - '//a[@href="{{channel}}/core/sync/atomic/struct.AtomicIsize.html"]' 'std::sync::atomic::AtomicIsize' -use std::sync::atomic::AtomicIsize; -// @has - '//a[@href="{{channel}}/std/io/trait.Read.html"]' 'std::io::Read' -use std::io::Read; -// @has - '//a[@href="{{channel}}/std/io/index.html"]' 'std::io' -use std::io; -// @has - '//a[@href="{{channel}}/std/process/fn.exit.html"]' 'std::process::exit' -use std::process::exit; -use std::cmp::Ordering; -use std::marker::PhantomData; - -pub fn bar2(readable: T) { - // @has - '//a[@href="{{channel}}/std/io/trait.Read.html#tymethod.read"]' 'read' - let _ = readable.read(&mut []); -} - -pub fn bar() { - // @has - '//a[@href="{{channel}}/core/sync/atomic/struct.AtomicIsize.html#method.new"]' 'AtomicIsize::new' - let _ = AtomicIsize::new(0); - // @has - '//a[@href="#48"]' 'local_private' - local_private(); -} - -pub fn extern_call() { - // @has - '//a[@href="{{channel}}/std/process/fn.exit.html"]' 'exit' - exit(0); -} - -pub fn macro_call() -> Result<(), ()> { - // @has - '//a[@href="{{channel}}/core/macro.try.html"]' 'try!' - try!(Err(())); - Ok(()) -} - -pub fn variant() { - // @has - '//a[@href="{{channel}}/core/cmp/enum.Ordering.html#variant.Less"]' 'Ordering::Less' - let _ = Ordering::Less; - // @has - '//a[@href="{{channel}}/core/marker/struct.PhantomData.html"]' 'PhantomData' - let _: PhantomData:: = PhantomData; -} - -fn local_private() {}