Skip to content

direct-0.1.0 beta regression #43665

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
est31 opened this issue Aug 5, 2017 · 3 comments
Closed

direct-0.1.0 beta regression #43665

est31 opened this issue Aug 5, 2017 · 3 comments
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@est31
Copy link
Member

est31 commented Aug 5, 2017

direct version 0.1.0 regressed from stable to beta (full log):

 error: use of unstable library feature 'ord_max_min' (see issue #25663)
    --> src/lib.rs:308:54
     |
 308 |                     (ordered_float::NotNaN::new(left.min(right.into_inner())).unwrap(), idx)
     |                                                      ^^^
 
 error[E0308]: mismatched types
    --> src/lib.rs:308:58
     |
 308 |                     (ordered_float::NotNaN::new(left.min(right.into_inner())).unwrap(), idx)
     |                                                          ^^^^^^^^^^^^^^^^^^ expected struct `ordered_float::NotNaN`, found f64
     |
     = note: expected type `ordered_float::NotNaN<f64>`
                found type `f64`

cc @Blei

@arielb1
Copy link
Contributor

arielb1 commented Aug 5, 2017

The new Ord::min shadows f64's min, which was accessed through the Deref impl at https://github.com/reem/rust-ordered-float/blob/b27bbd4/src/lib.rs#L225.

The new impl is actually more of what you want - it would allow the code to be simplified to left.min(right). Unfortunately, that won't work on stable. But you can still use cmp::min(left, right) instead of that complicated line (after a use std::cmp;).

@arielb1 arielb1 added regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Aug 5, 2017
@Blei
Copy link
Contributor

Blei commented Aug 7, 2017

Thank you! Fixed and published a new version.

@est31
Copy link
Member Author

est31 commented Aug 7, 2017

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants