Poor diagnostics due to `CompileTimeAssert` and weird `try_from` / `try_new`

[SV-97]

I just ran into some problems when trying to do something like

let x = 123_u32;
let y = u10::try_from(x);

This induced an error

error[E0080]: evaluation of `arbitrary_int::CompileTimeAssert::<32, 10>::SMALLER_OR_EQUAL` failed

from deep inside arbitrary-int's source without any source spans for the user code, which made finding the problem rather hard. I don't see why there's a compile time assert like that on an explicitly fallible conversion. Judging from the error message it seems like the try_from is internally using from which seems rather odd to me.

I also tried using try_new rather than try_from, however this would requires chaining multiple conversions when a single one should do: try to get u16 from u32 and then try to get u10 from u16. Since the error types between u10::try_new and u16::try_from don't match up this requires some rather ugly code (even when discarding the errors):

u16::try_from(x).ok().and_then(|x| u10::try_new(x).ok())

[danlehmann]

Hello,

The reason for the awkward from/tryfrom semantics is because of a Rust limitation: A type can not implement both From and TryFrom at the same time. Ideally, we'd want u5::try_from(u8) and u15::from(u8), but I was not able to specify something like that using trait bounds.

The const asserts are a workaround for this - if some future compiler version does support this, we can switch over without the risk of breaking client code.

[SV-97]

Ah that's a shame. What do you think about generalizing the input parameter of try_new to accept the larger types as well?

Just as an example of how that might look like I tested

fn try_new<T, E>(value: T) -> Result<Self, TryNewError>
where
    T: TryInto<Self::UnderlyingType, Error = E>,
    E: Into<TryNewError>;

with impls like

fn try_new<T,E>(value: T) -> Result<Self, TryNewError>
where
    T: TryInto<Self::UnderlyingType, Error = E>,
    E: Into<TryNewError>
{
    value.try_into().map_err(E::into).and_then(|value| if value <= Self::MAX.value {
        Ok(Self { value })
    } else {
        Err(TryNewError{})
    })

}

which requires two impls for the error types of the primitive type conversions

impl From<Infallible> for TryNewError {
    fn from(_: Infallible) -> Self {
        unreachable!() // Infallible is an empty type
    }
}

impl From<TryFromIntError> for TryNewError {
    fn from(_: TryFromIntError) -> Self {
        TryNewError
    }
}

and is a strict generalization of the current API if I'm not mistaken. (I didn't manage to get it working with the const_convert_and_const_trait_impl feature because const_convert seem to have been removed, but other than that all the tests still passed with these changes).

[danlehmann]

Hey, sorry for the late response! try_new being non-const is a step down, but maybe not a massive one, given how Result isn't exactly full of const functions either. So if someone wants a const new, they can probably just use regular new().

It would have to be a major version bump though - which would be a good idea anyway as there are a couple of other changes that require that as well.

[vxpm]

just ran into some trouble caused by this const assertion, and i think this really is not the way to solve the problem, since it has terrible debuggability:

• it does not show up in cargo check (it's a post-monomorphization error, so you need to build in order to trigger it)
• when it triggers, it gives you no context whatsoever
• it breaks the contract of the traits it's used in

i propose that the implementations should be specific to every type instead of generic over the bit length, probably using a macro to auto generate them. that is, UInt<u8 | u16 | u32 | u64 | u128, N> would manually implement From for types with bit length <= N and TryFrom for any type with a greater bit length.

the only downside i can see here is that this would generate a lot of implementations: a type with N bits will have 127 implementations, N of them From<Smaller> and 127 - N of them TryFrom<Bigger>. for 128 types, that's a total of 16256 trait implementations - but the only effect this has (as far as i see?) is maybe longer compilation times and documentation bloat.

edit: alright, this is not viable at all! did a simple test and compilation times get a real big hit from that many implementations. oh well.

[danlehmann]

Thanks for trying this out. Reducing the number of classes in other crates that implement similar types was the big motivation for arbitrary_int in the first place. As Rust's const generics get more powerful, things like this issue can hopefully be addressed in a cleaner way.