Clarify documentation of slice sorting methods #85985
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
this is a @rust-lang/libs decision, so I would prefer that someone from that team review. |
joshtriplett
reviewed
Jun 7, 2021
library/core/src/slice/mod.rs
Outdated
| /// [`Result::Err`] is returned, containing the index where a matching | ||
| /// element could be inserted while maintaining sorted order. | ||
| /// one of the matches could be returned. The index is chosen | ||
| /// deterministically, but is subject to change between releases. If the |
There was a problem hiding this comment.
Suggested change
| /// deterministically, but is subject to change between releases. If the | |
| /// deterministically, but is subject to change in future versions of Rust. If the |
(Likewise for the other two comments.)
|
Seems reasonable to me, modulo the minor comment nit I posted (which applies to all three comments). |
|
@bors r+ rollup |
|
📌 Commit fddf012 has been approved by |
bors
added
S-waiting-on-bors
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Jun 8, 2021
Clarify documentation of slice sorting methods After reading about [this](https://polkadot.network/a-polkadot-postmortem-24-05-2021/), I realized that although the documentation of these methods is not ambiguous in its current state, it is very easy to read it and erroneously assume that their exact behaviour can be relied upon to be deterministic. Although the docs make no guarantees about which index is returned when there are multiple matches, being more explicit about when and how their determinism can be relied upon should help prevent people from making this mistake in the future. r? `@steveklabnik`
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 8, 2021
Rollup of 11 pull requests Successful merges: - rust-lang#85906 (Use `Iterator::find` instead of open-coding it) - rust-lang#85951 (Update the documentation of `-C force-unwind-tables` for rust-lang#83482) - rust-lang#85985 (Clarify documentation of slice sorting methods) - rust-lang#85989 (Remove rustfmt tests from top-level .gitattributes) - rust-lang#86074 (Default panic message should print Box<dyn Any>) - rust-lang#86078 (Type page font weight) - rust-lang#86090 (:arrow_up: rust-analyzer) - rust-lang#86095 (Search description codeblock) - rust-lang#86096 (Comment out unused error codes and add description for E0316) - rust-lang#86101 (Correct type signature in doc for Bound::as_mut) - rust-lang#86103 (Remove lifetime hack) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After reading about this, I realized that although the documentation of these methods is not ambiguous in its current state, it is very easy to read it and erroneously assume that their exact behaviour can be relied upon to be deterministic. Although the docs make no guarantees about which index is returned when there are multiple matches, being more explicit about when and how their determinism can be relied upon should help prevent people from making this mistake in the future.
r? @steveklabnik