Prop.all is not stack-safe

[julienrf]

The current implementation of Prop.all is not stack-safe:

scalacheck/src/main/scala/org/scalacheck/Prop.scala

Lines 428 to 431 in 281d93c

	/** Combines properties into one, which is true if and only if all the
	* properties are true */
	def all(ps: Prop*): Prop =
	ps.foldLeft(proved)(_ && _)

The _ && _ expression creates a long chain of flatMap calls, which may blow the stack.

Note that the implementation changed in #531 (cc @non), and the previous implementation was stack-safe (it used the && operation on Prop.Result instead of Prop itself).

I believe we could write a stack-safe implementation of Prop#combine, which is used by Prop#&&.

[ashawley]

That change was merged and released in version 1.14.1. It's surprising it took so long for this to be noticed, but not a lot of people use Prop.all, so it's also not that surprising.

We have a trivial test of Prop.all in the test suite, but it wouldn't catch the stack overflow unless the min size setting for generators was increased. Don't try this at home since there are other tests other than Prop.all that similarly don't take well to large generator sizes, but this will produce the stack overflow for me:

> jvm/testOnly org.scalacheck.PropSpecification -- -x 100000

Again, don't try running the above test. It only works because I removed everything except the Prop.all test.

[ashawley]

I believe we could write a stack-safe implementation of Prop#combine, which is used by Prop#&&.

Yeah, I agree. It would be better to preserve the new versions Erik wrote of && and friends and also of Prop.all.

I believe the following would be the simplest solution,

   def combine(p: => Prop)(f: (Result, Result) => Result) =
    Prop(prms => f(this(prms), p(prms)))

However, I believe the seed should be different for the props.

I'd need to test it further, but hopefully @non will be able to weigh in.

[julienrf]

However, I believe the seed should be different for the props.

Then we should use flatMap. So, maybe we should make flatMap stack-safe (by using trampolining)?

[ashawley]

I suggested that it would be worth preserving the new versions of Prop.all and Prop.atLeastOne, but there's really no compelling reason given this defect. I'm confident that this was just an incidental improvement, and not a change made by @non for fixing #530. I'll open a PR reverting those changes.