Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Recursively document methods via Deref traits #80653

Merged
merged 5 commits into from
Jan 8, 2021
Merged

Recursively document methods via Deref traits #80653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
61c8aae
Extract `sidebar_deref_methods` function
jryans Dec 29, 2020
fd0ad03
Recursively document methods via `Deref` traits
jryans Dec 31, 2020
06ce97c
Use target in `Deref` method section IDs
jryans Dec 31, 2020
8eaf68f
Preserve non-local recursive `Deref` impls
jryans Jan 3, 2021
ea94607
Combine several `push_str` calls
jryans Jan 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/librustdoc/html/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,6 @@ fn init_id_map() -> FxHashMap<String, usize> {
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
}

Expand Down
206 changes: 123 additions & 83 deletions src/librustdoc/html/render/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<RefCell<IdMap>>,
/// Tracks section IDs for `Deref` targets so they match in both the main
/// body and the sidebar.
deref_id_map: Rc<RefCell<FxHashMap<DefId, String>>>,
crate shared: Arc<SharedContext<'tcx>>,
all: Rc<RefCell<AllTypes>>,
/// Storage for the errors produced while generating documentation so they
Expand Down Expand Up @@ -372,7 +375,6 @@ crate fn initial_ids() -> Vec<String> {
"implementors-list",
"synthetic-implementors-list",
"methods",
"deref-methods",
"implementations",
]
.iter()
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -3517,14 +3520,18 @@ fn render_assoc_items(
RenderMode::Normal
}
AssocItemRender::DerefFor { trait_, type_, deref_mut_ } => {
let id =
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
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,
"<h2 id=\"deref-methods\" class=\"small-section-header\">\
Methods from {}&lt;Target = {}&gt;\
<a href=\"#deref-methods\" class=\"anchor\"></a>\
"<h2 id=\"{id}\" class=\"small-section-header\">\
Methods from {trait_}&lt;Target = {type_}&gt;\
<a href=\"#{id}\" class=\"anchor\"></a>\
</h2>",
trait_.print(),
type_.print()
id = id,
trait_ = trait_.print(),
type_ = type_.print(),
);
RenderMode::ForDeref { mut_: deref_mut_ }
}
Expand All @@ -3548,9 +3555,6 @@ fn render_assoc_items(
);
}
}
if let AssocItemRender::DerefFor { .. } = what {
return;
}
jyn514 marked this conversation as resolved.
Show resolved Hide resolved
if !traits.is_empty() {
let deref_impl =
traits.iter().find(|t| t.inner_impl().trait_.def_id() == cache.deref_trait_did);
Expand All @@ -3560,6 +3564,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;
}
Comment on lines +3569 to +3571
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The impls were already documented by render_deref_methods and we don't want to duplicate them, is that the idea? The condition for running render_deref is different from what you check here though: this checks DerefFor and render_derf checks deref_impl.is_some(). Is there any time those could be different? If not, could you add an assert?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two concepts at play here simultaneously which is a bit confusing, so I'll try to explain and then we can work out whatever comments or assertions might be helpful.

Let's consider a two level Deref case:

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 }
}

The goal of if let Some(impl_) = deref_impl when calling render_deref_methods is to descend to the methods of the Deref target: e.g. from Foo to Bar and from Bar to Baz.

The goal of if let AssocItemRender::DerefFor { .. } = what is to restrict what we render when we have already descended at least one level: e.g. when we're at Bar and Baz, we only want to show their methods and nothing else.

Does that help at all? 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that helps me understand why these are different, thanks :) I still have my original question, although maybe I didn't word it very well. This return ignores synthetic impls, which is correct (the Target could implement Sized but not the original, and vice-versa), but it also ignores blanket and concrete impls. Where are those handled? Is it in render_deref_items?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, this change is preserving the previous behaviour: trait, synthetic, and blanket impls are all skipped for Deref targets (e.g. Bar and Baz in the above example). The methods in the Deref target that we want to include are rendered by the top bit of render_assoc_items.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks - I missed that this is only skipping trait impls, not inherent impls. I'm not sure that behavior is correct, but it doesn't need to be fixed here.


