num_rational::Ratio's Hash is incompatible with Eq.

[kennytm]

As of 0.1.38, num_rational::Ratio implements PartialEq and Eq by the numeric value, while Hash is derived. This violates the rule of Eq + Hash which is a == b ⇒ hash(a) == hash(b) when non-reduced ratio is involved:

let a = Rational::new_raw(4, 2);
let b = Rational::new_raw(6, 3);
assert_eq!(a, b); // ok.

let mut h = HashSet::new();
h.insert(a);
h.insert(b);
assert_eq!(h.len(), 1); // likely panic, 2 ≠ 1

The hash should be computed using the reduced value. This unfortunately means the GCD will need to be calculated every time we need the hash.

[cuviper]

Ugh. Yes, I agree this is wrong, and needs to be fixed.

I wish we could just assume they're in reduced form as an invariant, but we'd probably need to make new_raw unsafe to signal that contract.

I wonder if it's worth trying to be more clever than performing a full GCD-reduction in Hash? I'm thinking something like this pseudo-code:

while (denom != 0) {
    hash(numer / denom);
    (numer, denom) = (denom, numer % denom);
}

That's basically what Ratio::cmp is doing too, used for Ratio::eq.

Incidentally, I think cmp is broken for zero denominators. Might just deserve a non-zero assertion, unless we want to treat them like infinities...