num icon indicating copy to clipboard operation
num copied to clipboard

Signed::is_positive and Signed::is_negative implementation/documentation is inconsistent

Open Zeenobit opened this issue 3 years ago • 0 comments

Based on the documentation:

/// Returns true if the number is positive and false if the number is zero or negative.
fn is_positive(&self) -> bool;

/// Returns true if the number is negative and false if the number is zero or positive.
fn is_negative(&self) -> bool;

However, for signed integer types, we have the following, which matches the documentation:

#[inline]
fn is_positive(&self) -> bool { *self > 0 }

#[inline]
fn is_negative(&self) -> bool { *self < 0 }

But then for floating point types we have:

/// Returns `true` if the number is positive, including `+0.0` and `INFINITY`
#[inline]
fn is_positive(&self) -> bool {
    FloatCore::is_sign_positive(*self)
}

/// Returns `true` if the number is negative, including `-0.0` and `NEG_INFINITY`
#[inline]
fn is_negative(&self) -> bool {
    FloatCore::is_sign_negative(*self)
}

And so with a simple test like this:

dbg!(Signed::is_positive(&0i32));
dbg!(Signed::is_positive(&0.0f32));

We get:

Signed::is_positive(&0i32) = false
Signed::is_positive(&0.0f32) = true

I'm not sure what the best approach here (i.e. should 0 count as positive or not for floating point).

But the behavior should be consistent between floats and ints, and match the documentation on the Signed trait. Otherwise it leads to really subtle and dangerous arithmetic errors.

Zeenobit avatar May 26 '22 02:05 Zeenobit