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

Option and Result are missing documentation on their internal order #87238

Closed
orlp opened this issue Jul 18, 2021 · 5 comments
Closed

Option and Result are missing documentation on their internal order #87238

orlp opened this issue Jul 18, 2021 · 5 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@orlp
Copy link
Contributor

orlp commented Jul 18, 2021 •
edited by rustbot
Loading

Option<T> where T: Ord implements Ord (and similarly for PartialOrd), but as far as I can see the order it uses isn't documented anywhere. You have to read the source to find out that its Ord implementation is derived, and that None comes before Some in the enum declaration thus None sorts before Some. I think that should be documented.

The same issue also appears to apply to Result.
@orlp orlp changed the title Option is missing documentation on its order Option and Result are missing documentation on their internal order Jul 18, 2021
@Alexendoo Alexendoo added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jul 25, 2021
@jesyspa
Copy link
Contributor

jesyspa commented Jul 27, 2021

@rustbot claim

@jesyspa
Copy link
Contributor

jesyspa commented Jul 28, 2021

Looking into this: the instances in those cases are derived, and it looks like adding extra documentation to derived traits has been proposed previously (#44118) but is not currently supported. So by the looks of it this is not feasible without changes to rustdoc?

@orlp
Copy link
Contributor Author

orlp commented Jul 30, 2021

Looking into this: the instances in those cases are derived, and it looks like adding extra documentation to derived traits has been proposed previously (#44118) but is not currently supported. So by the looks of it this is not feasible without changes to rustdoc?

You could replace the derived implementation with an equivalent explicit one, and then add a comment.

@jesyspa
Copy link
Contributor

jesyspa commented Jul 30, 2021 •
edited
Loading

@rustbot release-assignment

Sorry, I don't really feel qualified to judge whether the added code complexity would be worth it, so I'll leave this to someone with more experience. Changed my mind based on discussion on Zulip.

@jesyspa jesyspa mentioned this issue Jul 31, 2021
Merged
camsteffen added a commit to camsteffen/rust that referenced this issue Aug 2, 2021
@camsteffen
Rollup merge of rust-lang#87654 - jesyspa:issue-87238-option-result-d…
b1166e1 
…oc, r=scottmcm

Add documentation for the order of Option and Result

This resolves issue rust-lang#87238.
@pierwill
Copy link
Member

pierwill commented Aug 8, 2021

Looks like this issue was addressed in #87654.

@orlp orlp closed this as completed Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@orlp @jesyspa @Alexendoo @pierwill