From 61c8aae0a9222b7d2481704094f85a0d8f6b333e Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 29 Dec 2020 07:58:32 +0000 Subject: [PATCH 1/5] Extract `sidebar_deref_methods` function --- src/librustdoc/html/render/mod.rs | 104 +++++++++++++++--------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index f7eb136a754e2..8d304b45590cf 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -4303,58 +4303,7 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { .filter(|i| i.inner_impl().trait_.is_some()) .find(|i| i.inner_impl().trait_.def_id() == c.deref_trait_did) { - debug!("found Deref: {:?}", impl_); - if let Some((target, real_target)) = - impl_.inner_impl().items.iter().find_map(|item| match *item.kind { - clean::TypedefItem(ref t, true) => Some(match *t { - clean::Typedef { item_type: Some(ref type_), .. } => (type_, &t.type_), - _ => (&t.type_, &t.type_), - }), - _ => None, - }) - { - debug!("found target, real_target: {:?} {:?}", target, real_target); - let deref_mut = v - .iter() - .filter(|i| i.inner_impl().trait_.is_some()) - .any(|i| i.inner_impl().trait_.def_id() == c.deref_mut_trait_did); - let inner_impl = target - .def_id() - .or_else(|| { - target - .primitive_type() - .and_then(|prim| c.primitive_locations.get(&prim).cloned()) - }) - .and_then(|did| c.impls.get(&did)); - if let Some(impls) = inner_impl { - debug!("found inner_impl: {:?}", impls); - out.push_str(""); - out.push_str(&format!( - "Methods from {}<Target={}>", - Escape(&format!( - "{:#}", - impl_.inner_impl().trait_.as_ref().unwrap().print() - )), - Escape(&format!("{:#}", real_target.print())) - )); - out.push_str(""); - let mut ret = impls - .iter() - .filter(|i| i.inner_impl().trait_.is_none()) - .flat_map(|i| { - get_methods(i.inner_impl(), true, &mut used_links, deref_mut) - }) - .collect::>(); - // We want links' order to be reproducible so we don't use unstable sort. - ret.sort(); - if !ret.is_empty() { - out.push_str(&format!( - "
{}
", - ret.join("") - )); - } - } - } + out.push_str(&sidebar_deref_methods(impl_, v)); } let format_impls = |impls: Vec<&Impl>| { let mut links = FxHashSet::default(); @@ -4422,6 +4371,57 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { out } +fn sidebar_deref_methods(impl_: &Impl, v: &Vec) -> String { + let mut out = String::new(); + let c = cache(); + + debug!("found Deref: {:?}", impl_); + if let Some((target, real_target)) = + impl_.inner_impl().items.iter().find_map(|item| match *item.kind { + clean::TypedefItem(ref t, true) => Some(match *t { + clean::Typedef { item_type: Some(ref type_), .. } => (type_, &t.type_), + _ => (&t.type_, &t.type_), + }), + _ => None, + }) + { + debug!("found target, real_target: {:?} {:?}", target, real_target); + let deref_mut = v + .iter() + .filter(|i| i.inner_impl().trait_.is_some()) + .any(|i| i.inner_impl().trait_.def_id() == c.deref_mut_trait_did); + let inner_impl = target + .def_id() + .or_else(|| { + target.primitive_type().and_then(|prim| c.primitive_locations.get(&prim).cloned()) + }) + .and_then(|did| c.impls.get(&did)); + if let Some(impls) = inner_impl { + debug!("found inner_impl: {:?}", impls); + out.push_str(""); + out.push_str(&format!( + "Methods from {}<Target={}>", + Escape(&format!("{:#}", impl_.inner_impl().trait_.as_ref().unwrap().print())), + Escape(&format!("{:#}", real_target.print())) + )); + out.push_str(""); + let mut used_links = FxHashSet::default(); + let mut ret = impls + .iter() + .filter(|i| i.inner_impl().trait_.is_none()) + .flat_map(|i| get_methods(i.inner_impl(), true, &mut used_links, deref_mut)) + .collect::>(); + // We want links' order to be reproducible so we don't use unstable sort. + ret.sort(); + if !ret.is_empty() { + out.push_str(&format!("
{}
", ret.join(""))); + } + } + } + + out +} + fn sidebar_struct(buf: &mut Buffer, it: &clean::Item, s: &clean::Struct) { let mut sidebar = String::new(); let fields = get_struct_fields_name(&s.fields); From fd0ad03902402d5bc23f9d04307388f3d6235961 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 31 Dec 2020 02:23:19 +0000 Subject: [PATCH 2/5] Recursively document methods via `Deref` traits --- src/librustdoc/html/render/mod.rs | 36 ++++++++++++++++-- src/test/rustdoc-ui/deref-recursive-cycle.rs | 17 +++++++++ src/test/rustdoc/deref-recursive.rs | 40 ++++++++++++++++++++ 3 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 src/test/rustdoc-ui/deref-recursive-cycle.rs create mode 100644 src/test/rustdoc/deref-recursive.rs diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 8d304b45590cf..28e638a58a8a2 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -3548,9 +3548,6 @@ fn render_assoc_items( ); } } - if let AssocItemRender::DerefFor { .. } = what { - return; - } if !traits.is_empty() { let deref_impl = traits.iter().find(|t| t.inner_impl().trait_.def_id() == cache.deref_trait_did); @@ -3560,6 +3557,12 @@ fn render_assoc_items( render_deref_methods(w, cx, impl_, containing_item, has_deref_mut, cache); } + // 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>, _) = @@ -3631,6 +3634,13 @@ fn render_deref_methods( let what = AssocItemRender::DerefFor { trait_: deref_type, type_: real_target, deref_mut_: deref_mut }; if let Some(did) = target.def_id() { + if let Some(type_did) = impl_.inner_impl().for_.def_id() { + // `impl Deref for S` + if did == type_did { + // Avoid infinite cycles + return; + } + } render_assoc_items(w, cx, container_item, did, what, cache); } else { if let Some(prim) = target.primitive_type() { @@ -4417,6 +4427,26 @@ fn sidebar_deref_methods(impl_: &Impl, v: &Vec) -> String { out.push_str(&format!("
{}
", ret.join(""))); } } + + // Recurse into any further impls that might exist for `target` + if let Some(target_did) = target.def_id() { + if let Some(target_impls) = c.impls.get(&target_did) { + if let Some(target_deref_impl) = target_impls + .iter() + .filter(|i| i.inner_impl().trait_.is_some()) + .find(|i| i.inner_impl().trait_.def_id() == c.deref_trait_did) + { + if let Some(type_did) = impl_.inner_impl().for_.def_id() { + // `impl Deref for S` + if target_did == type_did { + // Avoid infinite cycles + return out; + } + } + out.push_str(&sidebar_deref_methods(target_deref_impl, target_impls)); + } + } + } } out diff --git a/src/test/rustdoc-ui/deref-recursive-cycle.rs b/src/test/rustdoc-ui/deref-recursive-cycle.rs new file mode 100644 index 0000000000000..4cb518cbbbd5c --- /dev/null +++ b/src/test/rustdoc-ui/deref-recursive-cycle.rs @@ -0,0 +1,17 @@ +// check-pass +// #26207: Ensure `Deref` cycles are properly handled without errors. + +#[derive(Copy, Clone)] +struct S; + +impl std::ops::Deref for S { + type Target = S; + + fn deref(&self) -> &S { + self + } +} + +fn main() { + let s: S = *******S; +} diff --git a/src/test/rustdoc/deref-recursive.rs b/src/test/rustdoc/deref-recursive.rs new file mode 100644 index 0000000000000..0d1e7fb1932c5 --- /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"]' 'Methods from Deref' +// @has '-' '//*[@class="impl-items"]//*[@id="method.bar"]' 'pub fn bar(&self)' +// @has '-' '//*[@id="deref-methods"]' 'Methods from Deref' +// @has '-' '//*[@class="impl-items"]//*[@id="method.baz"]' 'pub fn baz(&self)' +// @has '-' '//*[@class="sidebar-title"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-links"]/a[@href="#method.bar"]' 'bar' +// @has '-' '//*[@class="sidebar-title"]' '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) {} +} From 06ce97c3c938c18ed392e72e535931443c1455c0 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 31 Dec 2020 02:45:15 +0000 Subject: [PATCH 3/5] Use target in `Deref` method section IDs There can now be multiple `Deref` method sections, so this adds the target type to the section ID to ensure they are unique. --- src/librustdoc/html/markdown.rs | 1 - src/librustdoc/html/render/mod.rs | 91 ++++++++++++++++------------- src/test/rustdoc/deref-recursive.rs | 10 ++-- src/test/rustdoc/deref-typedef.rs | 6 +- 4 files changed, 61 insertions(+), 47 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 336249e3afb5f..33639055b59ef 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1322,7 +1322,6 @@ fn init_id_map() -> FxHashMap { map.insert("trait-implementations".to_owned(), 1); map.insert("synthetic-implementations".to_owned(), 1); map.insert("blanket-implementations".to_owned(), 1); - map.insert("deref-methods".to_owned(), 1); map } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 28e638a58a8a2..754c954c3a72c 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -115,6 +115,9 @@ crate struct Context<'tcx> { crate render_redirect_pages: bool, /// The map used to ensure all generated 'id=' attributes are unique. id_map: Rc>, + /// Tracks section IDs for `Deref` targets so they match in both the main + /// body and the sidebar. + deref_id_map: Rc>>, crate shared: Arc>, all: Rc>, /// Storage for the errors produced while generating documentation so they @@ -372,7 +375,6 @@ crate fn initial_ids() -> Vec { "implementors-list", "synthetic-implementors-list", "methods", - "deref-methods", "implementations", ] .iter() @@ -506,6 +508,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { dst, render_redirect_pages: false, id_map: Rc::new(RefCell::new(id_map)), + deref_id_map: Rc::new(RefCell::new(FxHashMap::default())), shared: Arc::new(scx), all: Rc::new(RefCell::new(AllTypes::new())), errors: Rc::new(receiver), @@ -3517,14 +3520,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.deref_id_map.borrow_mut().insert(type_.def_id().unwrap(), id.clone()); write!( w, - "

\ - Methods from {}<Target = {}>\ - \ + "

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

", - trait_.print(), - type_.print() + id = id, + trait_ = trait_.print(), + type_ = type_.print(), ); RenderMode::ForDeref { mut_: deref_mut_ } } @@ -4175,14 +4182,14 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer, cache: ); } match *it.kind { - clean::StructItem(ref s) => sidebar_struct(buffer, it, s), - clean::TraitItem(ref t) => sidebar_trait(buffer, it, t), - clean::PrimitiveItem(_) => sidebar_primitive(buffer, it), - clean::UnionItem(ref u) => sidebar_union(buffer, it, u), - clean::EnumItem(ref e) => sidebar_enum(buffer, it, e), - clean::TypedefItem(_, _) => sidebar_typedef(buffer, it), + clean::StructItem(ref s) => sidebar_struct(cx, buffer, it, s), + clean::TraitItem(ref t) => sidebar_trait(cx, buffer, it, t), + clean::PrimitiveItem(_) => sidebar_primitive(cx, buffer, it), + clean::UnionItem(ref u) => sidebar_union(cx, buffer, it, u), + clean::EnumItem(ref e) => sidebar_enum(cx, buffer, it, e), + clean::TypedefItem(_, _) => sidebar_typedef(cx, buffer, it), clean::ModuleItem(ref m) => sidebar_module(buffer, &m.items), - clean::ForeignTypeItem => sidebar_foreign_type(buffer, it), + clean::ForeignTypeItem => sidebar_foreign_type(cx, buffer, it), _ => (), } @@ -4283,7 +4290,7 @@ fn small_url_encode(s: &str) -> String { .replace("\"", "%22") } -fn sidebar_assoc_items(it: &clean::Item) -> String { +fn sidebar_assoc_items(cx: &Context<'_>, it: &clean::Item) -> String { let mut out = String::new(); let c = cache(); if let Some(v) = c.impls.get(&it.def_id) { @@ -4313,7 +4320,7 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { .filter(|i| i.inner_impl().trait_.is_some()) .find(|i| i.inner_impl().trait_.def_id() == c.deref_trait_did) { - out.push_str(&sidebar_deref_methods(impl_, v)); + out.push_str(&sidebar_deref_methods(cx, impl_, v)); } let format_impls = |impls: Vec<&Impl>| { let mut links = FxHashSet::default(); @@ -4381,7 +4388,7 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { out } -fn sidebar_deref_methods(impl_: &Impl, v: &Vec) -> String { +fn sidebar_deref_methods(cx: &Context<'_>, impl_: &Impl, v: &Vec) -> String { let mut out = String::new(); let c = cache(); @@ -4408,22 +4415,26 @@ fn sidebar_deref_methods(impl_: &Impl, v: &Vec) -> String { .and_then(|did| c.impls.get(&did)); if let Some(impls) = inner_impl { debug!("found inner_impl: {:?}", impls); - out.push_str(""); - out.push_str(&format!( - "Methods from {}<Target={}>", - Escape(&format!("{:#}", impl_.inner_impl().trait_.as_ref().unwrap().print())), - Escape(&format!("{:#}", real_target.print())) - )); - out.push_str(""); let mut used_links = FxHashSet::default(); let mut ret = impls .iter() .filter(|i| i.inner_impl().trait_.is_none()) .flat_map(|i| get_methods(i.inner_impl(), true, &mut used_links, deref_mut)) .collect::>(); - // We want links' order to be reproducible so we don't use unstable sort. - ret.sort(); if !ret.is_empty() { + let deref_id_map = cx.deref_id_map.borrow(); + let id = deref_id_map + .get(&real_target.def_id().unwrap()) + .expect("Deref section without derived id"); + out.push_str(&format!("", id)); + out.push_str(&format!( + "Methods from {}<Target={}>", + Escape(&format!("{:#}", impl_.inner_impl().trait_.as_ref().unwrap().print())), + Escape(&format!("{:#}", real_target.print())) + )); + out.push_str(""); + // We want links' order to be reproducible so we don't use unstable sort. + ret.sort(); out.push_str(&format!("
{}
", ret.join(""))); } } @@ -4443,7 +4454,7 @@ fn sidebar_deref_methods(impl_: &Impl, v: &Vec) -> String { return out; } } - out.push_str(&sidebar_deref_methods(target_deref_impl, target_impls)); + out.push_str(&sidebar_deref_methods(cx, target_deref_impl, target_impls)); } } } @@ -4452,7 +4463,7 @@ fn sidebar_deref_methods(impl_: &Impl, v: &Vec) -> String { out } -fn sidebar_struct(buf: &mut Buffer, it: &clean::Item, s: &clean::Struct) { +fn sidebar_struct(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, s: &clean::Struct) { let mut sidebar = String::new(); let fields = get_struct_fields_name(&s.fields); @@ -4466,7 +4477,7 @@ fn sidebar_struct(buf: &mut Buffer, it: &clean::Item, s: &clean::Struct) { } } - sidebar.push_str(&sidebar_assoc_items(it)); + sidebar.push_str(&sidebar_assoc_items(cx, it)); if !sidebar.is_empty() { write!(buf, "
{}
", sidebar); @@ -4497,7 +4508,7 @@ fn is_negative_impl(i: &clean::Impl) -> bool { i.polarity == Some(clean::ImplPolarity::Negative) } -fn sidebar_trait(buf: &mut Buffer, it: &clean::Item, t: &clean::Trait) { +fn sidebar_trait(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, t: &clean::Trait) { let mut sidebar = String::new(); let mut types = t @@ -4597,7 +4608,7 @@ fn sidebar_trait(buf: &mut Buffer, it: &clean::Item, t: &clean::Trait) { } } - sidebar.push_str(&sidebar_assoc_items(it)); + sidebar.push_str(&sidebar_assoc_items(cx, it)); sidebar.push_str("Implementors"); if t.is_auto { @@ -4610,16 +4621,16 @@ fn sidebar_trait(buf: &mut Buffer, it: &clean::Item, t: &clean::Trait) { write!(buf, "
{}
", sidebar) } -fn sidebar_primitive(buf: &mut Buffer, it: &clean::Item) { - let sidebar = sidebar_assoc_items(it); +fn sidebar_primitive(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item) { + let sidebar = sidebar_assoc_items(cx, it); if !sidebar.is_empty() { write!(buf, "
{}
", sidebar); } } -fn sidebar_typedef(buf: &mut Buffer, it: &clean::Item) { - let sidebar = sidebar_assoc_items(it); +fn sidebar_typedef(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item) { + let sidebar = sidebar_assoc_items(cx, it); if !sidebar.is_empty() { write!(buf, "
{}
", sidebar); @@ -4641,7 +4652,7 @@ fn get_struct_fields_name(fields: &[clean::Item]) -> String { fields.join("") } -fn sidebar_union(buf: &mut Buffer, it: &clean::Item, u: &clean::Union) { +fn sidebar_union(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, u: &clean::Union) { let mut sidebar = String::new(); let fields = get_struct_fields_name(&u.fields); @@ -4653,14 +4664,14 @@ fn sidebar_union(buf: &mut Buffer, it: &clean::Item, u: &clean::Union) { )); } - sidebar.push_str(&sidebar_assoc_items(it)); + sidebar.push_str(&sidebar_assoc_items(cx, it)); if !sidebar.is_empty() { write!(buf, "
{}
", sidebar); } } -fn sidebar_enum(buf: &mut Buffer, it: &clean::Item, e: &clean::Enum) { +fn sidebar_enum(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, e: &clean::Enum) { let mut sidebar = String::new(); let mut variants = e @@ -4680,7 +4691,7 @@ fn sidebar_enum(buf: &mut Buffer, it: &clean::Item, e: &clean::Enum) { )); } - sidebar.push_str(&sidebar_assoc_items(it)); + sidebar.push_str(&sidebar_assoc_items(cx, it)); if !sidebar.is_empty() { write!(buf, "
{}
", sidebar); @@ -4769,8 +4780,8 @@ fn sidebar_module(buf: &mut Buffer, items: &[clean::Item]) { } } -fn sidebar_foreign_type(buf: &mut Buffer, it: &clean::Item) { - let sidebar = sidebar_assoc_items(it); +fn sidebar_foreign_type(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item) { + let sidebar = sidebar_assoc_items(cx, it); if !sidebar.is_empty() { write!(buf, "
{}
", sidebar); } diff --git a/src/test/rustdoc/deref-recursive.rs b/src/test/rustdoc/deref-recursive.rs index 0d1e7fb1932c5..5aef87c38cd27 100644 --- a/src/test/rustdoc/deref-recursive.rs +++ b/src/test/rustdoc/deref-recursive.rs @@ -1,14 +1,16 @@ +// ignore-tidy-linelength + // #26207: Show all methods reachable via Deref impls, recursing through multiple dereferencing // levels if needed. // @has 'foo/struct.Foo.html' -// @has '-' '//*[@id="deref-methods"]' 'Methods from Deref' +// @has '-' '//*[@id="deref-methods-Bar"]' 'Methods from Deref' // @has '-' '//*[@class="impl-items"]//*[@id="method.bar"]' 'pub fn bar(&self)' -// @has '-' '//*[@id="deref-methods"]' 'Methods from Deref' +// @has '-' '//*[@id="deref-methods-Baz"]' 'Methods from Deref' // @has '-' '//*[@class="impl-items"]//*[@id="method.baz"]' 'pub fn baz(&self)' -// @has '-' '//*[@class="sidebar-title"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-title"][@href="#deref-methods-Bar"]' 'Methods from Deref' // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.bar"]' 'bar' -// @has '-' '//*[@class="sidebar-title"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-title"][@href="#deref-methods-Baz"]' 'Methods from Deref' // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.baz"]' 'baz' #![crate_name = "foo"] diff --git a/src/test/rustdoc/deref-typedef.rs b/src/test/rustdoc/deref-typedef.rs index e2dac2cf417da..589f133b975a5 100644 --- a/src/test/rustdoc/deref-typedef.rs +++ b/src/test/rustdoc/deref-typedef.rs @@ -1,12 +1,14 @@ +// ignore-tidy-linelength + #![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"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-title"][@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' From 8eaf68f92c213358dda59dc3eb648036ab62e18d Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Sun, 3 Jan 2021 13:13:30 +0000 Subject: [PATCH 4/5] Preserve non-local recursive `Deref` impls This adjusts the `rustdoc` trait impl collection path to preserve `Deref` impls from other crates. This adds a first pass to map all of the `Deref` type to target edges and then recursively preserves all targets. --- src/librustdoc/passes/collect_trait_impls.rs | 100 ++++++++++++------- src/test/rustdoc/deref-recursive-pathbuf.rs | 26 +++++ 2 files changed, 90 insertions(+), 36 deletions(-) create mode 100644 src/test/rustdoc/deref-recursive-pathbuf.rs diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 9b0ae09cb3fd5..7b5e9e5905f33 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -3,7 +3,7 @@ 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, LOCAL_CRATE}; use rustc_middle::ty::DefIdTree; use rustc_span::symbol::sym; @@ -54,39 +54,6 @@ crate fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { } } - let mut cleaner = BadImplStripper { prims, items: crate_items }; - - // 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_item(for_) && trait_.def_id() == cx.tcx.lang_items().deref_trait() { - let target = items - .iter() - .find_map(|item| match *item.kind { - TypedefItem(ref t, true) => Some(&t.type_), - _ => None, - }) - .expect("Deref impl without Target type"); - - if let Some(prim) = target.primitive_type() { - cleaner.prims.insert(prim); - } else if let Some(did) = target.def_id() { - cleaner.items.insert(did); - } - } - } - } - - new_items.retain(|it| { - if let ImplItem(Impl { ref for_, ref trait_, ref blanket_impl, .. }) = *it.kind { - cleaner.keep_item(for_) - || trait_.as_ref().map_or(false, |t| cleaner.keep_item(t)) - || blanket_impl.is_some() - } else { - true - } - }); - // `tcx.crates()` doesn't include the local crate, and `tcx.all_trait_implementations` // doesn't work with it anyway, so pull them from the HIR map instead for &trait_did in cx.tcx.all_traits(LOCAL_CRATE).iter() { @@ -123,6 +90,63 @@ crate fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { } } + let mut cleaner = BadImplStripper { prims, items: crate_items }; + + let mut type_did_to_deref_target: FxHashMap = FxHashMap::default(); + // Gather all type to `Deref` target edges. + for it in &new_items { + if let ImplItem(Impl { ref for_, ref trait_, ref items, .. }) = *it.kind { + if trait_.def_id() == cx.tcx.lang_items().deref_trait() { + let target = items.iter().find_map(|item| match *item.kind { + TypedefItem(ref t, true) => Some(&t.type_), + _ => None, + }); + if let (Some(for_did), Some(target)) = (for_.def_id(), target) { + type_did_to_deref_target.insert(for_did, target); + } + } + } + } + // 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); + add_deref_target(map, cleaner, &target_did); + } + } + } + for type_did in type_did_to_deref_target.keys() { + // 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(type_did) { + add_deref_target(&type_did_to_deref_target, &mut cleaner, type_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(t)) + || blanket_impl.is_some() + } else { + true + } + }); + if let Some(ref mut it) = krate.module { if let ModuleItem(Module { ref mut items, .. }) = *it.kind { items.extend(synth.impls); @@ -192,16 +216,20 @@ struct BadImplStripper { } impl BadImplStripper { - fn keep_item(&self, ty: &Type) -> bool { + fn keep_impl(&self, ty: &Type) -> 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() { - self.items.contains(&did) + self.keep_impl_with_def_id(&did) } else { false } } + + fn keep_impl_with_def_id(&self, did: &DefId) -> bool { + self.items.contains(did) + } } diff --git a/src/test/rustdoc/deref-recursive-pathbuf.rs b/src/test/rustdoc/deref-recursive-pathbuf.rs new file mode 100644 index 0000000000000..759e881aab415 --- /dev/null +++ b/src/test/rustdoc/deref-recursive-pathbuf.rs @@ -0,0 +1,26 @@ +// ignore-tidy-linelength + +// #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"][@href="#deref-methods-PathBuf"]' 'Methods from Deref' +// @has '-' '//*[@class="sidebar-links"]/a[@href="#method.as_path"]' 'as_path' +// @has '-' '//*[@class="sidebar-title"][@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 } +} From ea946071ddd5e0cf395eb910253bc1622dcd1f6c Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Sun, 3 Jan 2021 17:32:48 +0000 Subject: [PATCH 5/5] Combine several `push_str` calls --- src/librustdoc/html/render/mod.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 754c954c3a72c..e90e26f20e347 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -4426,13 +4426,12 @@ fn sidebar_deref_methods(cx: &Context<'_>, impl_: &Impl, v: &Vec) -> Strin let id = deref_id_map .get(&real_target.def_id().unwrap()) .expect("Deref section without derived id"); - out.push_str(&format!("", id)); out.push_str(&format!( - "Methods from {}<Target={}>", + "Methods from {}<Target={}>", + id, Escape(&format!("{:#}", impl_.inner_impl().trait_.as_ref().unwrap().print())), - Escape(&format!("{:#}", real_target.print())) + Escape(&format!("{:#}", real_target.print())), )); - out.push_str(""); // We want links' order to be reproducible so we don't use unstable sort. ret.sort(); out.push_str(&format!("
{}
", ret.join("")));