let (synthetic, concrete): (Vec<&&Impl>, Vec<&&Impl>) =
traits.iter().partition(|t| t.inner_impl().synthetic);
let (blanket_impl, concrete): (Vec<&&Impl>, _) =
Expand Down Expand Up @@ -3631,6 +3641,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<Target = S> for S`
if did == type_did {
jryans marked this conversation as resolved.
Show resolved Hide resolved
// Avoid infinite cycles
return;
}
}
render_assoc_items(w, cx, container_item, did, what, cache);
} else {
if let Some(prim) = target.primitive_type() {
Expand Down Expand Up @@ -4165,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),
_ => (),
}

Expand Down Expand Up @@ -4273,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) {
Expand Down Expand Up @@ -4303,58 +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)
{
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("<a class=\"sidebar-title\" href=\"#deref-methods\">");
out.push_str(&format!(
"Methods from {}&lt;Target={}&gt;",
Escape(&format!(
"{:#}",
impl_.inner_impl().trait_.as_ref().unwrap().print()
)),
Escape(&format!("{:#}", real_target.print()))
));
out.push_str("</a>");
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::<Vec<_>>();
// We want links' order to be reproducible so we don't use unstable sort.
ret.sort();
if !ret.is_empty() {
out.push_str(&format!(
"<div class=\"sidebar-links\">{}</div>",
ret.join("")
));
}
}
}
out.push_str(&sidebar_deref_methods(cx, impl_, v));
}
let format_impls = |impls: Vec<&Impl>| {
let mut links = FxHashSet::default();
Expand Down Expand Up @@ -4422,7 +4388,81 @@ fn sidebar_assoc_items(it: &clean::Item) -> String {
out
}

fn sidebar_struct(buf: &mut Buffer, it: &clean::Item, s: &clean::Struct) {
fn sidebar_deref_methods(cx: &Context<'_>, impl_: &Impl, v: &Vec<Impl>) -> 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);
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::<Vec<_>>();
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!(
"<a class=\"sidebar-title\" href=\"#{}\">Methods from {}&lt;Target={}&gt;</a>",
id,
Escape(&format!("{:#}", impl_.inner_impl().trait_.as_ref().unwrap().print())),
Escape(&format!("{:#}", real_target.print())),
));
// We want links' order to be reproducible so we don't use unstable sort.
ret.sort();
out.push_str(&format!("<div class=\"sidebar-links\">{}</div>", 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<Target = S> for S`
if target_did == type_did {
// Avoid infinite cycles
return out;
}
}
out.push_str(&sidebar_deref_methods(cx, target_deref_impl, target_impls));
}
}
}
}

out
}

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);

Expand All @@ -4436,7 +4476,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, "<div class=\"block items\">{}</div>", sidebar);
Expand Down Expand Up @@ -4467,7 +4507,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
Expand Down Expand Up @@ -4567,7 +4607,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("<a class=\"sidebar-title\" href=\"#implementors\">Implementors</a>");
if t.is_auto {
Expand All @@ -4580,16 +4620,16 @@ fn sidebar_trait(buf: &mut Buffer, it: &clean::Item, t: &clean::Trait) {
write!(buf, "<div class=\"block items\">{}</div>", 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, "<div class=\"block items\">{}</div>", 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, "<div class=\"block items\">{}</div>", sidebar);
Expand All @@ -4611,7 +4651,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);

Expand All @@ -4623,14 +4663,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, "<div class=\"block items\">{}</div>", 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
Expand All @@ -4650,7 +4690,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, "<div class=\"block items\">{}</div>", sidebar);
Expand Down Expand Up @@ -4739,8 +4779,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, "<div class=\"block items\">{}</div>", sidebar);
}
Expand Down
Loading