rustdoc: Methods from Deref<Target = T<U>> is imprecise (contains false positives)

[ghost]

use std::ops::Deref;

pub struct Foo<T>(T);
impl Foo<i32> {
    pub fn get_i32(&self) -> i32 { self.0 }
}
impl Foo<u32> {
    pub fn get_u32(&self) -> u32 { self.0 }
}
pub struct Bar(Foo<i32>);
impl Deref for Bar {
    type Target = Foo<i32>;
    fn deref(&self) -> &Foo<i32> {
        &self.0
    }
}

The Methods from Deref for Bar will contain both get_i32 and get_u32, but should only contain get_i32.

Image

[tomjakubowski]

Just wondering (to help prioritize this), is there an example of this bug in the standard library docs?

[ghost]

I don't think there's anything which matches this particular example exactly.

The closest thing that I could find was DerefVec which includes all methods of Vec and doesn't separate them into different bounds like it does for Vec. So someone reading the docs for DerefVec may think that you can dedup on any DerefVec<T>, when in fact you can only do it where T: PartialEq.

http://doc.rust-lang.org/nightly/collections/vec/struct.DerefVec.html

[tomjakubowski]

Ah yeah. Preserving bounds in the deref is related but distinct enough it would be worth creating a new issue for it.

As an aside: when I tried to do the "methods available through Deref" feature myself, a couple months ago, I got stuck enumerating all of these miscellaneous issues and pondering pie in the sky solutions for them. I'm so happy acrichto stepped in to ship something covering the 95% use case :-)

—
Sent from Mailbox

On Thu, Apr 23, 2015 at 12:09 PM, whataloadofwhat
notifications@github.com wrote:

I don't think there's anything which matches this particular example exactly.
The closest thing that I could find was DerefVec which includes all methods of Vec and doesn't separate them into different bounds like it does for Vec. So someone reading the docs for DerefVec may think that you can dedup on any DerefVec<T>, when in fact you can only do it where T: PartialEq.

http://doc.rust-lang.org/nightly/collections/vec/struct.DerefVec.html

Reply to this email directly or view it on GitHub:
#24686 (comment)

[sfackler]

I hit this trying to make a unified Ref<T> wrapper for rust-openssl: https://github.com/sfackler/rust-openssl/blob/ab30ad0ce76ee6fd8cc14ecff2186171c7f610e3/openssl/src/types.rs

I'll probably work around it by dropping the generic Ref wrapper and making a separate FooRef type for each Foo, but it's a bit unfortunate.

[QuietMisdreavus]

Fixing this likely involves figuring out how to filter the impls somewhere around here. That type_ there is the target type, so you'd have to reach into there and make some kind of ,retain() call on non_trait that matched not just on the DefId (which just gets the top container) but also any typarams on the full type itself. The impl's for_ will have the type to match on.

[lolbinarycat]

Did some more looking into this, and I think the issue is here, where we are throwing away the generic arguments of the Target associated type:

rust/src/librustdoc/html/render/mod.rs

Line 1417 in 617aad8

clean::AssocTypeItem(box ref t, _) => Some(match *t {

Unfortunately, I can't find the other half, the generic arguments of the impl block that the methods are from, so I can't fix this, but I'll be adding a known-bug test in a PR.

[lolbinarycat]

This seems to have regressed somewhat recently, as now the two impl blocks are merged into one!

[lolbinarycat]

Is "methods from deref" a field in rustdoc-json output? if it is, and it has the same bug, this likely needs to be fixed somewhere outside of render.

[lolbinarycat]

Figured it out, the key function is is_doc_subtype_of.

[lolbinarycat]

Hmm.. it seems the sidebar is generated separately...