I also added a unit test to ensure the short-circuiting works correctly, for all of the Applicative methods.

Consider this code:

case thisWillFail *> thisWillTakeALongTime of
  Failure _ -> print "It didn't work."
  Success _ -> print "It worked!"

Since we know that the whole expression will be a Failure after the LHS fails, we now emit Failure itself sooner, so that thisWillTakeALongTime doesn't need to be evaluated unless/until the argument to Failure is evaluated.

Sure, I do understand that, but I'm talking exactly about this PR. This feels like unchecked breaking change. @chshersh are you sure you would like to take such a risk? I'm personally using Validation in production, so I wouldn't hurry to get to the newer version of relude then.

Am I missing something? How is this a breaking change?

@vrom911 Agree with you here. I'm also using Validation in production and it works. We have a separate issue to implement tests for Validation. So let's not change the code yet if it's untested.

Actually, this is worse than just a performance problem; we're not a valid Applicative:

The other methods have the following default definitions, which may be overridden with equivalent specialized implementations:

• u *> v = (id <$ u) <*> v
• u <* v = liftA2 const u v

(emphasis mine) Ours aren't equivalent. If u = Failure ["foo"] and v = undefined, then the LHS of both of those evaluate to undefined, but the RHS of both evaluate to Failure undefined.

Now that #176 is fixed, is this ready to merge?

@josephcsible Well, since we have tests now, we actually can implement our own more optimised versions of these operators (like it how was done at first). Because now we have more guarantees that they will work. And we won't rely on the inliner too much as a pleasant consequence.

@chshersh Done.