rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.47k stars 12.73k forks source link

Rustdoc displays private internals of associated constants when reexporting types #99630

Open analog-hors opened 2 years ago

analog-hors commented 2 years ago

types/src/lib.rs:

pub struct HasPrivateFields {
    _private: (),
}

impl HasPrivateFields {
    pub const ASSOC: Self = Self { _private: () };
}

main/src/lib.rs:

pub use types::HasPrivateFields;

This leaks private fields: Doc page displaying private fields in associated constant initializers

Tested with cargo clean && cargo +nightly doc.

Note that this also applies to verbose initializers.

This issue was first brought up in #97933. The previous issue was closed due to a hotfix implemented in PR #98814. However, the hotfix fails to handle the case in this issue.

Meta

rustc +nightly --version --verbose:

rustc 1.64.0-nightly (848090dcd 2022-07-22)
binary: rustc
commit-hash: 848090dcd18553b790461132ca9d2a020aeea9a2
commit-date: 2022-07-22
host: x86_64-pc-windows-msvc
release: 1.64.0-nightly
LLVM version: 14.0.6
fmease commented 2 years ago

Huh, that's unexpected. Let me look into that.

@rustbot label T-rustdoc A-rustdoc-ui A-cross-crate-reexports @rustbot claim

analog-hors commented 2 years ago

By the way, this is currently affecting my crate cozy-chess, which defines associated constants using a macro. The macro expands to a highly verbose expression, which rustdoc then documents, leaving the documents unreadable. I don't particularly like that rustdoc makes assumptions about what the readable form of a constant looks like. Could there be a temporary fix in the meantime? image

fmease commented 2 years ago

~This is already fixed on nightly (#98814).~

Since you are requiring a stable rustc, one workaround is to move the value of BitBoard::EDGES to a (private) free-standing const item EDGES and then set BitBoard::EDGES to EDGES. Sorry for the inconvenience. The PR mentioned above sadly wasn't backported to beta (yet).

analog-hors commented 2 years ago

To my knowledge, the linked PR does not fix the issue, which occurs when reexporting types.

fmease commented 2 years ago

Ah, I see (cozy-chesscozy-chess-types). Still, the workaround should work, right?

This issue will be fixed by my open PR #99688. However it is massive and a bit controversial I think, so reviews have stalled. I am still very eager to land it and I just contacted the relevant people so work on the PR can pick up pace again.

MinusKelvin commented 2 years ago

I think it would be good to have a separate hotfix PR to make the behavior in this case match that implemented in #98814, as this is clearly what was intended in the first place.

I think it may also be worth noting that there doesn't seem to be anywhere for experimental rustdoc features (I think it is clear that displaying non-trivial constant values is experimental) to live prior to being included in rustdoc proper. Even in nightly, rustc has feature gates, but as far as I'm aware rustdoc has no analogous system. This is further exasperated by the fact that docs.rs uses nightly to build documentation, making nightly rustdoc the de facto production Rust documentation generator.

Nemo157 commented 2 years ago

We do have some features that change rustdocs behaviour, like doc_auto_cfg, that seems like it could be an ok model to allow opt-in for new features we might want to make default in the future?

EDIT: though it has the downside that it's unlikely to be widely used unless it's something very useful like the cfg-docs. Having changes rollout to new builds on docs.rs quickly probably results in finding and fixing issues much faster than waiting for them to be stabilized. Less people build and thoroughly inspect docs locally.