From 473b6a0639aee631e8935fc2c55ab618d95326e7 Mon Sep 17 00:00:00 2001
From: Julian Ganz <neither@nut.email>
Date: Sat, 12 Jun 2021 13:12:53 +0200
Subject: [PATCH 1/2] impl: do recursive shrinking without recursive fn call

We try to shrink values recursively, i.e. when a shrunk value witnesses
a failure, we'd shrink that value further. Previously, this recursion
would be implemented via actual control flow recursion, i.e. a function
calling itself. Since the recursion could not be unrolled by the
compiler, this could result in stack overflows in some situations.

Albeit such an overflow would often hint at a faulty shrinker (e.g. a
shrinker yielding the original value), the stack overflow could also
occur in other situations.

This change switches from a recursive control flow to explicitly
swapping out the shrinking iterator during the iteration.
---
 src/tester.rs | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/tester.rs b/src/tester.rs
index f09326c..75f9cc1 100644
--- a/src/tester.rs
+++ b/src/tester.rs
@@ -369,7 +369,9 @@ impl<T: Testable,
             self_: fn($($name),*) -> T,
             a: ($($name,)*),
         ) -> Option<TestResult> {
-            for t in a.shrink() {
+            let mut r = Default::default();
+            let mut a = a.shrink();
+            while let Some(t) = a.next() {
                 let ($($name,)*) = t.clone();
                 let mut r_new = safe(move || {self_($($name),*)}).result(g);
                 if r_new.is_failure() {
@@ -378,16 +380,16 @@ impl<T: Testable,
                         r_new.arguments = Some(debug_reprs(&[$($name),*]));
                     }
 
-                    // The shrunk value *does* witness a failure, so keep
-                    // trying to shrink it.
-                    let shrunk = shrink_failure(g, self_, t);
+                    // The shrunk value *does* witness a failure, so remember
+                    // it for now
+                    r = Some(r_new);
 
-                    // If we couldn't witness a failure on any shrunk value,
-                    // then return the failure we already have.
-                    return Some(shrunk.unwrap_or(r_new))
+                    // ... and switch over to that value, i.e. try to shrink
+                    // it further.
+                    a = t.shrink()
                 }
             }
-            None
+            r
         }
 
         let self_ = *self;

From d930487433ee6d20fbb8e739fbf76b25f8342916 Mon Sep 17 00:00:00 2001
From: Julian Ganz <neither@nut.email>
Date: Sat, 12 Jun 2021 15:31:49 +0200
Subject: [PATCH 2/2] impl: move shrinking logic out of inline function

In the past, shrinking was implemented using recursion in the control
flow. `shrink_failure` would call itself. That function was introduced
originally in

        5b19e7c21e1afdcabb894668ad61be70bb023ef0

presumably in order to implement recursive shrinking. However, we
recently choose an approach which would not rely on recursive control
flow but on swapping out an iterator. Thus, the reason why
`shrink_failure` existed in the first place doesn't exist any more.
This change moves the logic in its original place, but also replaces the
`match` which enclosed the call to `shrink_failure` with an `if`.
---
 src/tester.rs | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/src/tester.rs b/src/tester.rs
index 75f9cc1..5f70f86 100644
--- a/src/tester.rs
+++ b/src/tester.rs
@@ -364,12 +364,12 @@ impl<T: Testable,
      $($name: Arbitrary + Debug),*> Testable for fn($($name),*) -> T {
     #[allow(non_snake_case)]
     fn result(&self, g: &mut Gen) -> TestResult {
-        fn shrink_failure<T: Testable, $($name: Arbitrary + Debug),*>(
-            g: &mut Gen,
-            self_: fn($($name),*) -> T,
-            a: ($($name,)*),
-        ) -> Option<TestResult> {
-            let mut r = Default::default();
+        let self_ = *self;
+        let a: ($($name,)*) = Arbitrary::arbitrary(g);
+        let ( $($name,)* ) = a.clone();
+        let mut r = safe(move || {self_($($name),*)}).result(g);
+
+        if r.is_failure() {
             let mut a = a.shrink();
             while let Some(t) = a.next() {
                 let ($($name,)*) = t.clone();
@@ -382,26 +382,16 @@ impl<T: Testable,
 
                     // The shrunk value *does* witness a failure, so remember
                     // it for now
-                    r = Some(r_new);
+                    r = r_new;
 
                     // ... and switch over to that value, i.e. try to shrink
                     // it further.
                     a = t.shrink()
                 }
             }
-            r
         }
 
-        let self_ = *self;
-        let a: ($($name,)*) = Arbitrary::arbitrary(g);
-        let ( $($name,)* ) = a.clone();
-        let r = safe(move || {self_($($name),*)}).result(g);
-        match r.status {
-            Pass|Discard => r,
-            Fail => {
-                shrink_failure(g, self_, a).unwrap_or(r)
-            }
-        }
+        r
     }
 }}}