Skip to content

Float::abs_sub isn't working properly #22105

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

Closed
mdinger opened this issue Feb 9, 2015 · 3 comments
Closed

Float::abs_sub isn't working properly #22105

mdinger opened this issue Feb 9, 2015 · 3 comments

Comments

@mdinger
Copy link
Contributor

mdinger commented Feb 9, 2015

I'm not sure what Float::abs_sub() is supposed to do but it's not working correctly.

#![feature(std_misc)]

fn main() {
    use std::num::Float;

    // Rules:    
    // - If `self < other`: `0`
    // - Otherwise `self - other`

    assert_eq!(3.0f64.abs_sub(5.0), 0.0); // okay
    assert_eq!(3.0f64.abs_sub(-5.0), 8.0);// okay

    assert_eq!(3.0f64.abs_sub(2.0), 1.0); // okay
    assert_eq!(3.0f64.abs_sub(-2.0), 5.0);// okay

    assert_eq!(-3.0f64.abs_sub(2.0), -1.0); // Wrong. What should this do?
    assert_eq!(-3.0f64.abs_sub(-2.0), -5.0);// Wrong. What should this do?

    assert_eq!(-1.0f64.abs_sub(2.0), 0.0);  // okay
    assert_eq!(-1.0f64.abs_sub(-2.0), -3.0);// Wrong. What should this do?

    assert_eq!(0.0f64.abs_sub(1.0), 0.0);   // okay
    assert_eq!(0.0f64.abs_sub(-1.0), 1.0);  // okay
}
@huonw
Copy link
Member

huonw commented Feb 9, 2015

These call the C function fdim (and variants) directly, which is described by its man page as

double fdim(double x, double y);

These functions return the positive difference, max(x-y,0), between their arguments.

so this behaviour seems rather confusing, and may be a bug in libm??

@huonw huonw added the A-libs label Feb 9, 2015
@huonw
Copy link
Member

huonw commented Feb 9, 2015

Oh, no, it's just an operator precedence thing: -3.0f64.abs_sub(-2.0) is parsed as -(3.0f64.abs_sub(-2.0)), not (-3.0f64).abs_sub(-2.0) (which is presumably the intention?).

Adding the parens makes it behave as expected:

#![feature(std_misc)]

fn main() {
    use std::num::Float;

    // Rules:    
    // - If `self < other`: `0`
    // - Otherwise `self - other`

    assert_eq!((-3.0f64).abs_sub(2.0), 0.0);
    assert_eq!((-3.0f64).abs_sub(-2.0), 0.0);
    assert_eq!((-1.0f64).abs_sub(-2.0), 1.0);
}

Closing as not-a-bug, but thanks for checking things so carefully!

@huonw huonw closed this as completed Feb 9, 2015
@mdinger
Copy link
Contributor Author

mdinger commented Feb 9, 2015

Hmm...that's a downside to using method syntax here. I can see that tripping people up often but don't see a way around it except recommending save to variables before calling the methods.

@huonw thanks!

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

No branches or pull requests

2 participants