Add crate that enables the full float APIs in #[no_std]

[robertbastian]

It provides an extension trait for f32 and f64 that replicates the whole std API, backed by libm. This allows us to remove explicit libm calls from our business logic, and use the std implementations on std targets via shadowing.

This PR uses core_float in icu_segmenter, I will also apply it in calendrical_calculations after #3985 is merged.

[sffc]

General issue: We should minimize the amount of math code in this crate. If we must add any, it should be well tested. One way to test it is to run it against the standard library implementation on a suitably diverse set of inputs.

Specific issue: Pretty sure the div/rem euclid impls here aren't right. The standard library impl Rem says:

The remainder has the same sign as the dividend and is computed as: x - (x / y).trunc() * y

But rem_euclid is a different thing. It is the remainder relative to the floor, not the trunc. You can't derive that by taking .abs().

[robertbastian]

All code here is copied from std, with calls to intrinsics replaced by libm. You're linking to Rem, which is a different operation to rem_euclid.

[sffc]

Ok r + rhs.abs() I think is correct. We care a lot about rem_euclid for calendrical calculations.

[robertbastian]

rhs.abs() is the more general form, we use rhs in our version because we require rhs > 0

[robertbastian]

this is the only code not copied from std::f64, libm doesn't have a powi so this is from u64::pow.

[Manishearth]

should this dep be cfg(target_os = none)?

[robertbastian]

Sure

[robertbastian]

actually this doesn't work, it's possible to build no_std on a target with an OS.

[sffc]

I'm not convinced that this PR is a positive change for these reasons:

1. Currently each component specifies and tests its own set of math operations relevant for that component. I think this status quo is good, because we only add these functions when they are needed, so all the code is covered and tested. With a mega trait¹ like this, most of this code is not going to be covered or used.
2. Related to the above point, this code is not covered by tests. We want high test coverage both to satisfy Coveralls but also because it helps ensure that we are consistent with std. Furthermore, there isn't anything specific to ensure we keep this in sync with rust std.
3. We are in the business of internationalization. I think we need to be conservative with non-i18n code landing in our repo. Existing utilities like Yoke, ZeroVec, ZeroFrom, TinyStr, and so forth are widely used by most all ICU4X components, and ones like FixedDecimal, ZeroTrie, and Writeable are definitely i18n-adjacent. core_maths is neither widely used (it is needed for only segmenter and calendrical calculations) nor i18n-adjacent (in both cases it is for implementing a non-i18n algorithm, ML and astronomy). Anywhere we need non-i18n utilities (like matrix math), we implement a narrow subset in private modules.

Footnotes

1. Also, mega traits are slightly dangerous because if you ever take them as a trait object, you get all the code for all their functions with no code slicing available. This could be mitigated by making the mega trait not be object safe. ↩

[robertbastian]

Furthermore, there isn't anything specific to ensure we keep this in sync with rust std.

We don't have to keep it in sync, Rust std is not going to change behaviour overnight.

We have other utility crates that are not used by every component, like crlify, deduplicating_array, even litemap is only used by two components.

This trait is not object safe because it takes self by value.

[sffc]

We have other utility crates that are not used by every component, like crlify, deduplicating_array, even litemap is only used by two components.

crlify is less than 50 lines of code, so the risks and maintenance burden are much less.

deduplicating_array is a bit larger, yes. I think the reason it ended up there was that we needed it as public API because it is in our data struct definition and couldn't figure out a better place to put it. But, we don't need core_maths in our public API.

litemap had previously been a utility used widely, until ZeroMap came along and replaced most of our usages. We have a path toward unifying our map solution in #3128.

[sffc]

We don't have to keep it in sync, Rust std is not going to change behaviour overnight.

The most likely situation I'm thinking of is Rust std adding new mathematical operations. Not super common but it does seem to happen from time to time (f64::clamp, f64::total_cmp, f64::from_ne_bytes and friends, f64::to_int_unchecked, f64::EPSILON, f64::copysign, f64::div_euclid, and f64::rem_euclid are all examples of APIs added after 1.0), and I don't want the ICU4X team to take on the responsibility to check for that.

[robertbastian]

I think you might be overestimating the maintenance burden. I'm happy to own this crate.

[sffc]

I would be happy with any of the following options (not in any particular order):

1. Write tests for this crate that compare your function output with std function output. I definitely do not want to land any more math code with spotty test coverage given Fix FixedDecimal rounding bug; add more tests #3644. If you add tests and @Manishearth is aligned on the crate concept, we can land this.
2. Make a new repository like robertbastian/coremaths (going through the releasing process) and then we take it in as a third-party dependency.
3. Close this PR and optionally open a new one polishing up the private component-specific traits.

(however, there are other 1.3 blocking issues that are all higher priority than landing this PR, in my opinion)

[robertbastian]

I will not write tests for std against std, that just does not make sense. It's also not possible to exhaustively test f32xf32 or f64 methods.

This is very simple code, which can easily reviewed by looking at the mathematical definition that each method represents. It is not comparable to the complicated business logic in #3644.

Make a new repository like robertbastian/coremaths (going through the releasing process) and then we take it in as a third-party dependency.

I'm happy to do that but what exactly does that achieve? I'll still want to depend on this from ICU4X.