Clarify that TotalEq needs an underlying equivalence relation and that TotalOrd
needs a total ordering and specifically named the required (and sufficient)
attributes.
`TotalEq` is now just an assertion about the `Eq` impl of a
type (i.e. `==` is a total equality if a type implements `TotalEq`) so
the extra method is just confusing.
Also, a new method magically appeared as a hack to allow deriving to
assert that the contents of a struct/enum are also TotalEq, because the
deriving infrastructure makes it very hard to do anything but create a
trait method. (You didn't hear about this horrible work-around from me
:(.)
The `Float` trait provides correct `min` and `max` methods on floating
point types, providing a consistent result regardless of the order the
parameters are passed.
These generic functions do not take the necessary performance hit to
correctly support a partial order, so the true requirement should be
given as a type bound.
Closes#12712
* `Ord` inherits from `Eq`
* `TotalOrd` inherits from `TotalEq`
* `TotalOrd` inherits from `Ord`
* `TotalEq` inherits from `Eq`
This is a partial implementation of #12517.
Formatting via reflection has been a little questionable for some time now, and
it's a little unfortunate that one of the standard macros will silently use
reflection when you weren't expecting it. This adds small bits of code bloat to
libraries, as well as not always being necessary. In light of this information,
this commit switches assert_eq!() to using {} in the error message instead of
{:?}.
In updating existing code, there were a few error cases that I encountered:
* It's impossible to define Show for [T, ..N]. I think DST will alleviate this
because we can define Show for [T].
* A few types here and there just needed a #[deriving(Show)]
* Type parameters needed a Show bound, I often moved this to `assert!(a == b)`
* `Path` doesn't implement `Show`, so assert_eq!() cannot be used on two paths.
I don't think this is much of a regression though because {:?} on paths looks
awful (it's a byte array).
Concretely speaking, this shaved 10K off a 656K binary. Not a lot, but sometime
significant for smaller binaries.
Basically, generic containers should not use the default methods since a
type of elements may not guarantees total order. str could use them
since u8's Ord guarantees total order. Floating point numbers are also
broken with the default methods because of NaN. Thanks for @thestinger.
Timespec also guarantees total order AIUI. I'm unsure whether
extra::semver::Identifier does so I left it alone. Proof needed.
Signed-off-by: OGINO Masanori <masanori.ogino@gmail.com>
Previously it would call:
f(sf1.cmp(&of1), f(sf2.cmp(&of2), ...))
(where s/of1 = 'self/other field 1', and f was
std::cmp::lexical_ordering)
This meant that every .cmp subcall got evaluated when calling a derived
TotalOrd.cmp.
This corrects this to use
let test = sf1.cmp(&of1);
if test == Equal {
let test = sf2.cmp(&of2);
if test == Equal {
// ...
} else {
test
}
} else {
test
}
This gives a lexical ordering by short-circuiting on the first comparison
that is not Equal.
It will be simpler to implement only one method for Ord, while we also
allow implementing all four Ord methods for semantics or performance
reasons.
We only supply three default methods (and not four), because don't have
any nice error reporting for the case where at least one method must be
implemented, but it's arbitrary which.
