diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 1ce6a5c00be74..826e7782db1e9 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -6,7 +6,7 @@ use std::rc::Rc; use std::sync::mpsc::{channel, Receiver}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_hir::def_id::LOCAL_CRATE; +use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_middle::ty::TyCtxt; use rustc_session::Session; use rustc_span::edition::Edition; @@ -54,6 +54,9 @@ crate struct Context<'tcx> { /// real location of an item. This is used to allow external links to /// publicly reused items to redirect to the right location. pub(super) render_redirect_pages: bool, + /// Tracks section IDs for `Deref` targets so they match in both the main + /// body and the sidebar. + pub(super) deref_id_map: RefCell>, /// The map used to ensure all generated 'id=' attributes are unique. pub(super) id_map: RefCell, /// Shared mutable state. @@ -70,7 +73,7 @@ crate struct Context<'tcx> { // `Context` is cloned a lot, so we don't want the size to grow unexpectedly. #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] -rustc_data_structures::static_assert_size!(Context<'_>, 104); +rustc_data_structures::static_assert_size!(Context<'_>, 144); /// Shared mutable state used in [`Context`] and elsewhere. crate struct SharedContext<'tcx> { @@ -513,6 +516,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { dst, render_redirect_pages: false, id_map: RefCell::new(id_map), + deref_id_map: RefCell::new(FxHashMap::default()), shared: Rc::new(scx), include_sources, }; @@ -536,6 +540,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { current: self.current.clone(), dst: self.dst.clone(), render_redirect_pages: self.render_redirect_pages, + deref_id_map: RefCell::new(FxHashMap::default()), id_map: RefCell::new(IdMap::new()), shared: Rc::clone(&self.shared), include_sources: self.include_sources, diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index e2ecf20fe848b..f78129050d7ec 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1054,6 +1054,19 @@ fn render_assoc_items( containing_item: &clean::Item, it: DefId, what: AssocItemRender<'_>, +) { + let mut derefs = FxHashSet::default(); + derefs.insert(it); + render_assoc_items_inner(w, cx, containing_item, it, what, &mut derefs) +} + +fn render_assoc_items_inner( + w: &mut Buffer, + cx: &Context<'_>, + containing_item: &clean::Item, + it: DefId, + what: AssocItemRender<'_>, + derefs: &mut FxHashSet, ) { info!("Documenting associated items of {:?}", containing_item.name); let cache = cx.cache(); @@ -1063,9 +1076,10 @@ fn render_assoc_items( }; let (non_trait, traits): (Vec<_>, _) = v.iter().partition(|i| i.inner_impl().trait_.is_none()); if !non_trait.is_empty() { + let mut tmp_buf = Buffer::empty_from(w); let render_mode = match what { AssocItemRender::All => { - w.write_str( + tmp_buf.write_str( "

\ Implementations\

", @@ -1073,21 +1087,28 @@ fn render_assoc_items( RenderMode::Normal } AssocItemRender::DerefFor { trait_, type_, deref_mut_ } => { + let id = + cx.derive_id(small_url_encode(format!("deref-methods-{:#}", type_.print(cx)))); + if let Some(def_id) = type_.def_id(cx.cache()) { + cx.deref_id_map.borrow_mut().insert(def_id, id.clone()); + } write!( - w, - "

\ + tmp_buf, + "

\ Methods from {trait_}<Target = {type_}>\ - \ + \

", + id = id, trait_ = trait_.print(cx), type_ = type_.print(cx), ); RenderMode::ForDeref { mut_: deref_mut_ } } }; + let mut impls_buf = Buffer::empty_from(w); for i in &non_trait { render_impl( - w, + &mut impls_buf, cx, i, containing_item, @@ -1104,18 +1125,27 @@ fn render_assoc_items( }, ); } + if !impls_buf.is_empty() { + w.push_buffer(tmp_buf); + w.push_buffer(impls_buf); + } } - if let AssocItemRender::DerefFor { .. } = what { - return; - } + if !traits.is_empty() { let deref_impl = traits.iter().find(|t| t.trait_did() == cx.tcx().lang_items().deref_trait()); if let Some(impl_) = deref_impl { let has_deref_mut = traits.iter().any(|t| t.trait_did() == cx.tcx().lang_items().deref_mut_trait()); - render_deref_methods(w, cx, impl_, containing_item, has_deref_mut); + render_deref_methods(w, cx, impl_, containing_item, has_deref_mut, derefs); + } + + // If we were already one level into rendering deref methods, we don't want to render + // anything after recursing into any further deref methods above. + if let AssocItemRender::DerefFor { .. } = what { + return; } + let (synthetic, concrete): (Vec<&&Impl>, Vec<&&Impl>) = traits.iter().partition(|t| t.inner_impl().synthetic); let (blanket_impl, concrete): (Vec<&&Impl>, _) = @@ -1167,6 +1197,7 @@ fn render_deref_methods( impl_: &Impl, container_item: &clean::Item, deref_mut: bool, + derefs: &mut FxHashSet, ) { let cache = cx.cache(); let deref_type = impl_.inner_impl().trait_.as_ref().unwrap(); @@ -1188,16 +1219,16 @@ fn render_deref_methods( if let Some(did) = target.def_id(cache) { if let Some(type_did) = impl_.inner_impl().for_.def_id(cache) { // `impl Deref for S` - if did == type_did { + if did == type_did || !derefs.insert(did) { // Avoid infinite cycles return; } } - render_assoc_items(w, cx, container_item, did, what); + render_assoc_items_inner(w, cx, container_item, did, what, derefs); } else { if let Some(prim) = target.primitive_type() { if let Some(&did) = cache.primitive_locations.get(&prim) { - render_assoc_items(w, cx, container_item, did, what); + render_assoc_items_inner(w, cx, container_item, did, what, derefs); } } } @@ -1987,7 +2018,9 @@ fn sidebar_assoc_items(cx: &Context<'_>, out: &mut Buffer, it: &clean::Item) { if let Some(impl_) = v.iter().find(|i| i.trait_did() == cx.tcx().lang_items().deref_trait()) { - sidebar_deref_methods(cx, out, impl_, v); + let mut derefs = FxHashSet::default(); + derefs.insert(did); + sidebar_deref_methods(cx, out, impl_, v, &mut derefs); } let format_impls = |impls: Vec<&Impl>| { @@ -2061,7 +2094,13 @@ fn sidebar_assoc_items(cx: &Context<'_>, out: &mut Buffer, it: &clean::Item) { } } -fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &[Impl]) { +fn sidebar_deref_methods( + cx: &Context<'_>, + out: &mut Buffer, + impl_: &Impl, + v: &[Impl], + derefs: &mut FxHashSet, +) { let c = cx.cache(); debug!("found Deref: {:?}", impl_); @@ -2078,7 +2117,7 @@ fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &[ if let Some(did) = target.def_id(c) { if let Some(type_did) = impl_.inner_impl().for_.def_id(c) { // `impl Deref for S` - if did == type_did { + if did == type_did || !derefs.insert(did) { // Avoid infinite cycles return; } @@ -2102,9 +2141,17 @@ fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &[ }) .collect::>(); if !ret.is_empty() { + let map; + let id = if let Some(target_def_id) = real_target.def_id(c) { + map = cx.deref_id_map.borrow(); + map.get(&target_def_id).expect("Deref section without derived id") + } else { + "deref-methods" + }; write!( out, - "

Methods from {}<Target={}>

", + "

Methods from {}<Target={}>

", + id, Escape(&format!("{:#}", impl_.inner_impl().trait_.as_ref().unwrap().print(cx))), Escape(&format!("{:#}", real_target.print(cx))), ); @@ -2117,6 +2164,21 @@ fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &[ out.push_str(""); } } + + // Recurse into any further impls that might exist for `target` + if let Some(target_did) = target.def_id_no_primitives() { + if let Some(target_impls) = c.impls.get(&target_did) { + if let Some(target_deref_impl) = target_impls.iter().find(|i| { + i.inner_impl() + .trait_ + .as_ref() + .map(|t| Some(t.def_id()) == cx.tcx().lang_items().deref_trait()) + .unwrap_or(false) + }) { + sidebar_deref_methods(cx, out, target_deref_impl, target_impls, derefs); + } + } + } } } diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 1beb3f2f4abbd..7a4198198fa69 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -3,7 +3,8 @@ use crate::clean::*; use crate::core::DocContext; use crate::fold::DocFolder; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_hir::def_id::DefId; use rustc_middle::ty::DefIdTree; use rustc_span::symbol::sym; @@ -51,12 +52,35 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate { } let mut cleaner = BadImplStripper { prims, items: crate_items }; + let mut type_did_to_deref_target: FxHashMap = FxHashMap::default(); + + // Follow all `Deref` targets of included items and recursively add them as valid + fn add_deref_target( + map: &FxHashMap, + cleaner: &mut BadImplStripper, + type_did: DefId, + ) { + if let Some(target) = map.get(&type_did) { + debug!("add_deref_target: type {:?}, target {:?}", type_did, target); + if let Some(target_prim) = target.primitive_type() { + cleaner.prims.insert(target_prim); + } else if let Some(target_did) = target.def_id_no_primitives() { + // `impl Deref for S` + if target_did == type_did { + // Avoid infinite cycles + return; + } + cleaner.items.insert(target_did.into()); + add_deref_target(map, cleaner, target_did); + } + } + } // scan through included items ahead of time to splice in Deref targets to the "valid" sets for it in &new_items { if let ImplItem(Impl { ref for_, ref trait_, ref items, .. }) = *it.kind { - if cleaner.keep_impl(for_) - && trait_.as_ref().map(|t| t.def_id()) == cx.tcx.lang_items().deref_trait() + if trait_.as_ref().map(|t| t.def_id()) == cx.tcx.lang_items().deref_trait() + && cleaner.keep_impl(for_, true) { let target = items .iter() @@ -71,16 +95,26 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate { } else if let Some(did) = target.def_id(&cx.cache) { cleaner.items.insert(did.into()); } + if let Some(for_did) = for_.def_id_no_primitives() { + if type_did_to_deref_target.insert(for_did, target).is_none() { + // Since only the `DefId` portion of the `Type` instances is known to be same for both the + // `Deref` target type and the impl for type positions, this map of types is keyed by + // `DefId` and for convenience uses a special cleaner that accepts `DefId`s directly. + if cleaner.keep_impl_with_def_id(for_did.into()) { + add_deref_target(&type_did_to_deref_target, &mut cleaner, for_did); + } + } + } } } } new_items.retain(|it| { if let ImplItem(Impl { ref for_, ref trait_, ref blanket_impl, .. }) = *it.kind { - cleaner.keep_impl(for_) - || trait_ - .as_ref() - .map_or(false, |t| cleaner.keep_impl_with_def_id(t.def_id().into())) + cleaner.keep_impl( + for_, + trait_.as_ref().map(|t| t.def_id()) == cx.tcx.lang_items().deref_trait(), + ) || trait_.as_ref().map_or(false, |t| cleaner.keep_impl_with_def_id(t.def_id().into())) || blanket_impl.is_some() } else { true @@ -179,14 +213,14 @@ struct BadImplStripper { } impl BadImplStripper { - fn keep_impl(&self, ty: &Type) -> bool { + fn keep_impl(&self, ty: &Type, is_deref: bool) -> bool { if let Generic(_) = ty { // keep impls made on generics true } else if let Some(prim) = ty.primitive_type() { self.prims.contains(&prim) } else if let Some(did) = ty.def_id_no_primitives() { - self.keep_impl_with_def_id(did.into()) + is_deref || self.keep_impl_with_def_id(did.into()) } else { false } diff --git a/src/test/rustdoc-ui/recursive-deref-ice.rs b/src/test/rustdoc-ui/recursive-deref-ice.rs new file mode 100644 index 0000000000000..c44fd27f40305 --- /dev/null +++ b/src/test/rustdoc-ui/recursive-deref-ice.rs @@ -0,0 +1,19 @@ +// check-pass + +// ICE found in https://github.com/rust-lang/rust/issues/83123 + +pub struct Attribute; + +pub struct Map<'hir> {} +impl<'hir> Map<'hir> { + pub fn attrs(&self) -> &'hir [Attribute] { &[] } +} + +pub struct List(T); + +impl std::ops::Deref for List { + type Target = [T]; + fn deref(&self) -> &[T] { + &[] + } +} diff --git a/src/test/rustdoc/deref-recursive-pathbuf.rs b/src/test/rustdoc/deref-recursive-pathbuf.rs new file mode 100644 index 0000000000000..9ab338ca9b1d1 --- /dev/null +++ b/src/test/rustdoc/deref-recursive-pathbuf.rs @@ -0,0 +1,25 @@ +// #26207: Show all methods reachable via Deref impls, recursing through multiple dereferencing +// levels and across multiple crates. +// For `Deref` on non-foreign types, look at `deref-recursive.rs`. + +// @has 'foo/struct.Foo.html' +// @has '-' '//*[@id="deref-methods-PathBuf"]' 'Methods from Deref' +// @has '-' '//*[@class="impl-items"]//*[@id="method.as_path"]' 'pub fn as_path(&self)' +// @has '-' '//*[@id="deref-methods-Path"]' 'Methods from Deref' +// @has '-' '//*[@class="impl-items"]//*[@id="method.exists"]' 'pub fn exists(&self)' +// @has '-' '//*[@class="sidebar-title"]/a[@href="#deref-methods-PathBuf"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-links"]/a[@href="#method.as_path"]' 'as_path' +// @has '-' '//*[@class="sidebar-title"]/a[@href="#deref-methods-Path"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-links"]/a[@href="#method.exists"]' 'exists' + +#![crate_name = "foo"] + +use std::ops::Deref; +use std::path::PathBuf; + +pub struct Foo(PathBuf); + +impl Deref for Foo { + type Target = PathBuf; + fn deref(&self) -> &PathBuf { &self.0 } +} diff --git a/src/test/rustdoc/deref-recursive.rs b/src/test/rustdoc/deref-recursive.rs new file mode 100644 index 0000000000000..c07e048b0651c --- /dev/null +++ b/src/test/rustdoc/deref-recursive.rs @@ -0,0 +1,41 @@ +// #26207: Show all methods reachable via Deref impls, recursing through multiple dereferencing +// levels if needed. +// For `Deref` on foreign types, look at `deref-recursive-pathbuf.rs`. + +// @has 'foo/struct.Foo.html' +// @has '-' '//*[@id="deref-methods-Bar"]' 'Methods from Deref' +// @has '-' '//*[@class="impl-items"]//*[@id="method.bar"]' 'pub fn bar(&self)' +// @has '-' '//*[@id="deref-methods-Baz"]' 'Methods from Deref' +// @has '-' '//*[@class="impl-items"]//*[@id="method.baz"]' 'pub fn baz(&self)' +// @has '-' '//*[@class="sidebar-title"]/a[@href="#deref-methods-Bar"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-links"]/a[@href="#method.bar"]' 'bar' +// @has '-' '//*[@class="sidebar-title"]/a[@href="#deref-methods-Baz"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-links"]/a[@href="#method.baz"]' 'baz' + +#![crate_name = "foo"] + +use std::ops::Deref; + +pub struct Foo(Bar); +pub struct Bar(Baz); +pub struct Baz; + +impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Bar { &self.0 } +} + +impl Deref for Bar { + type Target = Baz; + fn deref(&self) -> &Baz { &self.0 } +} + +impl Bar { + /// This appears under `Foo` methods + pub fn bar(&self) {} +} + +impl Baz { + /// This should also appear in `Foo` methods when recursing + pub fn baz(&self) {} +} diff --git a/src/test/rustdoc/deref-typedef.rs b/src/test/rustdoc/deref-typedef.rs index d42ff384b29b8..ad7a96c5dad1f 100644 --- a/src/test/rustdoc/deref-typedef.rs +++ b/src/test/rustdoc/deref-typedef.rs @@ -1,12 +1,12 @@ #![crate_name = "foo"] // @has 'foo/struct.Bar.html' -// @has '-' '//*[@id="deref-methods"]' 'Methods from Deref' +// @has '-' '//*[@id="deref-methods-FooJ"]' 'Methods from Deref' // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_a"]' 'pub fn foo_a(&self)' // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_b"]' 'pub fn foo_b(&self)' // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_c"]' 'pub fn foo_c(&self)' // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_j"]' 'pub fn foo_j(&self)' -// @has '-' '//*[@class="sidebar-title"]/a[@href="#deref-methods"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-title"]/a[@href="#deref-methods-FooJ"]' 'Methods from Deref' // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_a"]' 'foo_a' // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_b"]' 'foo_b' // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_c"]' 'foo_c' diff --git a/src/test/rustdoc/recursive-deref-sidebar.rs b/src/test/rustdoc/recursive-deref-sidebar.rs index fcb636ade8f7a..65a7debc2538d 100644 --- a/src/test/rustdoc/recursive-deref-sidebar.rs +++ b/src/test/rustdoc/recursive-deref-sidebar.rs @@ -15,7 +15,7 @@ impl Deref for A { fn deref(&self) -> &B { todo!() } } -// @!has recursive_deref_sidebar/struct.A.html '//div[@class="sidebar-links"]' 'foo_c' +// @has recursive_deref_sidebar/struct.A.html '//div[@class="sidebar-links"]' 'foo_c' impl Deref for B { type Target = C; fn deref(&self) -> &C { todo!() } diff --git a/src/test/rustdoc/recursive-deref.rs b/src/test/rustdoc/recursive-deref.rs index 3d17bce472154..a7504fbccfb50 100644 --- a/src/test/rustdoc/recursive-deref.rs +++ b/src/test/rustdoc/recursive-deref.rs @@ -1,9 +1,16 @@ use std::ops::Deref; +// Cyclic deref with the parent (which is not the top parent). pub struct A; pub struct B; +pub struct C; + +impl C { + pub fn c(&self) {} +} // @has recursive_deref/struct.A.html '//h3[@class="code-header in-band"]' 'impl Deref for A' +// @has '-' '//*[@class="impl-items"]//*[@id="method.c"]' 'pub fn c(&self)' impl Deref for A { type Target = B; @@ -13,8 +20,99 @@ impl Deref for A { } // @has recursive_deref/struct.B.html '//h3[@class="code-header in-band"]' 'impl Deref for B' +// @has '-' '//*[@class="impl-items"]//*[@id="method.c"]' 'pub fn c(&self)' impl Deref for B { - type Target = A; + type Target = C; + + fn deref(&self) -> &Self::Target { + panic!() + } +} + +// @has recursive_deref/struct.C.html '//h3[@class="code-header in-band"]' 'impl Deref for C' +impl Deref for C { + type Target = B; + + fn deref(&self) -> &Self::Target { + panic!() + } +} + +// Cyclic deref with the grand-parent (which is not the top parent). +pub struct D; +pub struct E; +pub struct F; +pub struct G; + +impl G { + // There is no "self" parameter so it shouldn't be listed! + pub fn g() {} +} + +// @has recursive_deref/struct.D.html '//h3[@class="code-header in-band"]' 'impl Deref for D' +// We also check that `G::g` method isn't rendered because there is no `self` argument. +// @!has '-' '//*[@id="deref-methods-G"]' +impl Deref for D { + type Target = E; + + fn deref(&self) -> &Self::Target { + panic!() + } +} + +// @has recursive_deref/struct.E.html '//h3[@class="code-header in-band"]' 'impl Deref for E' +// We also check that `G::g` method isn't rendered because there is no `self` argument. +// @!has '-' '//*[@id="deref-methods-G"]' +impl Deref for E { + type Target = F; + + fn deref(&self) -> &Self::Target { + panic!() + } +} + +// @has recursive_deref/struct.F.html '//h3[@class="code-header in-band"]' 'impl Deref for F' +// We also check that `G::g` method isn't rendered because there is no `self` argument. +// @!has '-' '//*[@id="deref-methods-G"]' +impl Deref for F { + type Target = G; + + fn deref(&self) -> &Self::Target { + panic!() + } +} + +// @has recursive_deref/struct.G.html '//h3[@class="code-header in-band"]' 'impl Deref for G' +impl Deref for G { + type Target = E; + + fn deref(&self) -> &Self::Target { + panic!() + } +} + +// Cyclic deref with top parent. +pub struct H; +pub struct I; + +impl I { + // There is no "self" parameter so it shouldn't be listed! + pub fn i() {} +} + +// @has recursive_deref/struct.H.html '//h3[@class="code-header in-band"]' 'impl Deref for H' +// @!has '-' '//*[@id="deref-methods-I"]' +impl Deref for H { + type Target = I; + + fn deref(&self) -> &Self::Target { + panic!() + } +} + +// @has recursive_deref/struct.I.html '//h3[@class="code-header in-band"]' 'impl Deref for I' +impl Deref for I { + type Target = H; fn deref(&self) -> &Self::Target { panic!()