proptest-rs / proptest

Hypothesis-like property testing for Rust
Apache License 2.0
1.63k stars 152 forks source link

Allow `BoxedStrategy` to have lifetime shorter than `'static` #399

Open poscat0x04 opened 7 months ago

poscat0x04 commented 7 months ago

Currently the BoxedStrategy implementation requires the boxed impl Strategy type to have 'static lifetime. This can be undesirable if the impl Strategy references some other stuff since all the referenced stuff will also be forced to have the lifetime 'static (this can easily happen since both prop_map and prop_flat_map requires the closure to be Fn which means they need to capture stuff by reference).

poscat0x04 commented 7 months ago

This should be pretty trivial to implement. Here's what I did in my code:

#[derive(Debug)]
pub struct BoxedStrat<'a, T>(
    Arc<dyn Strategy<Value = T, Tree = Box<dyn ValueTree<Value = T> + 'a>> + 'a>,
);

impl<'a, T: Debug> Strategy for BoxedStrat<'a, T> {
    type Tree = Box<dyn ValueTree<Value = T> + 'a>;

    type Value = T;

    fn new_tree(&self, runner: &mut TestRunner) -> NewTree<Self> {
        self.0.new_tree(runner)
    }
}

impl<'a, T: Debug> BoxedStrat<'a, T> {
    fn from_strat(strat: impl Strategy<Value = T> + 'a) -> Self {
        Self(Arc::new(BoxedStratWrapper(strat, PhantomData)))
    }
}

#[derive(Debug)]
struct BoxedStratWrapper<'a, T>(T, PhantomData<&'a T>);
impl<'a, T: Strategy> Strategy for BoxedStratWrapper<'a, T>
where
    T::Tree: 'a,
{
    type Tree = Box<dyn ValueTree<Value = T::Value> + 'a>;
    type Value = T::Value;

    fn new_tree(&self, runner: &mut TestRunner) -> NewTree<Self> {
        Ok(Box::new(self.0.new_tree(runner)?))
    }
}

unfortunately, this is not backward compatible.

cameron1024 commented 7 months ago

Hmm, this seems a little small to justify a 2.0, but I agree it's annoying. Would a new type like BoxedTempStrategy<'a, T> (name up for bikeshedding) work for your use case?

poscat0x04 commented 7 months ago

Sure