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

rustdoc: decouple stability and const-stability #91694

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

euclio
Copy link
Contributor

@euclio euclio commented Dec 9, 2021

This PR tweaks the stability rendering code to consider stability and const-stability separately. This fixes two issues:

  • Stabilities that match the enclosing item are now always omitted, even if the item has const-stability as well (Confusing display of 1.0.0 stability #90552)
  • Const-stable unstable functions will now have their (const-) stability rendered.

Fixes #90552.

@rust-highfive
Copy link
Collaborator

r? @ollie27

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 9, 2021
// Show const-stability even for unstable functions.
// @matches 'foo/struct.Bar.html' '//span[@class="since"]' '^const: 1.3.0$'
#[unstable(feature = "foo2", issue = "none")]
#[rustc_const_stable(feature = "rust1", since = "1.3.0")]
Copy link
Member

@Urgau Urgau Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is invalid, you cannot have a rustc_const_stable with an unstable attribute, but due to a bug in the compiler it's currently accepted despite being obviously wrong (cf. #79551).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree that it's "obviously wrong", allowing unstable functions to be called in a const-stable context is called out as a use-case in the rustc-guide.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, I didn't knew that. Thanks for that but I still think having both rustc_const_stable and unstable is "wrong" in a sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that unstable + rustc_const_stable doesn't make a lot of sense; stable functions can call unstable functions, so shouldn't rustc_const_stable functions be able to call rustc_const_unstable functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless of whether const-stable unstable functions are disallowed in the future, the implementation of this PR won't be affected: the invalid test case will just be removed.

For now, it's supported, so it makes sense to me to show it in the documentation.

@apiraino apiraino added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Dec 9, 2021
@bors
Copy link
Contributor

bors commented Jan 10, 2022

☔ The latest upstream changes (presumably #92719) made this pull request unmergeable. Please resolve the merge conflicts.

@euclio euclio force-pushed the stability-improvements branch from 5096afd to dc1c39b Compare January 13, 2022 16:59
@euclio
Copy link
Contributor Author

euclio commented Jan 13, 2022

Rebased.

@camelid
Copy link
Member

camelid commented Jan 13, 2022

r? @GuillaumeGomez


impl Bar {
// Do not show non-const stabilities that are the same as the enclosing item.
// @matches 'foo/struct.Bar.html' '//span[@class="since"]' '^const: 1.2.0$'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the 1.0.0 still displayed? If not, please put it back like it used to. In any case, please add a test to ensure both are displayed if both are present like it's currently the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GuillaumeGomez This is one of the goals of this PR. Currently, stabilities are omitted if the stability of the item matches the stability of the enclosing struct or module, except if they also have a const stability.

This PR updates the logic to always omit the stability if it matches, regardless of whether the item is const-stable or not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see. Thanks for the explanations!

@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 19, 2022

📌 Commit dc1c39b has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2022
…askrgr

Rollup of 13 pull requests

Successful merges:

 - rust-lang#89747 (Add MaybeUninit::(slice_)as_bytes(_mut))
 - rust-lang#89764 (Fix variant index / discriminant confusion in uninhabited enum branching)
 - rust-lang#91606 (Stabilize `-Z print-link-args` as `--print link-args`)
 - rust-lang#91694 (rustdoc: decouple stability and const-stability)
 - rust-lang#92183 (Point at correct argument when async fn output type lifetime disagrees with signature)
 - rust-lang#92582 (improve `_` constants in item signature handling)
 - rust-lang#92680 (intra-doc: Use the impl's assoc item where possible)
 - rust-lang#92704 (Change lint message to be stronger for &T -> &mut T transmute)
 - rust-lang#92861 (Rustdoc mobile: put out-of-band info on its own line)
 - rust-lang#92992 (Help optimize out backtraces when disabled)
 - rust-lang#93038 (Fix star handling in block doc comments)
 - rust-lang#93108 (:arrow_up: rust-analyzer)
 - rust-lang#93112 (Fix CVE-2022-21658)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 405cf20 into rust-lang:master Jan 20, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing display of 1.0.0 stability
9 participants