Skip to content

NotNan<f64> + f64 should return f64 instead of being an inexplicit potential panic #166

@Ten0

Description

@Ten0

When dealing with code that has NotNan<f64> values interact with inputs that are not yet checked to be NotNan, it's easy to forget that a particular value is not yet checked to be NotNan, and directly add it to a NotNan value, resulting in potential panics if the not-validated value was in fact NaN.

e.g.

let val: NotNan<f64> = struct1.field1 + struct2.field2

It's easy for a dev here to not notice that field2 (user-provided) wasn't checked to be NotNan yet, so this may cause panic.
This goes against the general philosophy of the language of forcing the dev to be explicit about error-case handling, to help them to not forget to handle everything appropriately (here, the correct thing to do would be to pre-validate field2 to be NotNan and give back an error to the user if it is, not panic, and the NotNan<f64> + f64 impl probably shouldn't be used).

If instead this would return f64, it wouldn't compile, avoiding such issues. This more natural mapping would also easily express that "from this point forward it's ok that I lose the NotNan guarantee", and allow writings such as:

let mut val: NotNan<f64> = ...;
let f: f64 = ...;
val = NotNan::new(val + f).ok_or_else(|| ...)?;

instead of having to write:

val = NotNan::new(val.into_inner() + f).ok_or_else(|| ...)?;

(It however does seem fine that operations on two NotNan may panic if they would generate a NaN, as such cases are much less likely to occur if the two operands are NotNan and the operation makes sense.)

This would have to be released not until the next major version, as it obviously would be a breaking change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions