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.

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

Nightly docs missing relevant float parsing impls #31948

Closed
hanna-kruppe opened this issue Feb 28, 2016 · 5 comments · Fixed by #33002
Closed

Nightly docs missing relevant float parsing impls #31948

hanna-kruppe opened this issue Feb 28, 2016 · 5 comments · Fixed by #33002
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@hanna-kruppe
Copy link
Contributor

#31903 hid all trait impls from doc(hidden) modules. This was a step in the right direction, since it removed several dead links (mentions of traits that really should be hidden). Unfortunately currently FromStr is implemented in the hidden module core::num::dec2flt and thus the current nightly doesn't document this impl at all (neither on the trait's page nor on the primitive pages). For the same reason, ParseFloatError doesn't seem to implement any traits (except Error o.O perhaps because of the bound on the associated type FromStr::Error).

@hanna-kruppe
Copy link
Contributor Author

cc @mitaa @alexcrichton

Since AFAIK one can't override this behavior with an explicit #[doc(..)] annotation (and several of the traits that ParseFloatError is missing are derived) I fear we have to either ditch the strategy introduced by #31903 or move impl FromStr for $float_type and ParseFloatError back up to core::num. The latter doesn't help other code which might have the same problem. Is there an easy way to implement a rule such as this?

if the trait or implementing type is hidden, don't document the impl

@alexcrichton
Copy link
Member

To me when thinking about #31903 it seemed like a mild antipattern to have public interfaces you want documented in a #[doc(hidden)] module. Would it be "easy enough" to move these impls up to core::num? If privacy or something gets in the way we can reconsider reverting perhaps.

@hanna-kruppe
Copy link
Contributor Author

Well it used to be that way, then (IIRC) you moved the impls into core::num::dec2flt, so it should just work.

@mitaa
Copy link
Contributor

mitaa commented Feb 29, 2016

FWIW, I thought of another related testcase which is broken in the same way

// foo.rs
extern crate bar;

pub use bar::Foo;
// bar.rs
pub struct Foo;

mod private {
    trait Bar {}
    impl Bar for ::Foo {}
}

(the impl-inlining code doesn't look at privacy at all)

AIUI this would need to be solved in the same or similar way as necessary for

if the trait or implementing type is hidden, don't document the impl

@alexcrichton
Copy link
Member

Oh maybe it was privacy related in that case...

@alexcrichton alexcrichton added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Mar 8, 2016
bors added a commit that referenced this issue Apr 19, 2016
rustdoc: refine cross-crate impl inlining

This changes the current rule that impls within `doc(hidden)` modules aren't inlined, to only inlining impls where the implemented trait and type are reachable in documentation.

fixes #14586
fixes #31948

.. and also applies the reachability checking to cross-crate links.

fixes #28480

r? @alexcrichton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants