Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OrderedFloat PartialOrd implementation should match Ord implementation #34

Closed
cake4289 opened this issue Oct 19, 2017 · 1 comment
Closed

Comments

@cake4289
Copy link

Otherwise OrderedFloat is not ordered at all, breaking the assumptions of any algorithm that uses it.

The documentation for Ord says

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.

The following test fails:

extern crate ordered_float;

use std::f64::NAN;
use ordered_float::OrderedFloat;

#[test]
fn less_than_nan() {
    let x = OrderedFloat(1.0);
    let y = OrderedFloat(NAN);
    assert!(x < y);
}

None of the tests in this crate cover using the comparison operators; they all use Ord::cmp directly. This should be fixed. The current code is unsound and could lead to unsafety.

@mbrubeck
Copy link
Collaborator

mbrubeck commented Sep 7, 2018

This is fixed on master.

@mbrubeck mbrubeck closed this as completed Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants