Skip to content

Commit

Permalink
Rollup merge of #106985 - jofas:106746-fix, r=ChrisDenton
Browse files Browse the repository at this point in the history
Enhanced doucmentation of binary search methods for `slice` and `VecDeque` for unsorted instances

Fixes #106746. Issue #106746 raises the concern that the binary search methods for slices and deques aren't explicit enough about the fact that they are only applicable to sorted slices/deques. I changed the explanation for these methods. I took the relatively harsh description of the behaviour of binary search on unsorted collections ("unspecified and meaningless") from the description of the [`partition_point`](https://doc.rust-lang.org/std/primitive.slice.html#method.partition_point) method:

> If this slice is not partitioned, the returned result is unspecified and meaningless, as this method performs a kind of binary search.
  • Loading branch information
JohnTitor committed Mar 30, 2023
2 parents f2d9a3d + b085007 commit d6f2740
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 20 deletions.
20 changes: 10 additions & 10 deletions library/alloc/src/collections/vec_deque/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2394,7 +2394,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
}

/// Binary searches this `VecDeque` for a given element.
/// This behaves similarly to [`contains`] if this `VecDeque` is sorted.
/// If the `VecDeque` is not sorted, the returned result is unspecified and
/// meaningless.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
Expand All @@ -2404,7 +2405,6 @@ impl<T, A: Allocator> VecDeque<T, A> {
///
/// See also [`binary_search_by`], [`binary_search_by_key`], and [`partition_point`].
///
/// [`contains`]: VecDeque::contains
/// [`binary_search_by`]: VecDeque::binary_search_by
/// [`binary_search_by_key`]: VecDeque::binary_search_by_key
/// [`partition_point`]: VecDeque::partition_point
Expand Down Expand Up @@ -2450,12 +2450,13 @@ impl<T, A: Allocator> VecDeque<T, A> {
}

/// Binary searches this `VecDeque` with a comparator function.
/// This behaves similarly to [`contains`] if this `VecDeque` is sorted.
///
/// The comparator function should implement an order consistent
/// with the sort order of the deque, returning an order code that
/// indicates whether its argument is `Less`, `Equal` or `Greater`
/// than the desired target.
/// The comparator function should return an order code that indicates
/// whether its argument is `Less`, `Equal` or `Greater` the desired
/// target.
/// If the `VecDeque` is not sorted or if the comparator function does not
/// implement an order consistent with the sort order of the underlying
/// `VecDeque`, the returned result is unspecified and meaningless.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
Expand All @@ -2465,7 +2466,6 @@ impl<T, A: Allocator> VecDeque<T, A> {
///
/// See also [`binary_search`], [`binary_search_by_key`], and [`partition_point`].
///
/// [`contains`]: VecDeque::contains
/// [`binary_search`]: VecDeque::binary_search
/// [`binary_search_by_key`]: VecDeque::binary_search_by_key
/// [`partition_point`]: VecDeque::partition_point
Expand Down Expand Up @@ -2505,10 +2505,11 @@ impl<T, A: Allocator> VecDeque<T, A> {
}

/// Binary searches this `VecDeque` with a key extraction function.
/// This behaves similarly to [`contains`] if this `VecDeque` is sorted.
///
/// Assumes that the deque is sorted by the key, for instance with
/// [`make_contiguous().sort_by_key()`] using the same key extraction function.
/// If the deque is not sorted by the key, the returned result is
/// unspecified and meaningless.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
Expand All @@ -2518,7 +2519,6 @@ impl<T, A: Allocator> VecDeque<T, A> {
///
/// See also [`binary_search`], [`binary_search_by`], and [`partition_point`].
///
/// [`contains`]: VecDeque::contains
/// [`make_contiguous().sort_by_key()`]: VecDeque::make_contiguous
/// [`binary_search`]: VecDeque::binary_search
/// [`binary_search_by`]: VecDeque::binary_search_by
Expand Down
20 changes: 10 additions & 10 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2387,7 +2387,8 @@ impl<T> [T] {
}

/// Binary searches this slice for a given element.
/// This behaves similarly to [`contains`] if this slice is sorted.
/// If the slice is not sorted, the returned result is unspecified and
/// meaningless.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
Expand All @@ -2399,7 +2400,6 @@ impl<T> [T] {
///
/// See also [`binary_search_by`], [`binary_search_by_key`], and [`partition_point`].
///
/// [`contains`]: slice::contains
/// [`binary_search_by`]: slice::binary_search_by
/// [`binary_search_by_key`]: slice::binary_search_by_key
/// [`partition_point`]: slice::partition_point
Expand Down Expand Up @@ -2462,12 +2462,13 @@ impl<T> [T] {
}

/// Binary searches this slice with a comparator function.
/// This behaves similarly to [`contains`] if this slice is sorted.
///
/// The comparator function should implement an order consistent
/// with the sort order of the underlying slice, returning an
/// order code that indicates whether its argument is `Less`,
/// `Equal` or `Greater` the desired target.
/// The comparator function should return an order code that indicates
/// whether its argument is `Less`, `Equal` or `Greater` the desired
/// target.
/// If the slice is not sorted or if the comparator function does not
/// implement an order consistent with the sort order of the underlying
/// slice, the returned result is unspecified and meaningless.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
Expand All @@ -2479,7 +2480,6 @@ impl<T> [T] {
///
/// See also [`binary_search`], [`binary_search_by_key`], and [`partition_point`].
///
/// [`contains`]: slice::contains
/// [`binary_search`]: slice::binary_search
/// [`binary_search_by_key`]: slice::binary_search_by_key
/// [`partition_point`]: slice::partition_point
Expand Down Expand Up @@ -2548,10 +2548,11 @@ impl<T> [T] {
}

/// Binary searches this slice with a key extraction function.
/// This behaves similarly to [`contains`] if this slice is sorted.
///
/// Assumes that the slice is sorted by the key, for instance with
/// [`sort_by_key`] using the same key extraction function.
/// If the slice is not sorted by the key, the returned result is
/// unspecified and meaningless.
///
/// If the value is found then [`Result::Ok`] is returned, containing the
/// index of the matching element. If there are multiple matches, then any
Expand All @@ -2563,7 +2564,6 @@ impl<T> [T] {
///
/// See also [`binary_search`], [`binary_search_by`], and [`partition_point`].
///
/// [`contains`]: slice::contains
/// [`sort_by_key`]: slice::sort_by_key
/// [`binary_search`]: slice::binary_search
/// [`binary_search_by`]: slice::binary_search_by
Expand Down

0 comments on commit d6f2740

Please sign in to comment.