Implement CoreFloat trait #32
Conversation
This is a subset of the `Float` trait, but works with `no_std`. Some code was simplified by using `CoreFloat`.
src/float.rs
Outdated
| pub trait CoreFloat: Num + Neg<Output = Self> + PartialOrd + Copy { | ||
| /// Returns positive infinity. | ||
| #[inline] | ||
| fn infinity() -> Self; |
There was a problem hiding this comment.
#[inline] has no effect without a function body -- see rust-lang/rust#47475. That should be moved to the actual implementations.
src/lib.rs
Outdated
| @@ -26,7 +26,7 @@ use core::fmt; | |||
| pub use bounds::Bounded; | |||
| #[cfg(feature = "std")] | |||
| pub use float::Float; | |||
| pub use float::FloatConst; | |||
| pub use float::{CoreFloat, FloatConst}; | |||
| // pub use real::Real; // NOTE: Don't do this, it breaks `use num_traits::*;`. | |||
There was a problem hiding this comment.
I think we may have the same concern as Real did here. If someone does use num_traits::*, then the common methods between Float and CoreFloat may be ambiguous. Would you mind if we didn't re-export this in the root?
There was a problem hiding this comment.
I'm not sure supporting wildcard imports is worthwhile. If you want to make sure not to break them, you can't add any new functions or types. However, in this case I guess it is easy enough to avoid that problem.
There was a problem hiding this comment.
I'm not sure either. I guess a wildcard making us self-conflicting is a worse than having a conflict with some unrelated crate though.
This avoids breaking `use num_traits::*`.
|
I think I addressed all your comments. |
|
Looks great, thanks! bors r+ |
Build succeeded |
|
@vks I just noticed that this doesn't require |
|
Yes, I did not need it. Is there a reason to require it?
…On Thu, Feb 8, 2018, 00:44 Josh Stone ***@***.***> wrote:
@vks <https://github.com/vks> I just noticed that this doesn't require
NumCast like Float does. Was that intentional?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#32 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACCtDRlamQQl55Ki6otN-CVVtmMJeUvks5tSjVqgaJpZM4R3nbo>
.
|
|
It's a benefit for those who use the trait too. It was there long before
Float itself needed it.
I think I'm going to add a few more methods from Float that are feasible
with no_std, before it's published and becomes a breaking change. I can add
NumCast as I do that...
|
This is a subset of the
Floattrait, but works withno_std.Some code was simplified by using
CoreFloat.