Robbepop / apint

Arbitrary precision integers library.
Other
26 stars 4 forks source link

Use TryFrom traits for utility types such as BitWidth #41

Open Robbepop opened 5 years ago

AaronKutch commented 4 years ago

I have been trying to rework the way BitWidth is constructed, and have almost done it on my tryfrom_construction branch (note: I forgot to make a new branch from master after the previous PR got squashed, so I will need to redo this branch before it can be PRed). The new way things are done is through these:

pub struct BitWidth(NonZeroUsize);

impl From<NonZeroUsize> for BitWidth {
    /// Creates a `BitWidth` from the given `NonZeroUsize`.
    fn from(width: NonZeroUsize) -> Self {
        BitWidth(width)
    }
}

impl TryFrom<usize> for BitWidth {
    type Error = Error; // This is the crates `Error` type

    /// Creates a `BitWidth` from the given `usize`.
    ///
    /// # Errors
    ///
    /// - If the given `width` is equal to zero.
    fn try_from(width: usize) -> Result<Self> {
        match NonZeroUsize::new(width) {
            Some(bitwidth) => Ok(BitWidth(bitwidth)),
            None => Err(Error::invalid_zero_bitwidth()),
        }
    }
}

pub(crate) fn bw<S>(width: S) -> BitWidth
where
    S: TryInto<BitWidth>,
{
    // For this case, we erase the regular error unwrapping message by converting
    // the `Result` to an `Option`, and displaying a different message.
    width.try_into().ok().expect(
        "Tried to construct an invalid BitWidth of 0 using the `apint::bw` function",
    )
}

To start off, I make BitWidth a wrapper around NonZeroUsize in case of compiler optimizations, but I do not expose this outside the crate. The bw function (I used to want to make it a macro, but macros are difficult deal with when exporting and importing) acts as a general purpose way of quickly making constant bitwidths. I think I like it enough that we should make it public right away. It replaces the BitWidth::wX() functions will help greatly with testing and using the crate in practice. The TryFrom impl causes the most changes. For example, verify_valid_bitwidth is removed in favor of BitWidth::try_from()? and signatures are changed from W: Into<BitWidth> into W: TryInto<BitWidth, Error = Error>. This enables directly inputing constants into the functions which is a massive ergonomics increase for users of the crate.

I encountered a huge problem, however. When I change the signatures:

pub fn truncate<W>(&mut self, target_width: W) -> Result<()>
    where
-        W: Into<BitWidth>,
+        W: TryInto<BitWidth, Error = Error>, // `Error = Error` is to set the return type to the crates `Error` type, or else it fails to compile
    {
        let actual_width = self.width();
-        let target_width = target_width.into();
+        let target_width = target_width.try_into()?;

rustc no longer accepts BitWidths directly as arguments. The workaround is to call .to_usize():

            // `excess_width` assigned to earlier
            self.most_significant_digit_mut()
-                 .truncate_to(excess_width)
+                .truncate_to(excess_width.to_usize())
                .expect(
                    "Excess bits are guaranteed to be within the bounds for valid \
                     truncation of a single `Digit`.",

The reason is that there is a blanket impl for TryFrom<T> for <T> that has a type Error = ! (the never type, or on stable Infallible. This will conflict with the requirement that Error = Error

error[E0271]: type mismatch resolving `<bitwidth::BitWidth as std::convert::TryInto<bitwidth::BitWidth>>::Error == errors::Error`                                                                
   --> src\apint\casting.rs:182:18
    |
182 |                 .truncate_to(excess_width)
    |                  ^^^^^^^^^^^ expected enum `std::convert::Infallible`, found struct `errors::Error`
AaronKutch commented 4 years ago

I have finally fixed the problem. My only concern is that the generic function will be compiled into many functions, one for each kind of input. In the worst case, I can just have the generic function call an inner function, and the compiler will certainly inline the outer part into caller code. I think we already do that for the into_ functions through try_forward_bin_mut_impl. Does this look good?

AaronKutch commented 4 years ago

Have you read this?