wip

Cargo.toml

@@ -23,8 +23,26 @@ insta = { version = "1.34.0" }
 
 [profile.dev.package]
 insta.opt-level = 3
+
+# optimising these packages were found to contribute to property testing
+# execution time.
+#
+# `flamegraph` below is https://github.com/flamegraph-rs/flamegraph
+# $ perf record --call-graph=dwarf -F 99999 <unittest executable> prop_roundtrip_
+# $ flamegraph --no-inline --perfdata perf.data
 rand_chacha.opt-level = 3
+rand_chacha.debug = 1
 regex.opt-level = 3
+regex.debug = 1
 regex-automata.opt-level = 3
+regex-automata.debug = 1
 regex-syntax.opt-level = 3
+regex-syntax.debug = 1
 proptest.opt-level = 3
+proptest.debug = 1
+serde.opt-level = 3
+serde.debug = 1
+serde_json.opt-level = 3
+serde_json.debug = 1
+jsonschema.opt-level = 3
+jsonschema.debug = 1

hugr/Cargo.toml

@@ -22,7 +22,7 @@ bench = false
 path = "src/lib.rs"
 
 [features]
-default = []
+default = ["proptest"]
 extension_inference = []
 proptest = ["dep:proptest","dep:proptest-derive","dep:regex-syntax"]
 

hugr/src/hugr/ident.rs

@@ -7,8 +7,6 @@ use smol_str::SmolStr;
 use thiserror::Error;
 
 pub static PATH_COMPONENT_REGEX_STR: &str = r"[\w--\d]\w*";
-#[cfg(all(test, feature = "proptest"))]
-pub static PATH_COMPONENT_NICE_REGEX_STR: &str = r"[[:alpha:]][[[:alpha:]]0-9]*";
 lazy_static! {
     pub static ref PATH_REGEX: Regex =
         Regex::new(&format!(r"^{0}(\.{0})*$", PATH_COMPONENT_REGEX_STR)).unwrap();
@@ -26,8 +24,8 @@ lazy_static! {
     serde::Serialize,
     serde::Deserialize,
 )]
-/// A non-empty dot-separated list of valid identifiers
 
