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

Conversation

birkenfeld
Copy link
Contributor

Fixes: #30607

@rust-highfive
Copy link
Contributor

r? @cmr

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

This seems sort of "extra special" to me in the sense that it's a bit of an odd condition to have in rustdoc. I'd personally be inclined to close the original issue as "not a bug", but curious what others think?

@steveklabnik
Copy link
Member

I am torn. Showing that the method is there is what's accurate; if it can't be called for other reasons, well, it can't be called. Dunno.

@emberian
Copy link
Member

emberian commented May 4, 2016

I somewhat agree, @alexcrichton. I think that if we were to have this sort of feature, we'd want to have some infrastructure in the compiler for determining which methods are reachable for a certain type, which would exclude such impossibilities, and render only those. The logic doesn't belong in rustdoc, but it's a "nice to have".

@alexcrichton
Copy link
Member

@steveklabnik in some sense the information is still accurate, it's just saying that the methods on the deref'd type T have some by-value methods, but you need to implicitly know that Deref only gives a reference, not something by value. It is magical, and perhaps the header for "methods inherited" could indicate this.

Additionally, this wouldn't work for types like impl<T: Copy> Copy for Foo<T>. That sort of conditional logic won't be handled here and would still need to be understood.

@birkenfeld
Copy link
Contributor Author

Additionally, this wouldn't work for types like impl<T: Copy> Copy for Foo. That sort of conditional logic won't be handled here and would still need to be understood.

That's a good point. I think I agree that this change is trying too hard. I'm fine with closing.

@alexcrichton
Copy link
Member

Ok, I'm gonna close this for now, but thanks for the PR @birkenfeld!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants