Skip to content

Commit

Permalink
Rollup merge of rust-lang#42260 - stjepang:document-cmp-traits-agreem…
Browse files Browse the repository at this point in the history
…ent, r=alexcrichton

Docs: impls of PartialEq/PartialOrd/Ord must agree

Fixes rust-lang#41270.

This PR brings two improvements to the docs:

1. Docs for `PartialEq`, `PartialOrd`, and `Ord` clarify that their implementations must agree.
2. Fixes a subtle bug in the Dijkstra example for `BinaryHeap`, where the impls are inconsistent.
Thanks @Rufflewind for spotting the bug!

r? @alexcrichton
cc @frankmcsherry
  • Loading branch information
Mark-Simulacrum authored May 28, 2017
2 parents e0589f1 + f542136 commit 2af8b0e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
5 changes: 4 additions & 1 deletion src/libcollections/binary_heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@
//! // instead of a max-heap.
//! impl Ord for State {
//! fn cmp(&self, other: &State) -> Ordering {
//! // Notice that the we flip the ordering here
//! // Notice that the we flip the ordering on costs.
//! // In case of a tie we compare positions - this step is necessary
//! // to make implementations of `PartialEq` and `Ord` consistent.
//! other.cost.cmp(&self.cost)
//! .then_with(|| self.position.cmp(&other.position))
//! }
//! }
//!
Expand Down
16 changes: 14 additions & 2 deletions src/libcore/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ use self::Ordering::*;
/// the rule that `eq` is a strict inverse of `ne`; that is, `!(a == b)` if and
/// only if `a != b`.
///
/// Implementations of `PartialEq`, `PartialOrd`, and `Ord` *must* agree with
/// each other. It's easy to accidentally make them disagree by deriving some
/// of the traits and manually implementing others.
///
/// An example implementation for a domain in which two books are considered
/// the same book if their ISBN matches, even if the formats differ:
///
Expand Down Expand Up @@ -386,6 +390,10 @@ impl<T: Ord> Ord for Reverse<T> {
/// Then you must define an implementation for `cmp()`. You may find it useful to use
/// `cmp()` on your type's fields.
///
/// Implementations of `PartialEq`, `PartialOrd`, and `Ord` *must* agree with each other. It's
/// easy to accidentally make them disagree by deriving some of the traits and manually
/// implementing others.
///
/// Here's an example where you want to sort people by height only, disregarding `id`
/// and `name`:
///
Expand Down Expand Up @@ -474,15 +482,19 @@ impl PartialOrd for Ordering {
///
/// ## How can I implement `PartialOrd`?
///
/// PartialOrd only requires implementation of the `partial_cmp` method, with the others generated
/// from default implementations.
/// `PartialOrd` only requires implementation of the `partial_cmp` method, with the others
/// generated from default implementations.
///
/// However it remains possible to implement the others separately for types which do not have a
/// total order. For example, for floating point numbers, `NaN < 0 == false` and `NaN >= 0 ==
/// false` (cf. IEEE 754-2008 section 5.11).
///
/// `PartialOrd` requires your type to be `PartialEq`.
///
/// Implementations of `PartialEq`, `PartialOrd`, and `Ord` *must* agree with each other. It's
/// easy to accidentally make them disagree by deriving some of the traits and manually
/// implementing others.
///
/// If your type is `Ord`, you can implement `partial_cmp()` by using `cmp()`:
///
/// ```
Expand Down

0 comments on commit 2af8b0e

Please sign in to comment.