+/// A non-empty dot-separated list of valid identifiers
 pub struct IdentList(SmolStr);
 
 impl IdentList {
@@ -91,7 +89,6 @@ mod test {
             fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
                 use crate::proptest::any_ident_string;
                 use proptest::collection::vec;
-                // we shrink to more readable (i.e. :alpha:) names
                 vec(any_ident_string(), 1..2)
                     .prop_map(|vs| {
                         IdentList::new(

hugr/src/hugr/serialize/test.rs

@@ -177,7 +177,9 @@ pub fn check_hugr_roundtrip(hugr: &Hugr, check_schema: bool) -> Hugr {
     new_hugr
 }
 
-#[allow(unused)]
+// for now this is only used in property testing, so otherwise configured out to
+// avoid unused warnings.
+#[cfg(feature = "proptest")]
 fn check_testing_roundtrip(t: impl Into<TestingModel>) {
     let before = Versioned::new(t.into());
     let after_strict = ser_roundtrip_validate(&before, Some(&TESTING_SCHEMA_STRICT));
@@ -389,7 +391,7 @@ mod proptest {
         type Strategy = BoxedStrategy<Self>;
         fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
             (
-                (0..(std::u32::MAX / 2) as usize).prop_map(|x| portgraph::NodeIndex::new(x).into()),
+                (0..i32::MAX as usize).prop_map(|x| portgraph::NodeIndex::new(x).into()),
                 any::<Option<ExtensionSet>>(),
                 any::<OpType>(),
             )

hugr/src/lib.rs

@@ -135,11 +135,10 @@
 // Unstable check, may cause false positives.
 // https://github.com/rust-lang/rust-clippy/issues/5112
 #![warn(clippy::debug_assert_with_mut_call)]
-
 // proptest-derive generates many of these warnings.
 // https://github.com/rust-lang/rust/issues/120363
 // https://github.com/proptest-rs/proptest/issues/447
-#![cfg_attr(all(test,feature = "proptest"), allow(non_local_definitions))]
+#![cfg_attr(all(test, feature = "proptest"), allow(non_local_definitions))]
 
 pub mod algorithm;
 pub mod builder;

hugr/src/ops/constant.rs

@@ -641,7 +641,7 @@ mod test {
             fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
                 use proptest::collection::vec;
                 let signed_strat = (..=LOG_WIDTH_MAX).prop_flat_map(|log_width| {
-                    use std::i64;
+                    use i64;
                     let max_val = (2u64.pow(log_width as u32) / 2) as i64;
                     let min_val = -max_val - 1;
                     (min_val..=max_val).prop_map(move |v| {
@@ -684,11 +684,7 @@ mod test {
                 use ::proptest::collection::vec;
                 let leaf_strat = prop_oneof![
                     any::<ExtensionValue>().prop_map(|e| Self::Extension { e }),
-                    prop_oneof![
-                        // TODO we need an example of each legal root, in particular FuncDe{fn,cl}
-                        Just(crate::builder::test::simple_dfg_hugr()),
-                    ]
-                    .prop_map(|x| Value::function(x).unwrap())
+                    crate::proptest::any_hugr().prop_map(|x| Value::function(x).unwrap())
                 ];
                 leaf_strat
                     .prop_recursive(
@@ -701,7 +697,7 @@ mod test {
                                 (
                                     any::<usize>(),
                                     vec(element.clone(), 0..3),
-                                    any_with::<SumType>(1.into())
+                                    any_with::<SumType>(1.into()) // for speed: don't generate large sum types for now
                                 )
                                     .prop_map(
                                         |(tag, values, sum_type)| {

hugr/src/ops/module.rs

@@ -133,8 +133,13 @@ impl OpTrait for AliasDefn {
 
 /// A type alias declaration. Resolved at link time.
 #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
+#[cfg_attr(all(feature = "proptest", test), derive(proptest_derive::Arbitrary))]
 pub struct AliasDecl {
     /// Alias name
+    #[cfg_attr(
+        all(feature = "proptest", test),
+        proptest(strategy = "crate::proptest::any_nonempty_smolstr()")
+    )]
     pub name: SmolStr,
     /// Flag to signify type is classical
     pub bound: TypeBound,
@@ -168,24 +173,3 @@ impl OpTrait for AliasDecl {
         <Self as StaticTag>::TAG
     }
 }
-
-#[cfg(test)]
-mod test {
-    #[cfg(feature = "proptest")]
-    mod proptest {
-        use crate::types::TypeBound;
-        use proptest::prelude::*;
-
-        impl Arbitrary for super::super::AliasDecl {
-            type Parameters = ();
-            type Strategy = BoxedStrategy<Self>;
-            fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
-                use crate::proptest::any_ident_string;
-                let bound = any::<TypeBound>();
-                (any_ident_string(), bound)
-                    .prop_map(|(name, bound)| Self::new(name, bound))
-                    .boxed()
-            }
-        }
-    }
-}

hugr/src/proptest.rs

@@ -3,10 +3,34 @@ use ::proptest::prelude::*;
 use lazy_static::lazy_static;
 use smol_str::SmolStr;
 
-#[derive(Clone, Copy, Debug)]
-pub struct TypeDepth(usize);
+use crate::Hugr;
 
-impl TypeDepth {
+#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq)]
+/// The types [Type], [TypeEnum], [SumType], [FunctionType], [TypeArg],
+/// [TypeParam], as well as several others, form a mutually recursive heirarchy.
+///
+/// The proptest `Strategy::prop_recursive` is inadequate to generate values for
+/// these types.  Instead, ther Arbitrary instances take a `RecursionDepth` as
+/// their (or part of their) [Arbitrary::Parameters]. We then use that parameter
+/// to generate children of that value. Usually we forward it unchanged, but in
+/// crucial locations we instead forward the `descend` of it.
+///
+/// Consider the tree of values generated. Each node is labelled with a
+/// [RecursionDepth].
+///
+/// Consider a path between two nodes of the same type(e.g. two [Type]s, or two
+/// [FunctionType]s).  The path must be decreasing in [RecursionDepth] because
+/// each child's [RecursionDepth] is derived from it's parents.
+///
+/// We must maintain the invariant that the [RecursionDepth] of the start of the
+/// path is strictly greater than the [RecursionDepth] of the end of the path.
+///
+/// With this invariant in place we are guaranteed to terminate in producing a
+/// value, because there are only finitely many different types a node can take.
+pub struct RecursionDepth(usize);
+
+impl RecursionDepth {
+    const DEFAULT_RECURSION_DEPTH: usize = 4;
     pub fn descend(&self) -> Self {
         if self.leaf() {
             *self
@@ -18,34 +42,41 @@ impl TypeDepth {
     pub fn leaf(&self) -> bool {
         self.0 == 0
     }
+
+    pub fn new() -> Self {
+        Self(Self::DEFAULT_RECURSION_DEPTH)
+    }
 }
 
-impl Default for TypeDepth {
+impl Default for RecursionDepth {
     fn default() -> Self {
-        Self(3)
+        Self::new()
     }
 }
 
-impl From<usize> for TypeDepth {
-    fn from(s: usize) -> Self {
-        Self(s)
+impl<I: TryInto<usize>> From<I> for RecursionDepth
+where
+    <I as TryInto<usize>>::Error: std::fmt::Debug,
+{
+    fn from(s: I) -> Self {
+        Self(s.try_into().unwrap())
     }
 }
 
 lazy_static! {
     static ref ANY_IDENT_STRING: SBoxedStrategy<String> = {
         use proptest::string::string_regex;
         prop_oneof![
-            string_regex(r"[[:alpha:]]{1,3}").unwrap(),
-            string_regex(crate::hugr::ident::PATH_COMPONENT_NICE_REGEX_STR).unwrap(),
+            // we shrink to more readable (i.e. :alpha:) names
+            string_regex(r"[[:alpha:]]+").unwrap(),
             string_regex(crate::hugr::ident::PATH_COMPONENT_REGEX_STR).unwrap(),
         ].sboxed()
     };
 
     static ref ANY_NONEMPTY_STRING: SBoxedStrategy<String> = {
         use proptest::string::string_regex;
         prop_oneof![
-            string_regex(r"[[:alpha:]]{1,3}").unwrap(),
+            // we shrink to more readable (i.e. :alpha:) names
             string_regex(r"[[:alpha:]]+").unwrap(),
             string_regex(r".+").unwrap(),
         ].sboxed()
@@ -54,7 +85,7 @@ lazy_static! {
     static ref ANY_STRING: SBoxedStrategy<String> = {
         use proptest::string::string_regex;
         prop_oneof![
-            string_regex(r"[[:alpha:]]{0,3}").unwrap(),
+            // we shrink to more readable (i.e. :alpha:) names
             string_regex(r"[[:alpha:]]*").unwrap(),
             string_regex(r".*").unwrap(),
         ].sboxed()
@@ -67,11 +98,22 @@ lazy_static! {
             any::<bool>().prop_map_into(),
             any::<u64>().prop_map_into(),
             any::<i64>().prop_map_into(),
+            // floats don't round trip !?!
             // any::<f64>().prop_map_into(),
             Just(Value::Number(3.into())),
             any_string().prop_map_into(),
         ].sboxed()
     };
+
+    static ref ANY_HUGR: SBoxedStrategy<Hugr>= {
+        // TODO we need more examples
+        // This is currently used for Value::Function
+        // With more uses we may need variants that return more constrained
+        // HUGRs.
+        prop_oneof![
+            Just(crate::builder::test::simple_dfg_hugr()),
+        ].sboxed()
+    };
 }
 
 pub fn any_nonempty_string() -> SBoxedStrategy<String> {
@@ -110,3 +152,7 @@ pub fn any_serde_yaml_value() -> impl Strategy<Value = serde_yaml::Value> {
         )
         .boxed()
 }
+
+pub fn any_hugr() -> SBoxedStrategy<Hugr> {
+    ANY_HUGR.clone()
+}