Skip to content

NaN > NaN returns true #50811

Closed
Closed

Description

@jimblandy

The following tests do not pass for me, but I think they should, as comparisons with NaN should always simply evaluate to false:

#[test]
fn test_zero_zero() {
    assert_eq!(0.0/0.0 < 0.0/0.0, false);
    assert_eq!(0.0/0.0 > 0.0/0.0, false);
}

#[test]
fn test_nan_nan() {
    assert_eq!(std::f64::NAN < std::f64::NAN, false);
    assert_eq!(std::f64::NAN > std::f64::NAN, false);
}

fn main() {
    println!("Hello, world!");
}

I get this result on both debug and release builds. I'm on an x86_64 running Linux, rustc 1.27.0-nightly (f0fdaba 2018-05-15).

Activity

  1. jorendorff commented on May 16, 2018

    @jorendorff
    Contributor

    Playground link for this test.

    https://play.rust-lang.org/?gist=4d6f7f54553b065615caac65b3cb6222&version=nightly&mode=debug

    Affects both debug and release, Nightly and Stable.

  2. varkor commented on May 16, 2018

    @varkor
    Contributor

    Playground:

    fn main() {
        let operator = std::f64::NAN > std::f64::NAN;
        let method = std::f64::NAN.gt(&std::f64::NAN);
        println!("{} {}", operator, method); // false false
        assert_eq!(operator, method); // doesn't fail
        assert_eq!(std::f64::NAN > std::f64::NAN, std::f64::NAN.gt(&std::f64::NAN)); // fails
    }

    I think this is an issue with assert_eq! rather than the floating-point comparisons, but I haven't dug into it yet.

  3. added
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
    C-bugCategory: This is a bug.
    on May 16, 2018
  4. kennytm commented on May 16, 2018

    @kennytm
    Member

    This seems to be a regression introduced by 1.26.

  5. kennytm commented on May 16, 2018

    @kennytm
    Member

    Reduced, related to rvalue promotion I guess.

    extern { fn abort() -> !; }
    
    fn main() {
        let f = &(std::f64::NAN > std::f64::NAN);
        if *f {
            unsafe { abort(); }
        }
    }

    Edit: Or maybe a bug in Miri (cc @oli-obk)

    extern { fn abort() -> !; }
    
    static B: bool = std::f64::NAN > std::f64::NAN;
    
    fn main() {
        if B {
            unsafe { abort(); }
        }
    }
  6. kennytm commented on May 16, 2018

    @kennytm
    Member

    Bisection between c83fa5d...097efa9 gives #46882 as the regression PR, as expected.

  7. added
    A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)
    on May 16, 2018
  8. oli-obk commented on May 16, 2018

    @oli-obk
    Contributor

    I guess this logic, which I blindly copied, was never correct to begin with:

    https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/operator.rs#L188

    I think this is how old const eval did it, and we didn't want to regress that. cc @eddyb

  9. oli-obk commented on May 16, 2018

    @oli-obk
    Contributor

    40b118c#diff-580fb7d0b0ede57fb2ce24a5dd33910cL50

    Has the comment about existing bad behaviour.

  10. kennytm commented on May 16, 2018

    @kennytm
    Member

    @oli-obk The old logic used unwrap, meaning NaN > NaN would just ICE (?). I think we can safely replace them directly as PrimVal::from_bool(l > r) etc.

    Edit: 🤔 The unwrap is in 40b118c#diff-e847a6da94440f7bf6783206efe4275bL138

  11. kennytm commented on May 16, 2018

    @kennytm
    Member

    The incorrect behavior mentioned in #50811 (comment) was this:

    let x: [u8; (std::f64::NAN > std::f64::NAN) as usize] = [1];

    If we fix the float comparison behavior, this code will no longer compile. But I think this is clearly a bug fix and breaking code like this is acceptable.

  12. nikomatsakis commented on May 17, 2018

    @nikomatsakis
    Contributor

    Fixed in #50812

  13. added a commit that references this issue on May 21, 2018
  14. added a commit that references this issue on May 24, 2018
  15. added 4 commits that reference this issue on Jun 1, 2018
  16. added a commit that references this issue on May 20, 2026
  17. added a commit that references this issue on May 20, 2026
  18. added a commit that references this issue on May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)C-bugCategory: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-highHigh priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions