Closed kurnevsky closed 5 years ago
I think my previous opinion hasn't really changed, and a ~4x hit to compile times seemingly confirms it.
@BurntSushi I think it's essential to have this implementation to compete with proptest. If compilation time is so important we can have it behind a feature gate (added with the last commit).
I don't agree. We don't need to "compete" with anyone. If proptest is better for arrays, then use proptest.
I came across this PR because I wanted this feature and had to resort to using tuples and then manually converting them to arrays in the body of the function. (My use case for this is randomizing the shape of an n-dimensional array where n=8.)
I understand the issue with compile times, and I ran cargo clean; time cargo build
for a few cases on my machine (Core i5-2520M):
arrays
feature: 24.185sarrays
feature: 40.015sarrays
feature, but limited to arrays with length <= 8 instead of 32: 24.587sNote that these times include building the dependencies; I'm not sure how to measure the time to build quickcheck
by itself. I think it makes sense to include the time to build the dependencies anyway, though, because in practice users only need to build quickcheck
and its dependencies once; they don't need to rebuild quickcheck
each time they test their crate.
The additional build time to support arrays of length <= 8 is negligible. What I'd suggest is:
long_arrays
or something).Thoughts?
We've made it this far without Arbitrary
impls for arrays, we can wait a bit longer for const generics to land.
It increases compilation time from ~3.5s to 13.5s in release mode on my laptop. Also not sure how long shrinking can take for [T; 32]. What do you think?
Fixes #92 #187