about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMark Simulacrum <mark.simulacrum@gmail.com>2017-05-27 20:54:04 -0600
committerGitHub <noreply@github.com>2017-05-27 20:54:04 -0600
commit423b410fcef856cd45cca3179119d62dd3a04af6 (patch)
treef01300cc11a5676a794a1216108f7840dde788e6
parent906c9bcfb97c4618765e6a8b8fb009ccbc4dbd3b (diff)
parentf5421367a2d72d5da415d2bbf97538099ff3ade0 (diff)
Rollup merge of #42260 - stjepang:document-cmp-traits-agreement, r=alexcrichton
Docs: impls of PartialEq/PartialOrd/Ord must agree

Fixes #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
-rw-r--r--src/libcollections/binary_heap.rs5
-rw-r--r--src/libcore/cmp.rs16
2 files changed, 18 insertions, 3 deletions
diff --git a/src/libcollections/binary_heap.rs b/src/libcollections/binary_heap.rs
index 4e9dea09f79..988f8851625 100644
--- a/src/libcollections/binary_heap.rs
+++ b/src/libcollections/binary_heap.rs
@@ -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))
 //!     }
 //! }
 //!
diff --git a/src/libcore/cmp.rs b/src/libcore/cmp.rs
index d4544dadaeb..661cf73c7f3 100644
--- a/src/libcore/cmp.rs
+++ b/src/libcore/cmp.rs
@@ -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:
 ///
@@ -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`:
 ///
@@ -474,8 +482,8 @@ 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 ==
@@ -483,6 +491,10 @@ impl PartialOrd for Ordering {
 ///
 /// `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()`:
 ///
 /// ```