Skip to content

rustdoc: do not show self-by-value methods from Deref target if not Copy #33396

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 0 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ impl<'a, 'tcx> Clean<Crate> for visit_ast::RustdocVisitor<'a, 'tcx> {
if let Some(t) = cx.tcx_opt() {
cx.deref_trait_did.set(t.lang_items.deref_trait());
cx.renderinfo.borrow_mut().deref_trait_did = cx.deref_trait_did.get();
cx.renderinfo.borrow_mut().copy_trait_did = t.lang_items.copy_trait();
}

let mut externs = Vec::new();
Expand Down
80 changes: 51 additions & 29 deletions src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ pub struct Cache {
seen_mod: bool,
stripped_mod: bool,
deref_trait_did: Option<DefId>,
copy_trait_did: Option<DefId>,

// In rare case where a structure is defined in one module but implemented
// in another, if the implementing module is parsed before defining module,
Expand All @@ -279,6 +280,7 @@ pub struct RenderInfo {
pub external_paths: ::core::ExternalPaths,
pub external_typarams: HashMap<DefId, String>,
pub deref_trait_did: Option<DefId>,
pub copy_trait_did: Option<DefId>,
}

/// Helper struct to render all source code to HTML pages
Expand Down Expand Up @@ -505,6 +507,7 @@ pub fn run(mut krate: clean::Crate,
external_paths,
external_typarams,
deref_trait_did,
copy_trait_did,
} = renderinfo;

let paths = external_paths.into_iter()
Expand All @@ -529,6 +532,7 @@ pub fn run(mut krate: clean::Crate,
orphan_methods: Vec::new(),
traits: mem::replace(&mut krate.external_traits, HashMap::new()),
deref_trait_did: deref_trait_did,
copy_trait_did: copy_trait_did,
typarams: external_typarams,
inlined: inlined,
};
Expand Down Expand Up @@ -2435,7 +2439,7 @@ impl<'a> AssocItemLink<'a> {

enum AssocItemRender<'a> {
All,
DerefFor { trait_: &'a clean::Type, type_: &'a clean::Type },
DerefFor { trait_: &'a clean::Type, type_: &'a clean::Type, target_is_copy: bool },
}

fn render_assoc_items(w: &mut fmt::Formatter,
Expand All @@ -2452,19 +2456,17 @@ fn render_assoc_items(w: &mut fmt::Formatter,
i.impl_.trait_.is_none()
});
if !non_trait.is_empty() {
let render_header = match what {
match what {
AssocItemRender::All => {
write!(w, "<h2 id='methods'>Methods</h2>")?;
true
}
AssocItemRender::DerefFor { trait_, type_ } => {
AssocItemRender::DerefFor { trait_, type_, .. } => {
write!(w, "<h2 id='deref-methods'>Methods from \
{}&lt;Target={}&gt;</h2>", trait_, type_)?;
false
{}&lt;Target={}&gt;</h2>", trait_, type_)?;
}
};
}
for i in &non_trait {
render_impl(w, cx, i, AssocItemLink::Anchor(None), render_header,
render_impl(w, cx, i, AssocItemLink::Anchor(None), &what,
containing_item.stable_since())?;
}
}
Expand All @@ -2486,7 +2488,7 @@ fn render_assoc_items(w: &mut fmt::Formatter,
for i in &manual {
let did = i.trait_did().unwrap();
let assoc_link = AssocItemLink::GotoSource(did, &i.impl_.provided_trait_methods);
render_impl(w, cx, i, assoc_link, true, containing_item.stable_since())?;
render_impl(w, cx, i, assoc_link, &what, containing_item.stable_since())?;
}
if !derived.is_empty() {
write!(w, "<h3 id='derived_implementations'>\
Expand All @@ -2495,7 +2497,7 @@ fn render_assoc_items(w: &mut fmt::Formatter,
for i in &derived {
let did = i.trait_did().unwrap();
let assoc_link = AssocItemLink::GotoSource(did, &i.impl_.provided_trait_methods);
render_impl(w, cx, i, assoc_link, true, containing_item.stable_since())?;
render_impl(w, cx, i, assoc_link, &what, containing_item.stable_since())?;
}
}
}
Expand All @@ -2511,10 +2513,20 @@ fn render_deref_methods(w: &mut fmt::Formatter, cx: &Context, impl_: &Impl,
_ => None,
}
}).next().expect("Expected associated type binding");
let what = AssocItemRender::DerefFor { trait_: deref_type, type_: target };
if let Some(did) = target.def_id() {
// Check if the target type is not Copy, in which case we should not
// render methods that take self by value.
let c = cache();
let target_is_copy = c.impls.get(&did).map(|v| {
v.iter().any(|i| i.impl_.trait_.def_id() == c.copy_trait_did)
}).unwrap_or(false);
let what = AssocItemRender::DerefFor { trait_: deref_type, type_: target,
target_is_copy: target_is_copy };
render_assoc_items(w, cx, container_item, did, what)
} else {
// Primitive types are always Copy.
let what = AssocItemRender::DerefFor { trait_: deref_type, type_: target,
target_is_copy: true };
if let Some(prim) = target.primitive_type() {
if let Some(c) = cache().primitive_locations.get(&prim) {
let did = DefId { krate: *c, index: prim.to_def_index() };
Expand All @@ -2525,12 +2537,12 @@ fn render_deref_methods(w: &mut fmt::Formatter, cx: &Context, impl_: &Impl,
}
}

// Render_header is false when we are rendering a `Deref` impl and true
// otherwise. If render_header is false, we will avoid rendering static
// methods, since they are not accessible for the type implementing `Deref`
// For Deref methods, we will avoid rendering some methods, since they are not
// accessible for the type implementing `Deref`. We also don't render the
// header, it has already been rendered with a special format.
fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLink,
render_header: bool, outer_version: Option<&str>) -> fmt::Result {
if render_header {
what: &AssocItemRender, outer_version: Option<&str>) -> fmt::Result {
if let AssocItemRender::All = *what {
write!(w, "<h3 class='impl'><code>{}</code>", i.impl_)?;
let since = i.stability.as_ref().map(|s| &s.since[..]);
render_stability_since_raw(w, since, outer_version)?;
Expand All @@ -2541,21 +2553,32 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
}

fn doctraititem(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item,
link: AssocItemLink, render_static: bool, is_default_item: bool,
link: AssocItemLink, what: &AssocItemRender, is_default_item: bool,
outer_version: Option<&str>) -> fmt::Result {
let shortty = shortty(item);
let name = item.name.as_ref().unwrap();

let is_static = match item.inner {
clean::MethodItem(ref method) => method.self_ == SelfTy::SelfStatic,
clean::TyMethodItem(ref method) => method.self_ == SelfTy::SelfStatic,
_ => false
// Prevent rendering static and self-by-value methods on Deref trait
// (the latter only if the target isn't Copy)
let prevent_render = if let AssocItemRender::DerefFor { target_is_copy, .. } = *what {
match item.inner {
clean::MethodItem(ref method) => {
(method.self_ == SelfTy::SelfStatic) ||
(method.self_ == SelfTy::SelfValue && !target_is_copy)
}
clean::TyMethodItem(ref method) => {
(method.self_ == SelfTy::SelfStatic) ||
(method.self_ == SelfTy::SelfValue && !target_is_copy)
}
_ => false
}
} else {
false
};

match item.inner {
clean::MethodItem(..) | clean::TyMethodItem(..) => {
// Only render when the method is not static or we allow static methods
if !is_static || render_static {
if !prevent_render {
let id = derive_id(format!("{}.{}", shortty, name));
write!(w, "<h4 id='{}' class='{}'>", id, shortty)?;
write!(w, "<code>")?;
Expand Down Expand Up @@ -2593,7 +2616,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
_ => panic!("can't make docs for trait item with name {:?}", item.name)
}

if !is_default_item && (!is_static || render_static) {
if !is_default_item && !prevent_render {
document(w, cx, item)
} else {
Ok(())
Expand All @@ -2602,14 +2625,14 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi

write!(w, "<div class='impl-items'>")?;
for trait_item in &i.impl_.items {
doctraititem(w, cx, trait_item, link, render_header, false, outer_version)?;
doctraititem(w, cx, trait_item, link, &what, false, outer_version)?;
}

fn render_default_items(w: &mut fmt::Formatter,
cx: &Context,
t: &clean::Trait,
i: &clean::Impl,
render_static: bool,
what: &AssocItemRender,
outer_version: Option<&str>) -> fmt::Result {
for trait_item in &t.items {
let n = trait_item.name.clone();
Expand All @@ -2619,8 +2642,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
let did = i.trait_.as_ref().unwrap().def_id().unwrap();
let assoc_link = AssocItemLink::GotoSource(did, &i.provided_trait_methods);

doctraititem(w, cx, trait_item, assoc_link, render_static, true,
outer_version)?;
doctraititem(w, cx, trait_item, assoc_link, &what, true, outer_version)?;
}
Ok(())
}
Expand All @@ -2629,7 +2651,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
// default items which weren't overridden in the implementation block.
if let Some(did) = i.trait_did() {
if let Some(t) = cache().traits.get(&did) {
render_default_items(w, cx, t, &i.impl_, render_header, outer_version)?;
render_default_items(w, cx, t, &i.impl_, &what, outer_version)?;
}
}
write!(w, "</div>")?;
Expand Down
16 changes: 16 additions & 0 deletions src/test/auxiliary/issue-19190-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,25 @@ pub struct Baz;
impl Baz {
pub fn baz(&self) {}
pub fn static_baz() {}
pub fn value_baz(self) {}
}

impl Deref for Bar {
type Target = Baz;
fn deref(&self) -> &Baz { loop {} }
}

pub struct CopyBar;
#[derive(Clone, Copy)]
pub struct CopyBaz;

impl CopyBaz {
pub fn baz(&self) {}
pub fn static_baz() {}
pub fn value_baz(self) {}
}

impl Deref for CopyBar {
type Target = CopyBaz;
fn deref(&self) -> &CopyBaz { loop {} }
}
9 changes: 8 additions & 1 deletion src/test/rustdoc/issue-19190-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,22 @@ pub use issue_19190_3::Foo;
// @has issue_19190_3/struct.Bar.html
// @has - '//*[@id="method.baz"]' 'fn baz(&self)'
// @!has - '//*[@id="method.static_baz"]' 'fn static_baz()'
// @!has - '//*[@id="method.value_baz"]' 'fn value_baz()'
pub use issue_19190_3::Bar;

// @has issue_19190_3/struct.Bar.html
// @has - '//*[@id="method.baz"]' 'fn baz(&self)'
// @!has - '//*[@id="method.static_baz"]' 'fn static_baz()'
// @has - '//*[@id="method.value_baz"]' 'fn value_baz()'
pub use issue_19190_3::CopyBar;

// @has issue_19190_3/struct.MyBar.html
// @has - '//*[@id="method.baz"]' 'fn baz(&self)'
// @!has - '//*[@id="method.static_baz"]' 'fn static_baz()'
// @!has - '//*[@id="method.value_baz"]' 'fn value_baz()'
pub struct MyBar;

impl Deref for MyBar {
type Target = Baz;
fn deref(&self) -> &Baz { loop {} }
}

2 changes: 2 additions & 0 deletions src/test/rustdoc/issue-19190.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub struct Bar;
impl Foo {
pub fn foo(&self) {}
pub fn static_foo() {}
pub fn value_foo(self) {}
}

impl Deref for Bar {
Expand All @@ -26,3 +27,4 @@ impl Deref for Bar {
// @has issue_19190/struct.Bar.html
// @has - '//*[@id="method.foo"]' 'fn foo(&self)'
// @!has - '//*[@id="method.static_foo"]' 'fn static_foo()'
// @!has - '//*[@id="method.value_foo"]' 'fn value_foo()'