Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve fuzz.rs #47

Open
AaronKutch opened this issue Aug 19, 2019 · 0 comments
Open

Improve fuzz.rs #47

AaronKutch opened this issue Aug 19, 2019 · 0 comments

Comments

@AaronKutch
Copy link
Contributor

AaronKutch commented Aug 19, 2019

There are two problems I have right now:

  • invariants such as unused bits being unset is not checked very well. Important functions are tested pretty well as a side effect of testing their overflowing counterparts, but I would like to have a concise way to test invariants at every unwrap in fuzz.rs.
  • unwraps use up a lot of space. Replacing them all with ? makes fuzz.rs much cleaner, but the problem is that I do not know what line errors come from. There is a backtrace function planned for Error, but there may be several months yet before it arrives in nightly.

This is not urgent, but I would prefer to fix the invariant problem at least before a new release of apint.

I am thinking about making a free function that checks all invariants and unwraps the results at the same time. As an example, it would change this:

            assert_eq!(
                lhs.clone()
                    .into_zero_extend(BitWidth::new(size * 2).unwrap()).unwrap()
                    .into_wrapping_mul(
                        &rhs.clone()
                            .into_zero_extend(BitWidth::new(size * 2).unwrap()).unwrap()
                    ).unwrap()
                    .into_zero_resize(rand_width),
                lhs.clone()
                    .into_wrapping_mul(&rhs).unwrap()
                    .into_zero_resize(rand_width)
            );

into this:

            assert_eq!(
               f(f(lhs.clone()
                    .into_zero_extend(BitWidth::new(size * 2)?))
                    .into_wrapping_mul(
                        &f(rhs.clone()
                            .into_zero_extend(BitWidth::new(size * 2)?))
                    ))
                    .into_zero_resize(rand_width),
                f(lhs.clone()
                    .into_wrapping_mul(&rhs))
                    .into_zero_resize(rand_width)
            );

Not much of an improvement, but at least invariants are checked. The only way I can think of which improves it more is doing evil stuff with feature flags, such as changing the Result type in the crate into something else which implements the Try trait. Maybe we could have a the feature flag change have #[cfg(feature_flag)] on the mod fuzz; (removing fuzz.rs entirely during any other compilation, so that users of the crate do not experience weird stuff when running with test).

The less weird stuff we are doing with feature flags though, the better. fuzz.rs is not intended to be very friendly to the eyes anyway, so I believe the free function approach is best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant