search

rust-num / num-traits

Numeric traits for generic mathematics in Rust
Apache License 2.0
732 stars 135 forks source link

Associated constants for One and Zero #54

Closed kvark closed 5 years ago

kvark commented 6 years ago

Any reason we can't have ONE and ZERO implemented as constants rather than methods? Given that num_traits is a low level crate, it effectively forbids any higher level constructs with associated constants atm.

kvark commented 6 years ago

Unless there are objections, I'm giving this a try (affecting way more traits than I expected, but still).

cuviper commented 6 years ago

It would be a breaking change to both users and implementors, which we strongly avoid, and associated constants require a newer rustc than our conservative 1.8. Also, some types which implement these traits can't have constant values at all, like BigInt.

kvark commented 6 years ago

@cuviper thanks for the answer!

It would be a breaking change to both users and implementors, which we strongly avoid,

The way I see it is adding the associated consts first, using them internally where possible, marking the old methods with #[deprecated] and then removing the old ones in 0.3. So I don't think this is an issue.

associated constants require a newer rustc than our conservative 1.8

I'm curious about this. Why 1.8 in particular? What are the guidelines/schedule for updating this base requirement?

some types which implement these traits can't have constant values at all, like BigInt

This is truly unfortunate, and it's a real blocker.

kvark commented 6 years ago

Following up on the last one - there does appear to be some light at the end of the tunnel, see https://github.com/rust-num/num-bigint/issues/36

cuviper commented 6 years ago

The way I see it is adding the associated consts first

Even that is a breaking change for third-party implementations of the traits. The only non-breaking way to expand a trait is with a default implementation of new items, which we can't do here.

I'm curious about this. Why 1.8 in particular? What are the guidelines/schedule for updating this base requirement?

It used to be 1.0, then other factors forced us to raise it to 1.8 -- rust-num/num#257. I treat raising the minimum rustc as a breaking change, but the community has mixed opinions on this -- rust-lang-nursery/api-guidelines#123. I don't have a schedule, but I just keep it conservative, since num-traits is a common dependency, often even in other crates' public API. Increasing num-traits' minimum rustc will force that decision on all of its dependents too.

Following up on the last one - there does appear to be some light at the end of the tunnel, see rust-num/num-bigint#36

Maybe, but this is just one example. We don't control all possible implementations of these traits.

cuviper commented 6 years ago

Of course, nothing stops you from implementing your own Zero and One traits with different trade-offs. The ubiquity of num-traits is a benefit to its use, but also constrains its ability to change.

kvark commented 6 years ago

Even that is a breaking change for third-party implementations of the traits.

Indeed.

Increasing num-traits' minimum rustc will force that decision on all of its dependents too.

ok, thanks for the links!

Maybe, but this is just one example. We don't control all possible implementations of these traits.

While true, you do control the goals of this crate's traits. There is a value in restricting the functionality a bit at the exchange of having nice associated constants use (which makes a big part of this crate's API much more elegant). It's hard to move forward if you have "all possible implementations" in scope. Even rustc has utilities to check all the crates.io contents for breaking changes.

For making this bold move, I'd think all we need is to ask the clients (are there any communication channels for this project other than the GH issues?) for whether or not this is going to be a showstopper for them.

vks commented 6 years ago

Maybe, but this is just one example. We don't control all possible implementations of these traits.

Other crates are for example ramp and rust-gmp. I think they can't easily make their bigints constant and would probably rather drop using One, Zero, Num etc.

For making this bold move, I'd think all we need is to ask the clients

I would rather recommend to implement your own trait. You can then impl<T: MyNum> num_traits::Num for T etc.

vks commented 6 years ago

BTW, this comment is outdated:

// FIXME (#5527): This should be an associated constant

This issue number is outdated and it is not so clear whether it should be an associated constant, if bignums are supposed to be able to implement it.

cuviper commented 6 years ago

Yeah, that FIXME is an old issue from when this was still in rust-lang/rust. We should remove it.

vks commented 6 years ago

Yeah, that FIXME is an old issue from when this was still in rust-lang/rust. We should remove it.

See #62.

cuviper commented 6 years ago

If you want a way forward here, I suggest something new like trait OneConst: One with a compiler-probed cfg guard, like we do for cfg(has_i128).

cuviper commented 5 years ago

As discussed in #91, the better long term solution is to use impl const in https://github.com/rust-lang/rfcs/pull/2632. Folks can define their own traits for OneConst etc. as a stop-gap measure, but I don't think we want to be stuck with that API in num-traits.