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.

Sign up for GitHub

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

Jump to bottom

Improve rustdoc const bounds #93412

Merged
merged 5 commits into from
Mar 6, 2022
Merged

Improve rustdoc const bounds #93412

merged 5 commits into from
Mar 6, 2022

Conversation

fee1-dead
Copy link
Member

@fee1-dead fee1-dead commented Jan 28, 2022 •
edited
Loading

  • Rustdoc no longer displays ~const in trait bounds, because it currently means nothing for stable users, and because we still haven't decided on the final syntax yet.
  • Rustdoc will hide trait bounds where the trait is Drop AND it is ~const, i.e. ~const Drop bounds because it has no effect on stable users as well.
  • Because of additional logic that hides the whole where statement where it consists of ~const Drop bounds (so it doesn't display struct Foo<T>() where ; like that), bounds that have no trait e.g. where [T; N+1]: ; are also hidden.

Cherry-picked from #92433.

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

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

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jan 28, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 28, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@fee1-dead fee1-dead mentioned this pull request Jan 28, 2022
Closed
@GuillaumeGomez
Copy link
Member

Looks good to me overall, thanks! Your PR seems to done a bit more than what the test you updated seems to check. Can you add more checks please?

Also cc @camelid in case you want to take a second look. :)

camelid
camelid reviewed Jan 29, 2022
View reviewed changes
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

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

Please also add some tests of the expected behavior, e.g. for skipping the ~const part or excluding ~const Drop.

@fee1-dead
Copy link
Member Author

I have addressed the comments and added a test for the display.

@GuillaumeGomez
Copy link
Member

Looks good to me, thanks! Waiting for @camelid now. ;)

@camelid
Copy link
Member

camelid commented Feb 23, 2022

Sorry, I don't have time to review the code. Could you summarize the current version of the changes in this PR?

@fee1-dead
Copy link
Member Author

  • Hide ~const in rustdoc, it is provisional syntax.
  • Hide ~const Drop bounds completely because they do not mean anything to stable users.

@fee1-dead
Copy link
Member Author

It also hides const generic wf bounds: [T; SIZE]: (no trait) will be hidden

@camelid
Copy link
Member

camelid commented Feb 23, 2022

Sounds reasonable. Can you add something to the tracking issue for ~const about reconsidering the rustdoc display when stabilizing? One more thing: are ~const Trait bounds no-ops for stable users or are they interpreted as regular Trait bounds?

@fee1-dead
Copy link
Member Author

fee1-dead commented Feb 23, 2022 •
edited
Loading

Sure, I'll update the issue when I have access to a computer.

As for : ~const Trait they behave as : Trait to stable users. Only : ~const Drop is a no-op to stable users.

@fee1-dead
Copy link
Member Author

@camelid: Did you want to review this PR when you have time again? Or do you think it is fine as is?

@mbartlett21 mbartlett21 mentioned this pull request Feb 28, 2022
Merged
@camelid
Copy link
Member

camelid commented Mar 2, 2022

The changes sound good to me. I haven't reviewed the code, but as long as Guillaume has, I don't think I need to. It'd be good to update the PR description with your extended description of the changes, but other than that I think this should be good.

@GuillaumeGomez
Copy link
Member

Please ping me once the PR description has been updated so I can approve it.

@fee1-dead fee1-dead mentioned this pull request Mar 6, 2022
Merged
@fee1-dead
Copy link
Member Author

@GuillaumeGomez: I have updated the PR description.

@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r=

@bors
Copy link
Contributor

bors commented Mar 6, 2022

📌 Commit 484936b has been approved by ``

@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 Mar 6, 2022
@GuillaumeGomez
Copy link
Member

Second time will work better...

@bors r+

@bors
Copy link
Contributor

bors commented Mar 6, 2022

📌 Commit 484936b has been approved by GuillaumeGomez

@matthiaskrgr matthiaskrgr mentioned this pull request Mar 6, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2022
@bors
Auto merge of rust-lang#94673 - matthiaskrgr:rollup-2tnifg9, r=matthi…
a64180f 
…askrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#93412 (Improve rustdoc const bounds)
 - rust-lang#94617 (Update `itertools`)
 - rust-lang#94669 (Update -Z unpretty error message)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit faf1a7f into rust-lang:master Mar 6, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 6, 2022
@fee1-dead fee1-dead deleted the improve-rustdoc-const-bounds branch June 15, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camelid camelid camelid left review comments

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

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
Milestone
1.61.0
Development

Successfully merging this pull request may close these issues.
7 participants
@fee1-dead @rust-highfive @rust-log-analyzer @GuillaumeGomez @camelid @bors @rustbot