Make it harder to build a vector mutator without inherent complexity

loiclec · loiclec · commit bd5d499b9c4d · 2022-07-30T17:27:08.000+02:00
diff --git a/fuzzcheck/src/builder.rs b/fuzzcheck/src/builder.rs
@@ -337,7 +337,7 @@ where
         }
         fn fuzz_test() {
             fuzzcheck::fuzz_test(foo)
-                .mutator(VecMutator::new(u8::default_mutator(), 2 ..= 10, true))
+                .mutator(VecMutator::new(u8::default_mutator(), 2 ..= 10))
                 // ..
                 # ;
         }
diff --git a/fuzzcheck/src/mutators/fixed_len_vector.rs b/fuzzcheck/src/mutators/fixed_len_vector.rs
@@ -22,7 +22,7 @@ where
     max_complexity: Cell<f64>,
     search_space_complexity: Cell<f64>,
     has_inherent_complexity: bool,
-    inherent_complexity: f64,
+    inherent_complexity: Cell<f64>,
     _phantom: PhantomData<T>,
 }
 impl<T, M> FixedLenVecMutator<T, M>
@@ -32,7 +32,7 @@ where
 {
     #[no_coverage]
     pub fn new_with_repeated_mutator(mutator: M, len: usize) -> Self {
-        Self::new(vec![mutator; len], true)
+        Self::new(vec![mutator; len])
     }
 }
 
