rustdoc: use stability, instead of features, to decide what to show #124864
Conversation
rustbot
added
S-waiting-on-review
notriddle
force-pushed
the
notriddle/feature-flags-are-not-stability-markers
branch
from
a7efed9 to
ffee30a
Compare
|
Thanks for taking care of this! |
| return; | ||
| } | ||
|
|
||
| if !tcx.features().rustc_private && !cx.render_options.force_unstable_if_unmarked { |
There was a problem hiding this comment.
quick question, why wouldn't the following be a potential solution:
if !cx.render_options.force_unstable_if_unmarked {(just removing the F-rustc_private check)
There was a problem hiding this comment.
It would be a potential solution, but it requires more code than is_compiler_internal(LOCAL_CRATE.as_def_id()). I'm trying to simplify this code, because I had trouble understanding the old version.
| #![crate_name = "foo"] | ||
| #![feature(rustc_private)] | ||
|
|
||
| extern crate issue_76736_1; | ||
| extern crate issue_76736_2; | ||
|
|
||
| // @has foo/struct.Foo.html | ||
| // @has - '//*[@class="impl"]//h3[@class="code-header"]' 'MaybeResult' | ||
| // @!has - '//*[@class="impl"]//h3[@class="code-header"]' 'MaybeResult' |
There was a problem hiding this comment.
I don't know, couldn't this impl be useful? The user is explicitly opting into using internal APIs with #![feature(rustc_private)], then why should we refuse them access to potential useful information (here: “impl MaybeResult for Foo” via the upstream blanket impl, which they can indeed make use of)
I feel like I'm missing something obvious. If I'm not mistaken the suggestion from my review comment one above wouldn't throw this impl out and would still fix the linked issue.
There was a problem hiding this comment.
“The user,” in this case, is the standard library. It uses the feature rudtc_private, but doesn’t want private items to show up in its docs.
There was a problem hiding this comment.
Well, the std library doesn't contain #[feature(rustc_internal)] directly as you probably know but yeah.
There are other users of rustc_internal. I' on mobile rn but yesterday I grepped through compiler/ and saw the feature being enabled for one of the alternate backends (cranelift or gcc, I don't remember) and I assume rustfmt enables it too for rustc_{parse,ast}?
I'm just nitpicking but I don't wanna block this PR any longer.
To decide if internal items should be inlined in a doc page, check if the crate is itself internal, rather than if it has the rustc_private feature flag. The standard library uses internal items, but is not itself internal and should not show internal items on its docs pages.
notriddle
force-pushed
the
notriddle/feature-flags-are-not-stability-markers
branch
from
ffee30a to
6d6f67a
Compare
There was a problem hiding this comment.
The approach looks fine, although I still have some doubts as elaborated in my other comments.
I wonder if the tests should use //@ compile-flags: -Zforce-unstable-if-unmarked instead of #![feature(rustc_internals)] to be “closer to reality” but eh, not pressing.
|
Does this need backporting? @bors r+ rollup |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#124548 (Handle normalization failure in `struct_tail_erasing_lifetimes`) - rust-lang#124761 (Fix insufficient logic when searching for the underlying allocation) - rust-lang#124864 (rustdoc: use stability, instead of features, to decide what to show) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#124864 - notriddle:notriddle/feature-flags-are-not-stability-markers, r=fmease rustdoc: use stability, instead of features, to decide what to show Fixes rust-lang#124635 To decide if internal items should be inlined in a doc page, check if the crate is itself internal, rather than if it has the rustc_private feature flag. The standard library uses internal items, but is not itself internal and should not show internal items on its docs pages.
notriddle
added
the
beta-nominated
|
Backport to beta approved as per Zulip poll. @rustbot label beta-accepted |
rustbot
added
the
beta-accepted
cuviper
removed
the
beta-nominated
[beta] backports - Do not ICE on foreign malformed `diagnostic::on_unimplemented` rust-lang#124683 - Fix more ICEs in `diagnostic::on_unimplemented` rust-lang#124875 - rustdoc: use stability, instead of features, to decide what to show rust-lang#124864 - Don't do post-method-probe error reporting steps if we're in a suggestion rust-lang#125100 - Make `non-local-def` lint Allow by default rust-lang#124950 r? cuviper
Fixes #124635
To decide if internal items should be inlined in a doc page, check if the crate is itself internal, rather than if it has the rustc_private feature flag. The standard library uses internal items, but is not itself internal and should not show internal items on its docs pages.