BurntSushi / quickcheck

Automated property based testing for Rust (with shrinking).
The Unlicense
2.4k stars 149 forks source link

Implement `Arbitrary` for `Box` #154

Closed bryal closed 7 years ago

bryal commented 7 years ago

Lots of boilerplate is required when implementing Arbitrary for recursive types. Having an impl of Arbitrary for boxes would be quite nice when working with such structures!

BurntSushi commented 7 years ago

Ah yeah this is great! I'm surprised we've made it this far without it. :-)

BurntSushi commented 7 years ago

OK, so I initially merged this and put out a 0.4.2 release of quickcheck. However, @bluss let me know that this is (unfortunately) a breaking change. It doesn't seem like it would be, but Box is special:

A #[fundamental] type Foo is one where implementing a blanket impl over Foo is a breaking change. As described, & and &mut are fundamental. This attribute would be applied to Box, making Box behave the same as & and &mut with respect to coherence.

I completely forgot about Box being #[fundamental].

This means we either need to revert this change or just make a new 0.5 release. I'm OK with a new 0.5 release.

BurntSushi commented 7 years ago

From @bluss: Here's an example%0A%20%20%20%20%7D%0A%7D%0A%0A&version=stable&backtrace=0) of code that would work on quickcheck 0.4.1 but would break with this patch applied.

BurntSushi commented 7 years ago

I've yanked quickcheck 0.4.2.

bluss commented 7 years ago

Yeah, I think that's for the best.

The rule summary is

The rules are what permit impl RemoteTrait for Box<LocalType> or impl<'a> RemoteTrait for &'a LocalType etc to compile, if the blanket impls don't exist in the remote crate.

bryal commented 7 years ago

Interesting, hadn't thought of that! I think a 0.5 release sounds good.