@@ -41,8 +41,34 @@ where
     T: Clone + 'static,
     M: Mutator<T>,
 {
+    /// Note: only use this function if you really know what you are doing!
+    ///
+    /// Create a `FixedLenVecMutator` using the given submutators.
+    /// The complexity of the generated vectors will be only the sum of the
+    /// complexities of their elements.
+    ///
+    /// This is not the default behaviour.
+    /// Normally, a vector such as `[1u8, 2u8]` would have a complexity of `17`:
+    /// `2 * 8` for the first two integers, and `+ 1` for the inherent
+    /// complexity of the vector itself. If the vector contains elements with a
+    /// minimum complexity of 0.0, then its length would also influence its
+    /// complexity. For example, `[(), ()]` would have a complexity of `3.0`:
+    /// `1` for the vector and  `+ 2` for the length.
+    ///
+    /// By using this function to create the `FixedLenVecMutator`, we get rid
+    /// of the "inherent" part of the vector complexity. For example, the vector
+    /// `[1u8, 2u8]` will have a complexity of 16.0 and the vector `[[], []]`
+    /// will have a complexity of 0.0.
+    ///
+    /// Note that *all mutators in a fuzz test must agree on the complexity of
+    /// a value*. For example, if you are mutating a 2-tuple of vectors:
+    /// `(Vec<u8>, Vec<u8>)` using two `FixedLenVecMutator`, then both must
+    /// agree on whether to include the inherent complexity of the vectors or
+    /// not. That is, given the value `([1, 2], [1, 2])`, it is an error to
+    /// evaluate the complexity of the first vector as `16.0` and the complexity
+    /// of the second vector as `17.0`.
     #[no_coverage]
-    pub fn new(mutators: Vec<M>, inherent_complexity: bool) -> Self {
+    pub fn new_without_inherent_complexity(mutators: Vec<M>) -> Self {
         assert!(!mutators.is_empty());
 
         Self {
@@ -52,8 +78,25 @@ where
             min_complexity: Cell::new(std::f64::INFINITY),
             max_complexity: Cell::default(),
             search_space_complexity: Cell::default(),
-            has_inherent_complexity: inherent_complexity,
-            inherent_complexity: 0.,
+            has_inherent_complexity: false,
+            inherent_complexity: Cell::default(),
+            _phantom: PhantomData,
+        }
+    }
+
+    #[no_coverage]
+    pub fn new(mutators: Vec<M>) -> Self {
+        assert!(!mutators.is_empty());
+
+        Self {
+            rng: Rng::default(),
+            mutators,
+            initialized: Cell::new(false),
+            min_complexity: Cell::new(std::f64::INFINITY),
+            max_complexity: Cell::default(),
+            search_space_complexity: Cell::default(),
+            has_inherent_complexity: true,
+            inherent_complexity: Cell::default(),
             _phantom: PhantomData,
         }
     }
@@ -213,6 +256,7 @@ impl<T: Clone + 'static, M: Mutator<T>> Mutator<Vec<T>> for FixedLenVecMutator<T
             |cplx, m| cplx + m.global_search_space_complexity(),
         );
         self.initialized.set(true);
+        self.inherent_complexity.set(inherent_complexity);
         self.min_complexity.set(min_complexity);
         self.max_complexity.set(max_complexity);
         self.search_space_complexity.set(search_space_complexity);
@@ -304,7 +348,7 @@ impl<T: Clone + 'static, M: Mutator<T>> Mutator<Vec<T>> for FixedLenVecMutator<T
     #[doc(hidden)]
     #[no_coverage]
     fn complexity(&self, _value: &Vec<T>, cache: &Self::Cache) -> f64 {
-        cache.sum_cplx + self.inherent_complexity
+        cache.sum_cplx + self.inherent_complexity.get()
     }
 
     #[doc(hidden)]
@@ -321,7 +365,7 @@ impl<T: Clone + 'static, M: Mutator<T>> Mutator<Vec<T>> for FixedLenVecMutator<T
     fn random_arbitrary(&self, max_cplx: f64) -> (Vec<T>, f64) {
         let target_cplx = crate::mutators::gen_f64(&self.rng, 1.0..max_cplx);
         let (v, sum_cplx) = self.new_input_with_complexity(target_cplx);
-        (v, sum_cplx + self.inherent_complexity)
+        (v, sum_cplx + self.inherent_complexity.get())
     }
 
     #[doc(hidden)]
@@ -347,11 +391,11 @@ impl<T: Clone + 'static, M: Mutator<T>> Mutator<Vec<T>> for FixedLenVecMutator<T
             let step = &mut step.crossover_steps[choice];
             let old_el_cplx = self.mutators[choice].complexity(&value[choice], &cache.inner[choice]);
             let current_cplx = self.complexity(value, cache);
-            let max_el_cplx = current_cplx - old_el_cplx - self.inherent_complexity;
+            let max_el_cplx = current_cplx - old_el_cplx - self.inherent_complexity.get();
             if let Some((el, new_el_cplx)) = step.get_next_subvalue(subvalue_provider, max_el_cplx) && self.mutators[choice].is_valid(el) {
                 let mut el = el.clone();
                 std::mem::swap(&mut value[choice], &mut el);
-                let cplx = cache.sum_cplx - old_el_cplx + new_el_cplx + self.inherent_complexity;
+                let cplx = cache.sum_cplx - old_el_cplx + new_el_cplx + self.inherent_complexity.get();
                 let token = UnmutateVecToken::ReplaceElement(choice, el);
                 return Some((token, cplx));
             }
@@ -364,7 +408,7 @@ impl<T: Clone + 'static, M: Mutator<T>> Mutator<Vec<T>> for FixedLenVecMutator<T
             let idcs = &idcs[..count];
             Some(self.mutate_elements(value, cache, idcs, current_cplx, max_cplx))
         } else {
-            let spare_cplx = max_cplx - current_cplx - self.inherent_complexity;
+            let spare_cplx = max_cplx - current_cplx - self.inherent_complexity.get();
             let idx = step.element_step % value.len();
             step.element_step += 1;
             self.mutate_element(value, cache, step, subvalue_provider, idx, current_cplx, spare_cplx)
diff --git a/fuzzcheck/src/mutators/grammar/mutators.rs b/fuzzcheck/src/mutators/grammar/mutators.rs
@@ -18,6 +18,27 @@ use crate::mutators::tuples::Tuple1Mutator;
 use crate::mutators::vector::VecMutator;
 use crate::Mutator;
 
+// NOTE: the complexity of the vectors in the AST is the complexity of their
+// elements and nothing else. That is, we don't take their inherent complexity
+// into account. This is because we only really care about the complexity of the
+// string representation of the AST. Adding complexity for each sequence would
+// only add noise. It would not translate to actual added complexity from the
+// user's perspective.
+// For example:
+//
+// The grammar:
+//      [a-z]?[a-z]?[a-z]?
+// producing the value:
+//      ae
+// vs.
+// The grammar:
+//      [a-z]{,3}
+// producing the same value:
+//      ae
+//
+// The AST of the first value contains four vectors wherease the AST of the
+// second value contains only 1. But the two values are equally complex from
+// the user's point of view.
 make_single_variant_mutator! {
     pub enum AST {
         Token(char),
@@ -244,7 +265,9 @@ impl ASTMutator {
     fn recur(m: RecurToMutator<ASTMutator>) -> Self {
         Self {
             inner: Box::new(Either::Right(Either::Left(ASTSingleVariant::Sequence(
-                Tuple1Mutator::new(Either::Left(FixedLenVecMutator::new(vec![m], false))),
+                Tuple1Mutator::new(Either::Left(FixedLenVecMutator::new_without_inherent_complexity(vec![
+                    m,
+                ]))),
             )))),
         }
     }
@@ -283,13 +306,14 @@ impl ASTMutator {
                     let m = Self::from_grammar_rec(g.clone(), others);
                     ms.push(m);
                 }
-                Self::sequence(Either::Left(FixedLenVecMutator::new(ms, false)))
+                Self::sequence(Either::Left(FixedLenVecMutator::new_without_inherent_complexity(ms)))
+            }
+            Grammar::Repetition(g, range) => {
+                Self::sequence(Either::Right(VecMutator::new_without_inherent_complexity(
+                    Self::from_grammar_rec(g.clone(), others),
+                    range.start..=range.end - 1,
+                )))
             }
-            Grammar::Repetition(g, range) => Self::sequence(Either::Right(VecMutator::new(
-                Self::from_grammar_rec(g.clone(), others),
-                range.start..=range.end - 1,
-                false,
-            ))),
             Grammar::Recurse(g) => {
                 if let Some(m) = others.get(&g.as_ptr()) {
                     Self::recur(RecurToMutator::from(m))
diff --git a/fuzzcheck/src/mutators/mod.rs b/fuzzcheck/src/mutators/mod.rs
@@ -318,7 +318,7 @@ pub(crate) fn gen_f64(rng: &fastrand::Rng, range: Range<f64>) -> f64 {
 #[must_use]
 #[no_coverage]
 fn size_to_cplxity(size: usize) -> f64 {
-    (usize::BITS - (size.saturating_sub(1)).leading_zeros()) as f64
+    (usize::BITS - size.leading_zeros()) as f64
 }
 
 #[cfg(test)]
@@ -330,12 +330,12 @@ mod test {
     #[no_coverage]
     fn test_size_to_cplxity() {
         assert_eq!(0.0, size_to_cplxity(0));
-        assert_eq!(0.0, size_to_cplxity(1));
-        assert_eq!(1.0, size_to_cplxity(2));
+        assert_eq!(1.0, size_to_cplxity(1));
+        assert_eq!(2.0, size_to_cplxity(2));
         assert_eq!(2.0, size_to_cplxity(3));
-        assert_eq!(2.0, size_to_cplxity(4));
+        assert_eq!(3.0, size_to_cplxity(4));
         assert_eq!(3.0, size_to_cplxity(5));
-        assert_eq!(3.0, size_to_cplxity(8));
+        assert_eq!(4.0, size_to_cplxity(8));
         assert_eq!(5.0, size_to_cplxity(31));
     }
 }
diff --git a/fuzzcheck/src/mutators/vector/mod.rs b/fuzzcheck/src/mutators/vector/mod.rs
@@ -28,7 +28,7 @@ where
     type Mutator = VecMutator<T, T::Mutator>;
     #[no_coverage]
     fn default_mutator() -> Self::Mutator {
-        VecMutator::new(T::default_mutator(), 0..=usize::MAX, true)
+        VecMutator::new(T::default_mutator(), 0..=usize::MAX)
     }
 }
 
@@ -81,13 +81,25 @@ where
     M: Mutator<T>,
 {
     #[no_coverage]
-    pub fn new(m: M, len_range: RangeInclusive<usize>, inherent_complexity: bool) -> Self {
+    pub fn new_without_inherent_complexity(m: M, len_range: RangeInclusive<usize>) -> Self {
         Self {
             m,
             len_range,
             rng: fastrand::Rng::new(),
             mutations: VectorMutation::default(),
-            inherent_complexity,
+            inherent_complexity: false,
+            _phantom: PhantomData,
+        }
+    }
+
+    #[no_coverage]
+    pub fn new(m: M, len_range: RangeInclusive<usize>) -> Self {
+        Self {
+            m,
+            len_range,
+            rng: fastrand::Rng::new(),
+            mutations: VectorMutation::default(),
+            inherent_complexity: true,
             _phantom: PhantomData,
         }
     }
diff --git a/fuzzcheck/src/mutators/vector/only_choose_length.rs b/fuzzcheck/src/mutators/vector/only_choose_length.rs
@@ -7,7 +7,7 @@
 //! use fuzzcheck::{Mutator, DefaultMutator};
 //! use fuzzcheck::mutators::vector::VecMutator;
 //!
-//! let m /* : impl Mutator<Vec<()>> */ = VecMutator::new(<()>::default_mutator(), 2..=5, true);
+//! let m /* : impl Mutator<Vec<()>> */ = VecMutator::new(<()>::default_mutator(), 2..=5);
 //! ```
 //! Then the values that `m` can produce are only:
 //! ```txt
diff --git a/fuzzcheck/tests/derived_mutually_recursive_structs.rs b/fuzzcheck/tests/derived_mutually_recursive_structs.rs
@@ -45,7 +45,7 @@ make_mutator! {
                 OptionMutator<MutuallyRecursiveA, AMutator<VecMutator<MutuallyRecursiveB, RecurToMutator<BMutator>>>>
              = {
                 OptionMutator::new(AMutator::new(
-                    VecMutator::new(self_.into(), 0..=usize::MAX, true),
+                    VecMutator::new(self_.into(), 0..=usize::MAX),
                     <Vec<u64>>::default_mutator(),
                 ))
             })]
diff --git a/fuzzcheck/tests/derived_recursive_struct.rs b/fuzzcheck/tests/derived_recursive_struct.rs
@@ -56,8 +56,7 @@ make_mutator! {
                             self_.into()
                         )
                     ),
-                    0..=usize::MAX,
-                    true
+                    0..=usize::MAX
                 )
             }
         )]
@@ -79,7 +78,7 @@ make_mutator! {
     struct SampleStruct2 {
         #[field_mutator(
             VecMutator<Box<SampleStruct2>, BoxMutator<RecurToMutator<SampleStruct2Mutator>>> = {
-                VecMutator::new(BoxMutator::new(self_.into()), 0..=10, true)
+                VecMutator::new(BoxMutator::new(self_.into()), 0..=10)
             }
         )]
         w: Vec<Box<SampleStruct2>>,
diff --git a/fuzzcheck/tests/vector.rs b/fuzzcheck/tests/vector.rs
@@ -2,19 +2,7 @@ use fuzzcheck::mutators::integer::U8Mutator;
 use fuzzcheck::mutators::vector::VecMutator;
 #[test]
 fn test_vector_mutator() {
-    // let m = VecMutator::new(U8Mutator::default(), 0..=10);
-    // fuzzcheck_mutators::testing_utilities::test_mutator(m, 100.0, 100.0, false, 500, 500);
-    // let m = VecMutator::new(U8Mutator::default(), 0..=10);
-    // fuzzcheck_mutators::testing_utilities::test_mutator(m, 20000.0, 20000.0, false, 500, 500);
-    // let m = VecMutator::new(U8Mutator::default(), 10..=20);
-    // fuzzcheck_mutators::testing_utilities::test_mutator(m, 10000.0, 10000.0, false, 500, 500);
-    // // todo: test with an unlimited range
-
-    let m = VecMutator::new(
-        VecMutator::new(U8Mutator::default(), 0..=usize::MAX, false),
-        0..=usize::MAX,
-        false,
-    );
+    let m = VecMutator::new(VecMutator::new(U8Mutator::default(), 0..=usize::MAX), 0..=usize::MAX);
     fuzzcheck::mutators::testing_utilities::test_mutator(m, 500.0, 500.0, false, true, 100, 150);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -337,7 +337,7 @@ where`
`337`	`337`	`}`
`338`	`338`	`fn fuzz_test() {`
`339`	`339`	`fuzzcheck::fuzz_test(foo)`
`340`		`- .mutator(VecMutator::new(u8::default_mutator(), 2 ..= 10, true))`
	`340`	`+ .mutator(VecMutator::new(u8::default_mutator(), 2 ..= 10))`
`341`	`341`	`// ..`
`342`	`342`	`# ;`
`343`	`343`	`}`
Original file line number	Diff line number	Diff line change
`@@ -56,8 +56,7 @@ make_mutator! {`
`56`	`56`	`self_.into()`
`57`	`57`	`)`
`58`	`58`	`),`
`59`		`- 0..=usize::MAX,`
`60`		`- true`
	`59`	`+ 0..=usize::MAX`
`61`	`60`	`)`
`62`	`61`	`}`
`63`	`62`	`)]`
`@@ -79,7 +78,7 @@ make_mutator! {`
`79`	`78`	`struct SampleStruct2 {`
`80`	`79`	`#[field_mutator(`
`81`	`80`	`VecMutator<Box<SampleStruct2>, BoxMutator<RecurToMutator<SampleStruct2Mutator>>> = {`
`82`		`- VecMutator::new(BoxMutator::new(self_.into()), 0..=10, true)`
	`81`	`+ VecMutator::new(BoxMutator::new(self_.into()), 0..=10)`
`83`	`82`	`}`
`84`	`83`	`)]`
`85`	`84`	`w: Vec<Box<SampleStruct2>>,`