From 1cfc1874b526fd8a681ebfaf64c554077586c8b1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 May 2021 18:18:32 +0200 Subject: [PATCH 1/3] document PartialEq, PartialOrd, Ord requirements more explicitly --- library/core/src/cmp.rs | 76 ++++++++++++++++++++++++++++------------- 1 file changed, 53 insertions(+), 23 deletions(-) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index 3f0acf435fe..fc02e3a3a1b 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -27,12 +27,23 @@ use self::Ordering::*; /// Trait for equality comparisons which are [partial equivalence /// relations](https://en.wikipedia.org/wiki/Partial_equivalence_relation). /// +/// `x.eq(y)` can also be written `x == y`, and `x.ne(y)` can be written `x != y`. +/// We use the easier-to-read infix notation in the remainder of this documentation. +/// /// This trait allows for partial equality, for types that do not have a full /// equivalence relation. For example, in floating point numbers `NaN != NaN`, /// so floating point types implement `PartialEq` but not [`trait@Eq`]. /// -/// Formally, the equality must be (for all `a`, `b`, `c` of type `A`, `B`, -/// `C`): +/// Implementations must ensure that `eq` and `ne` are consistent with each other: +/// `a != b` if and only if `!(a == b)`. +/// This is ensured by the default implementation of `ne`. +/// If [`PartialOrd`] or [`Ord`] are also implemented for `Self` and `Rhs`, their methods must also +/// be consistent with `PartialEq` (see the documentation of those traits for the exact +/// requirememts). It's easy to accidentally make them disagree by deriving some of the traits and +/// manually implementing others. +/// +/// The equality relation `==` must satisfy the following conditions +/// (for all `a`, `b`, `c` of type `A`, `B`, `C`): /// /// - **Symmetric**: if `A: PartialEq` and `B: PartialEq`, then **`a == b` /// implies `b == a`**; and @@ -53,15 +64,6 @@ use self::Ordering::*; /// /// ## How can I implement `PartialEq`? /// -/// `PartialEq` only requires the [`eq`] method to be implemented; [`ne`] is defined -/// in terms of it by default. Any manual implementation of [`ne`] *must* respect -/// 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: /// @@ -634,7 +636,19 @@ impl Clone for Reverse { /// An order is a total order if it is (for all `a`, `b` and `c`): /// /// - total and asymmetric: exactly one of `a < b`, `a == b` or `a > b` is true; and -/// - transitive, `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`. +/// - transitive: `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`. +/// +/// Implementations of [`PartialEq`], [`PartialOrd`], and `Ord` *must* agree with each other (for +/// all `a` and `b`): +/// +/// - `a.partial_cmp(b) == Some(a.cmp(b))` +/// - `a.cmp(b) == Ordering::Equal` if and only if `a == b` +/// (already follows from the above and the requirements of `PartialOrd`) +/// +/// It's easy to accidentally make them disagree by +/// deriving some of the traits and manually implementing others. +/// +/// Furthermore, the `max`, `min`, and `clamp` methods of this trait must be consistent with `cmp`. /// /// ## Derivable /// @@ -659,12 +673,6 @@ impl Clone for Reverse { /// 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. That is, `a.cmp(b) == Ordering::Equal` if -/// and only if `a == b` and `Some(a.cmp(b)) == a.partial_cmp(b)` for -/// all `a` and `b`. 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`: /// @@ -824,15 +832,41 @@ impl PartialOrd for Ordering { /// Trait for values that can be compared for a sort-order. /// +/// The `lt`, `le`, `gt`, and `ge` methods of this trait can be called using +/// the `<`, `<=`, `>`, and `>=` operators, respectively. +/// +/// The methods of this trait must be consistent with each other and with those of `PartialEq` in +/// the following sense: +/// +/// - `a == b` if and only if `partial_cmp(a, b) == Some(Equal)`. +/// - `a < b` if and only if `partial_cmp(a, b) == Some(Less)`. +/// - `a > b` if and only if `partial_cmp(a, b) == Some(Greater)`. +/// - `a <= b` if and only if `a < b || a == b`. +/// - `a >= b` if and only if `a > b || a == b`. +/// - `a != b` if and only if `!(a == b)` (already part of `PartialEq`). +/// +/// If [`Ord`] is also implemented for `Self` and `Rhs`, it must also be consistent with +/// `partial_cmp` (see the documentation of that trait for the exact requirements). It's +/// easy to accidentally make them disagree by deriving some of the traits and manually +/// implementing others. +/// /// The comparison must satisfy, for all `a`, `b` and `c`: /// -/// - asymmetry: if `a < b` then `!(a > b)`, as well as `a > b` implying `!(a < b)`; and /// - transitivity: `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`. +/// - duality: `a < b` if and only if `b > a`. /// /// Note that these requirements mean that the trait itself must be implemented symmetrically and /// transitively: if `T: PartialOrd` and `U: PartialOrd` then `U: PartialOrd` and `T: /// PartialOrd`. /// +/// ## Corollaries +/// +/// The following corollaries follow from the above requirements: +/// +/// - irreflexivity of `<` and `>`: `!(a < a)`, `!(a > a)` +/// - transitivity of `>`: if `a > b` and `b > c` then `a > c` +/// - duality of `partial_cmp`: `partial_cmp(a, b) == partial_cmp(b, a).map(Ordering::reverse)` +/// /// ## Derivable /// /// This trait can be used with `#[derive]`. When `derive`d on structs, it will produce a @@ -850,10 +884,6 @@ 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`]: /// /// ``` From a7abd130926a42f1cbaf4c03d4a5fa3450bd3789 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 27 May 2021 12:01:49 +0200 Subject: [PATCH 2/3] make Ord doc style consistent with the other two; explain which properties are ensured by default impls --- library/core/src/cmp.rs | 45 ++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index fc02e3a3a1b..a17cd632583 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -35,8 +35,10 @@ use self::Ordering::*; /// so floating point types implement `PartialEq` but not [`trait@Eq`]. /// /// Implementations must ensure that `eq` and `ne` are consistent with each other: -/// `a != b` if and only if `!(a == b)`. -/// This is ensured by the default implementation of `ne`. +/// +/// - `a != b` if and only if `!(a == b)` +/// (ensured by the default implementation). +/// /// If [`PartialOrd`] or [`Ord`] are also implemented for `Self` and `Rhs`, their methods must also /// be consistent with `PartialEq` (see the documentation of those traits for the exact /// requirememts). It's easy to accidentally make them disagree by deriving some of the traits and @@ -633,22 +635,25 @@ impl Clone for Reverse { /// Trait for types that form a [total order](https://en.wikipedia.org/wiki/Total_order). /// -/// An order is a total order if it is (for all `a`, `b` and `c`): +/// Implementations must ensure to be consistent with the [`PartialOrd`] implementation, and that +/// `max`, `min`, and `clamp` are consistent with `cmp`: /// -/// - total and asymmetric: exactly one of `a < b`, `a == b` or `a > b` is true; and -/// - transitive: `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`. +/// - `partial_cmp(a, b) == Some(cmp(a, b))`. +/// - `max(a, b) == max_by(a, b, cmp)` (ensured by the default implementation). +/// - `min(a, b) == min_by(a, b, cmp)` (ensured by the default implementation). +/// - For `a.clamp(min, max)`, see the [method docs](#method.clamp) +/// (ensured by the default implementation). /// -/// Implementations of [`PartialEq`], [`PartialOrd`], and `Ord` *must* agree with each other (for -/// all `a` and `b`): -/// -/// - `a.partial_cmp(b) == Some(a.cmp(b))` -/// - `a.cmp(b) == Ordering::Equal` if and only if `a == b` -/// (already follows from the above and the requirements of `PartialOrd`) -/// -/// It's easy to accidentally make them disagree by +/// It's easy to accidentally make `cmp` and `partial_cmp` disagree by /// deriving some of the traits and manually implementing others. /// -/// Furthermore, the `max`, `min`, and `clamp` methods of this trait must be consistent with `cmp`. +/// ## Corollaries +/// +/// From the above and the requirements of `PartialOrd`, it follows that `<` defines a strict total order. +/// This means that for all `a`, `b` and `c`: +/// +/// - exactly one of `a < b`, `a == b` or `a > b` is true; and +/// - `<` is transitive: `a < b` and `b < c` implies `a < c`. The same must hold for both `==` and `>`. /// /// ## Derivable /// @@ -839,10 +844,14 @@ impl PartialOrd for Ordering { /// the following sense: /// /// - `a == b` if and only if `partial_cmp(a, b) == Some(Equal)`. -/// - `a < b` if and only if `partial_cmp(a, b) == Some(Less)`. -/// - `a > b` if and only if `partial_cmp(a, b) == Some(Greater)`. -/// - `a <= b` if and only if `a < b || a == b`. -/// - `a >= b` if and only if `a > b || a == b`. +/// - `a < b` if and only if `partial_cmp(a, b) == Some(Less)` +/// (ensured by the default implementation). +/// - `a > b` if and only if `partial_cmp(a, b) == Some(Greater)` +/// (ensured by the default implementation). +/// - `a <= b` if and only if `a < b || a == b` +/// (ensured by the default implementation). +/// - `a >= b` if and only if `a > b || a == b` +/// (ensured by the default implementation). /// - `a != b` if and only if `!(a == b)` (already part of `PartialEq`). /// /// If [`Ord`] is also implemented for `Self` and `Rhs`, it must also be consistent with From 45675f3d9527a5212e96c922dfe4aa1efb7c7a4f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 15 Jun 2021 10:20:08 +0200 Subject: [PATCH 3/3] wording Co-authored-by: Jane Lusby --- library/core/src/cmp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index a17cd632583..34f7764c49b 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -635,7 +635,7 @@ impl Clone for Reverse { /// Trait for types that form a [total order](https://en.wikipedia.org/wiki/Total_order). /// -/// Implementations must ensure to be consistent with the [`PartialOrd`] implementation, and that +/// Implementations must be consistent with the [`PartialOrd`] implementation, and ensure /// `max`, `min`, and `clamp` are consistent with `cmp`: /// /// - `partial_cmp(a, b) == Some(cmp(a, b))`.