rust-num / num-traits

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

Improve `bounds.rs` #248

Open DoubleHyphen opened 2 years ago

DoubleHyphen commented 2 years ago

1) Implemented all FIXMEs: → Trait now contains associated constants rather than non-const functions. → LowerBounded and UpperBounded are now supertraits of Bounded, not subtraits. 2) Expanded implementations to also cover NonZero data-types. 3) Made the code DRYer, with regards to the implementation of bounded_impl!. 4) Added a couple tests.

DoubleHyphen commented 2 years ago

To clarify, as the code is currently written all tests pass. But I think something's amiss.

I just tried a toy program that used the new Bounded trait. So, I tried using the MAX and MIN consts within a function that takes a Bounded parametre, only to realise that the compiler considered <T as LowerBounded>::MIN to be something completely different than <T as Bounded>::MIN, and kept asking for clarification. This is of course unreasonably non-ergonomic, so I deleted the MAX and MIN implementations in the Bounded trait itself… and the test suite lost its mind.

The closest I've come to making some semblance of sense out of this is that, apparently, there are several macros in the test suite (eg assert_eq_from) that use Bounded as a trait object. Apparently, when Bounded has separate MIN and MAX members it's all fine, but as soon as they're removed, all of a sudden it's unacceptable to use it as a trait object because… wait for it… it has associated consts?

I ultimately managed to get it to work by 1) adding use num_traits::bounds::{LowerBounded, UpperBounded}; to the beginning and 2) changing the trait objects to <_ as LowerBounded>::MIN etc. However, I can't say definitively if that fixes the actual issue, because there's probably a delicate dance happening here between visibility modifiers. As such, I'd like to politely ask for the guidance of a more senior person here.

cuviper commented 2 years ago

This is of course unreasonably non-ergonomic, so I deleted the MAX and MIN implementations in the Bounded trait itself…

Agreed so far...

and the test suite lost its mind.

The closest I've come to making some semblance of sense out of this is that, apparently, there are several macros in the test suite (eg assert_eq_from) that use Bounded as a trait object.

This is bizarre. I would not expect Bounded::MAX to be a trait object at all, just a partially qualified path, on its way to being inferred as a fully qualified <Something as Bounded>::MAX. It's even Sized!

Even in the same file, test wrapping_bounded complains that it can't find MIN/MAX. I don't know why it's not looking into the supertraits there.

SunnyWar commented 1 year ago

I want this so bad.