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

Unconditionally bound with T: FloatCore. #138

Merged
merged 2 commits into from
Sep 3, 2023
Merged

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Sep 2, 2023

Previously, the std feature was non-additive; that is, it was possible for enabling the std feature to break code in another crate. Specifically, consider the following code:

fn problem<T>(inputs: &mut [OrderedFloat<T>])
where
    T: num_traits::float::FloatCore,
{
    inputs.sort();
}

This code will compile if the std feature is disabled, but then fail when it is enabled because Ord for OrderedFloat<T> requires that T: Float, rather than T: FloatCore.

In order to eliminate this compatibility hazard, instead always require FloatCore and, where necessary, also require Float.

This is a breaking change; generic code may need updated bounds.

Also, for reasons I do not entirely understand, #[derive(PartialEq)] wouldn't compile for the UniformNotNan and UniformOrdered distribution types, so I had to replace them with ones with explicit bounds. This may be a sign that something deeper is wrong, or it might be an inadequacy of the rustc trait solver...

Previously, the `std` feature was non-additive; that is, it was
possible for *enabling* the `std` feature to break code in another
crate. Specifically, consider the following code:

    fn problem<T>(inputs: &mut [OrderedFloat<T>])
    where
        T: num_traits::float::FloatCore,
    {
        inputs.sort();
    }

This code will compile if the `std` feature is disabled, but then fail
when it is enabled because `Ord for OrderedFloat<T>` requires that
`T: Float`, rather than `T: FloatCore`.

In order to eliminate this compatibility hazard, instead always
require `FloatCore` and, where necessary, also require `Float`.

**This is a breaking change;** generic code may need updated bounds.

Also, for reasons I do not entirely understand, `#[derive(PartialEq)]`
wouldn't compile for the `UniformNotNan` and `UniformOrdered`
distribution types, so I had to replace them with ones with explicit
bounds. This may be a sign that something deeper is wrong, or it might
be an inadequacy of the rustc trait solver...
Copy link
Collaborator

@mbrubeck mbrubeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I guess this will become ordered-float 4.0. I’ll wait a little while before releasing it, to figure out if there are any other breaking changes we want to bundle in at the same time.

@mbrubeck mbrubeck merged commit 28a2d1d into reem:master Sep 3, 2023
2 checks passed
@kpreid kpreid mentioned this pull request Sep 6, 2023
@kpreid kpreid deleted the stdfloat branch September 6, 2023 15:01
@mbrubeck
Copy link
Collaborator

ordered-float 4.0.0 has been released.

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 this pull request may close these issues.

None yet

2 participants