search

isaacholt100 / bnum

Arbitrary, fixed size numeric types that extend the functionality of primitive numeric types in Rust.
https://crates.io/crates/bnum
Other
73 stars 10 forks source link

Derive arbitrary if fuzzing #20

Closed lrubasze closed 1 year ago

lrubasze commented 1 year ago

Introduced "fuzzing" feature to enable struct-aware fuzzing of the types introduced by bnum crate.

Unfortunately arbitrary works only for std at the moment.

More details on arbitrary: https://github.com/rust-fuzz/arbitrary https://docs.rs/arbitrary/latest/arbitrary/

lrubasze commented 1 year ago

Hi @isaacholt100, I thought that maybe you could find "fuzzing" feature useful. Hope my PR covers it properly. If not then I am open to amend it according to your expectations.

And CI failed - maybe you could advise what could be wrong with it?

isaacholt100 commented 1 year ago

Hi @lrubasze, thanks for writing this PR. I'm in the middle of exam season at the moment so unfortunately I won't have time to look at this for a bit, but once my exams are over I'll make sure I have a proper look. Looks like CI is failing because arithmetic traits such as Sub have const implementations but aren't marked as const. This is strange as I'm fairly sure these traits used to be const, so this is possibly a breaking change in the nightly compiler? I'll investigate further when I have time though.

lrubasze commented 1 year ago

Hi @isaacholt100, thanks for prompt reply. Waiting for your feedback then 😄 Good luck during your exams 🤞

isaacholt100 commented 1 year ago

Hi @lrubasze, exams are over now so I'll hopefully be able to merge this in a day or two. It seems as though the Rust team are redesigning const traits, so it looks like as of v1.71.0-nightly, a lot of standard library traits such as From, Add, etc. can't be const implemented, which is why the check is failing at the moment. What I'll do is write const method equivalents of these, to be implemented on the integer types themselves rather than the traits, then remove the const impls of the traits. I'll then merge this PR with my changes and publish a new version based off these updates.

isaacholt100 commented 1 year ago

Have merged these changes into the arbitrary branch which I will soon publish as a new version. Apologies, as the CI was failing it made it a bit awkward to merge this PR and as there was a small number of changes I decided it would be easier to just transfer these across to the most up to date version. The commit message I made for these changes mentions this issue to give you credit, hope that is ok!

isaacholt100 commented 1 year ago

Version 0.7.0 published!

lrubasze commented 1 year ago

Hi @isaacholt100, sorry for late reply - was on holiday. Many thanks proceeding with this. I have noticed you changed the feature name from fuzzing to arbitrary. Usually the flag that enables some features related to fuzzing is called fuzzing, I saw it in many libraries. But that is not a problem at all, arbitrary is just fine 😄

isaacholt100 commented 1 year ago

Hi @lrubasze, no worries, hope you had a good holiday! I chose the feature name to arbitrary as I've done the same for other features which just activate an external crate, but I wasn't aware of this convention, so thanks for letting me know!