From 8a473ca3469340741a7108d52dd488c799f70fad Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 22 Oct 2021 21:45:06 +0200 Subject: [PATCH 1/6] Recursively document Deref --- src/librustdoc/html/render/context.rs | 9 ++- src/librustdoc/html/render/mod.rs | 82 ++++++++++++++++---- src/librustdoc/passes/collect_trait_impls.rs | 36 ++++++++- 3 files changed, 111 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 0e29cc85f9e..db718fbe673 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: Rc>>, /// 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<'_>, 112); /// 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: Rc::new(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: Rc::new(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 07dea624d7c..b84cd718d4c 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1053,6 +1053,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(); @@ -1072,12 +1085,18 @@ 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, - "

\ + "

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

", + id = id, trait_ = trait_.print(cx), type_ = type_.print(cx), ); @@ -1104,17 +1123,22 @@ fn render_assoc_items( ); } } - 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>, _) = @@ -1166,6 +1190,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(); @@ -1187,16 +1212,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); } } } @@ -1986,7 +2011,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>| { @@ -2060,7 +2087,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_); @@ -2077,7 +2110,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; } @@ -2101,9 +2134,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))), ); @@ -2116,6 +2157,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 91a0cb413eb..d559dd4effe 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; @@ -53,6 +54,29 @@ 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() { + // `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 { @@ -73,6 +97,16 @@ 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() { + 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); + } + } + } } } } From 0c38f31bf23506ad33bbd922ea6095f1010712df Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 22 Oct 2021 22:39:33 +0200 Subject: [PATCH 2/6] Add tests for recursive deref --- src/librustdoc/passes/collect_trait_impls.rs | 11 ++- src/test/rustdoc-ui/recursive-deref-ice.rs | 19 +++++ src/test/rustdoc/deref-recursive-pathbuf.rs | 24 ++++++ src/test/rustdoc/deref-recursive.rs | 40 ++++++++++ src/test/rustdoc/deref-typedef.rs | 4 +- src/test/rustdoc/recursive-deref-sidebar.rs | 2 +- src/test/rustdoc/recursive-deref.rs | 77 +++++++++++++++++++- 7 files changed, 169 insertions(+), 8 deletions(-) create mode 100644 src/test/rustdoc-ui/recursive-deref-ice.rs create mode 100644 src/test/rustdoc/deref-recursive-pathbuf.rs create mode 100644 src/test/rustdoc/deref-recursive.rs diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index d559dd4effe..b306eb98bb5 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -81,8 +81,8 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate { // 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_) { let target = items .iter() @@ -221,8 +221,11 @@ impl BadImplStripper { 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()) + } else if ty.def_id_no_primitives().is_some() { + // We want to keep *ALL* deref implementations in case some of them are used in + // the current crate. + // FIXME: Try to filter the one actually used... + true } 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 00000000000..c44fd27f403 --- /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 00000000000..ac23eced386 --- /dev/null +++ b/src/test/rustdoc/deref-recursive-pathbuf.rs @@ -0,0 +1,24 @@ +// #26207: Show all methods reachable via Deref impls, recursing through multiple dereferencing +// levels and across multiple crates. + +// @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 00000000000..ac43b10ec85 --- /dev/null +++ b/src/test/rustdoc/deref-recursive.rs @@ -0,0 +1,40 @@ +// #26207: Show all methods reachable via Deref impls, recursing through multiple dereferencing +// levels if needed. + +// @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 d42ff384b29..ad7a96c5dad 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 fcb636ade8f..65a7debc253 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 3d17bce4721..18634e1b360 100644 --- a/src/test/rustdoc/recursive-deref.rs +++ b/src/test/rustdoc/recursive-deref.rs @@ -1,7 +1,9 @@ use std::ops::Deref; +// Cyclic deref with the parent (which is not the top parent). pub struct A; pub struct B; +pub struct C; // @has recursive_deref/struct.A.html '//h3[@class="code-header in-band"]' 'impl Deref for A' impl Deref for A { @@ -14,7 +16,80 @@ impl Deref for A { // @has recursive_deref/struct.B.html '//h3[@class="code-header in-band"]' 'impl Deref for B' 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; + +// @has recursive_deref/struct.D.html '//h3[@class="code-header in-band"]' 'impl Deref for D' +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' +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' +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; + +// @has recursive_deref/struct.H.html '//h3[@class="code-header in-band"]' 'impl Deref for H' +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!() From 3398877858b28862854097bd33a4b9bbadcd79af Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 23 Oct 2021 22:18:33 +0200 Subject: [PATCH 3/6] Remove the Rc wrapping of deref_id_map --- src/librustdoc/html/render/context.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index db718fbe673..f7bfd7add29 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -56,7 +56,7 @@ crate struct Context<'tcx> { 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: Rc>>, + 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. @@ -73,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<'_>, 112); +rustc_data_structures::static_assert_size!(Context<'_>, 144); /// Shared mutable state used in [`Context`] and elsewhere. crate struct SharedContext<'tcx> { @@ -516,7 +516,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { dst, render_redirect_pages: false, id_map: RefCell::new(id_map), - deref_id_map: Rc::new(RefCell::new(FxHashMap::default())), + deref_id_map: RefCell::new(FxHashMap::default()), shared: Rc::new(scx), include_sources, }; @@ -540,7 +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: Rc::new(RefCell::new(FxHashMap::default())), + deref_id_map: RefCell::new(FxHashMap::default()), id_map: RefCell::new(IdMap::new()), shared: Rc::clone(&self.shared), include_sources: self.include_sources, From dd68d207a5713850f757b7355e175021062db059 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 23 Oct 2021 22:57:02 +0200 Subject: [PATCH 4/6] Don't display "Methods from Deref<...>" if no method is display (the ones which don't have `self` argument) --- src/librustdoc/html/render/mod.rs | 12 +++++++++--- src/test/rustdoc/recursive-deref.rs | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index b84cd718d4c..ceabc76c2e9 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1075,9 +1075,10 @@ fn render_assoc_items_inner( }; 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\

", @@ -1091,7 +1092,7 @@ fn render_assoc_items_inner( cx.deref_id_map.borrow_mut().insert(def_id, id.clone()); } write!( - w, + tmp_buf, "

\ Methods from {trait_}<Target = {type_}>\ \ @@ -1103,9 +1104,10 @@ fn render_assoc_items_inner( 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, @@ -1122,6 +1124,10 @@ fn render_assoc_items_inner( }, ); } + if !impls_buf.is_empty() { + w.push_buffer(tmp_buf); + w.push_buffer(impls_buf); + } } if !traits.is_empty() { diff --git a/src/test/rustdoc/recursive-deref.rs b/src/test/rustdoc/recursive-deref.rs index 18634e1b360..9833599e123 100644 --- a/src/test/rustdoc/recursive-deref.rs +++ b/src/test/rustdoc/recursive-deref.rs @@ -5,7 +5,12 @@ 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; @@ -15,6 +20,7 @@ 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 = C; @@ -38,7 +44,13 @@ 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' +// @!has '-' '//*[@id="deref-methods-G"]' impl Deref for D { type Target = E; @@ -48,6 +60,7 @@ impl Deref for D { } // @has recursive_deref/struct.E.html '//h3[@class="code-header in-band"]' 'impl Deref for E' +// @!has '-' '//*[@id="deref-methods-G"]' impl Deref for E { type Target = F; @@ -57,6 +70,7 @@ impl Deref for E { } // @has recursive_deref/struct.F.html '//h3[@class="code-header in-band"]' 'impl Deref for F' +// @!has '-' '//*[@id="deref-methods-G"]' impl Deref for F { type Target = G; @@ -78,7 +92,13 @@ impl Deref for G { 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; From f09b67a696aac41bb2a8f5146b4017ffd141e232 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 23 Oct 2021 23:26:29 +0200 Subject: [PATCH 5/6] Fix panic when documenting libproc-macro --- src/librustdoc/passes/collect_trait_impls.rs | 23 +++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index b306eb98bb5..9ccc4e5b89f 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -66,7 +66,7 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate { 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() { + } else if let Some(target_did) = target.def_id_no_primitives() { // `impl Deref for S` if target_did == type_did { // Avoid infinite cycles @@ -82,7 +82,7 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate { for it in &new_items { if let ImplItem(Impl { ref for_, ref trait_, ref items, .. }) = *it.kind { if trait_.as_ref().map(|t| t.def_id()) == cx.tcx.lang_items().deref_trait() - && cleaner.keep_impl(for_) + && cleaner.keep_impl(for_, true) { let target = items .iter() @@ -97,7 +97,7 @@ 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() { + 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 @@ -113,10 +113,10 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate { 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 @@ -215,17 +215,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 ty.def_id_no_primitives().is_some() { - // We want to keep *ALL* deref implementations in case some of them are used in - // the current crate. - // FIXME: Try to filter the one actually used... - true + } else if let Some(did) = ty.def_id_no_primitives() { + is_deref || self.keep_impl_with_def_id(did.into()) } else { false } From 78b604569b4381ef47b7c0897be5c318b8f88618 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 30 Oct 2021 16:30:14 +0200 Subject: [PATCH 6/6] Document tests a bit more --- src/test/rustdoc/deref-recursive-pathbuf.rs | 1 + src/test/rustdoc/deref-recursive.rs | 1 + src/test/rustdoc/recursive-deref.rs | 3 +++ 3 files changed, 5 insertions(+) diff --git a/src/test/rustdoc/deref-recursive-pathbuf.rs b/src/test/rustdoc/deref-recursive-pathbuf.rs index ac23eced386..9ab338ca9b1 100644 --- a/src/test/rustdoc/deref-recursive-pathbuf.rs +++ b/src/test/rustdoc/deref-recursive-pathbuf.rs @@ -1,5 +1,6 @@ // #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' diff --git a/src/test/rustdoc/deref-recursive.rs b/src/test/rustdoc/deref-recursive.rs index ac43b10ec85..c07e048b065 100644 --- a/src/test/rustdoc/deref-recursive.rs +++ b/src/test/rustdoc/deref-recursive.rs @@ -1,5 +1,6 @@ // #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' diff --git a/src/test/rustdoc/recursive-deref.rs b/src/test/rustdoc/recursive-deref.rs index 9833599e123..a7504fbccfb 100644 --- a/src/test/rustdoc/recursive-deref.rs +++ b/src/test/rustdoc/recursive-deref.rs @@ -50,6 +50,7 @@ impl 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; @@ -60,6 +61,7 @@ impl Deref for D { } // @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; @@ -70,6 +72,7 @@ impl Deref for E { } // @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;