rust-num / num-traits

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

Support conversion of primitive integer types in `const` context #226

Closed U007D closed 2 years ago

U007D commented 3 years ago

Support primitive integer conversion via new const_conversion feature or when building with a version of the compiler where const_trait_impl has stabilized.

ctrlcctrlv commented 2 years ago

(Merged into https://github.com/MFEK/num-traits.rlib/commit/827172009369cb8f891aa9732363a92918ee6327 due to https://github.com/rust-num/num-traits/issues/189#issuecomment-1011828593. You likely don't care and the maintainer does plan to return, just explaining the merge message above.)

ctrlcctrlv commented 2 years ago

Oh, here's something you might indeed care about though @U007D—you probably want to add https://github.com/MFEK/num-traits.rlib/commit/54b6a41bdb15ed90a9a67c7e85e879c5ee24ee0d to your branch because otherwise your PR adds a warning.

petoknm commented 2 years ago

It would be nice to have const FromPrimitive as well, but I can't get it to work.

I tried to just add const to the FromPrimitive impl in the macro expansion. Any idea why it goes wrong? Here's the error for u32.

Compile error ``` error[E0015]: calls in constant functions are limited to constant functions, tuple structs and tuple variants --> num-traits.rlib-c0584f9c87376f82/8271720/src/cast.rs:625:17 | 563 | / macro_rules! impl_from_primitive { 564 | | ($T:ty, $to_ty:ident) => { 565 | | #[allow(deprecated)] 566 | | impl const FromPrimitive for $T { ... | 625 | | n.$to_ty() | | ^^^^^^^^^^ ... | 628 | | }; 629 | | } | |_- in this expansion of `impl_from_primitive!` ... 641 | impl_from_primitive!(u32, to_u32); | --------------------------------- in this macro invocation ```

As far as I can see, it should work, because the <u32 as ToPrimitive>::to_u32() is a const fn. Feature const_conversion is enabled.

cuviper commented 2 years ago

Hi -- I'm not inclined to merge any unstable features in the API, especially because I don't want the maintenance burden if that syntax changes at all. So I'm going to close this for now, but you're of course welcome to keep experimenting in a fork, and please feel free to reopen or file a new PR when the language has stabilized this.