From 940251b87f9d73087e2f51411fff9eba84a7108e Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Sat, 7 Jan 2023 15:23:32 +0300 Subject: refactor(tvix/value): use proptest strategies from imbl crate Instead of going through Vec/BTreeMap for generating our internal types, use the proptest strategies from imbl. The one thing I couldn't figure out in the previous implementation is where the ranges/sizes of generated collections came from. The strategies in proptest use different types (Range, with an unknown default value, and SizeRange with 0..100). I've opted to specify 0..100 directly, but we can probably make it configurable. Change-Id: I749bc4c703fe424099240cab822b1642e5216361 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7791 Autosubmit: tazjin Tested-by: BuildkiteCI Reviewed-by: flokli --- tvix/eval/src/value/arbitrary.rs | 46 ++++++++++++++++++++++++++++++++-------- tvix/eval/src/value/attrs.rs | 34 ++--------------------------- tvix/eval/src/value/list.rs | 23 -------------------- tvix/eval/src/value/mod.rs | 6 ------ 4 files changed, 39 insertions(+), 70 deletions(-) (limited to 'tvix/eval/src') diff --git a/tvix/eval/src/value/arbitrary.rs b/tvix/eval/src/value/arbitrary.rs index cd7629cfb923..f0bdc06fd51d 100644 --- a/tvix/eval/src/value/arbitrary.rs +++ b/tvix/eval/src/value/arbitrary.rs @@ -1,9 +1,10 @@ //! Support for configurable generation of arbitrary nix values +use imbl::proptest::{ord_map, vector}; use proptest::{prelude::*, strategy::BoxedStrategy}; use std::ffi::OsString; -use super::{NixAttrs, NixList, NixString, Value}; +use super::{attrs::AttrsRep, NixAttrs, NixList, NixString, Value}; #[derive(Clone)] pub enum Parameters { @@ -25,6 +26,39 @@ impl Default for Parameters { } } +impl Arbitrary for NixAttrs { + type Parameters = Parameters; // as Arbitrary>::Parameters; + type Strategy = BoxedStrategy; + + fn arbitrary_with(args: Self::Parameters) -> Self::Strategy { + prop_oneof![ + // Empty attrs representation + Just(Self(AttrsRep::Empty)), + // KV representation (name/value pairs) + ( + any_with::(args.clone()), + any_with::(args.clone()) + ) + .prop_map(|(name, value)| Self(AttrsRep::KV { name, value })), + // Map representation + ord_map(NixString::arbitrary(), Value::arbitrary_with(args), 0..100) + .prop_map(|map| Self(AttrsRep::Im(map))) + ] + .boxed() + } +} + +impl Arbitrary for NixList { + type Parameters = ::Parameters; + type Strategy = BoxedStrategy; + + fn arbitrary_with(args: Self::Parameters) -> Self::Strategy { + vector(::arbitrary_with(args), 0..100) + .prop_map(NixList::from) + .boxed() + } +} + impl Arbitrary for Value { type Parameters = Parameters; type Strategy = BoxedStrategy; @@ -65,14 +99,8 @@ fn leaf_value() -> impl Strategy { fn non_internal_value() -> impl Strategy { leaf_value().prop_recursive(3, 5, 5, |inner| { prop_oneof![ - any_with::(( - Default::default(), - Default::default(), - Parameters::Strategy(inner.clone()) - )) - .prop_map(Value::attrs), - any_with::((Default::default(), Parameters::Strategy(inner))) - .prop_map(Value::List) + NixAttrs::arbitrary_with(Parameters::Strategy(inner.clone())).prop_map(Value::attrs), + any_with::(Parameters::Strategy(inner)).prop_map(Value::List) ] }) } diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index 6515fb515a5d..10d071720219 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -24,7 +24,7 @@ use super::Value; mod tests; #[derive(Clone, Debug, Deserialize)] -enum AttrsRep { +pub(super) enum AttrsRep { Empty, Im(OrdMap), @@ -92,7 +92,7 @@ impl AttrsRep { #[repr(transparent)] #[derive(Clone, Debug, Default)] -pub struct NixAttrs(AttrsRep); +pub struct NixAttrs(pub(super) AttrsRep); impl FromIterator<(K, V)> for NixAttrs where @@ -192,36 +192,6 @@ impl<'de> Deserialize<'de> for NixAttrs { } } -#[cfg(feature = "arbitrary")] -mod arbitrary { - use super::*; - use std::collections::BTreeMap; - - use proptest::prelude::*; - use proptest::prop_oneof; - use proptest::strategy::{BoxedStrategy, Just, Strategy}; - - impl Arbitrary for NixAttrs { - type Parameters = as Arbitrary>::Parameters; - - type Strategy = BoxedStrategy; - - fn arbitrary_with(args: Self::Parameters) -> Self::Strategy { - prop_oneof![ - Just(Self(AttrsRep::Empty)), - ( - any_with::(args.2.clone()), - any_with::(args.2.clone()) - ) - .prop_map(|(name, value)| Self(AttrsRep::KV { name, value })), - any_with::>(args) - .prop_map(|map| Self::from_iter(map.into_iter())) - ] - .boxed() - } - } -} - impl NixAttrs { pub fn empty() -> Self { Self(AttrsRep::Empty) diff --git a/tvix/eval/src/value/list.rs b/tvix/eval/src/value/list.rs index 6d830b7283d0..5d1daf7c9c0b 100644 --- a/tvix/eval/src/value/list.rs +++ b/tvix/eval/src/value/list.rs @@ -35,29 +35,6 @@ impl From> for NixList { } } -#[cfg(feature = "arbitrary")] -mod arbitrary { - use proptest::{ - prelude::{any_with, Arbitrary}, - strategy::{BoxedStrategy, Strategy}, - }; - - use super::*; - - impl Arbitrary for NixList { - // TODO(tazjin): im seems to implement arbitrary instances, - // but I couldn't figure out how to enable them. - type Parameters = as Arbitrary>::Parameters; - type Strategy = BoxedStrategy; - - fn arbitrary_with(args: Self::Parameters) -> Self::Strategy { - any_with::>(args) - .prop_map(NixList::from_vec) - .boxed() - } - } -} - impl NixList { pub fn len(&self) -> usize { self.0.len() diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index af8627bf9b76..9ab23043c523 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -556,12 +556,6 @@ impl From for Value { } } -impl From> for Value { - fn from(val: Vec) -> Self { - Self::List(NixList::from_vec(val)) - } -} - fn type_error(expected: &'static str, actual: &Value) -> ErrorKind { ErrorKind::TypeError { expected, -- cgit 1.4.1