'unsafeEvaluateCek' is overly lazy

[effectfully]

Area

[x] Plutus Foundation Related to the GHC plugin, Haskell-to-Plutus compiler, on-chain code
[] Plutus Application Framework Related to the Plutus application backend (PAB), emulator, Plutus libraries
[] Marlowe Related to Marlowe
[] Other Any other topic (Playgrounds, etc.)

So we have this issue with unsafeEvaluateCek not forcing the final result of a builtin application when the result of unsafeEvaluateCek is forced. I thought it might be some strictness problem within the definition of the CEK machine, however after inspecting and reinspecting all of its internals, I wasn't able to spot any problem, everything there looks exactly like it's supposed to. Then I looked into runCekM and nothing there seemed suspicious as well. So I started inserting bangs everywhere until I made the CEK machine so stupidly strict that it started forcing the result of a builtin application before doing budgeting for it and yet it still wasn't strict enough somehow. At this point I was convinced that the problem is not in the definition of the CEK machine, but rather how it was invoked. And indeed, unsafeEvaluateCek is defined like this:

unsafeEvaluateCek emitTime params = first unsafeExtractEvaluationResult . evaluateCek emitTime params

and this seemingly innocuous definition is our culprit. first uses an irrefutable match (via bimap) for (,) (i.e. matches lazily on the spine of the tuple), which causes all subsequent forcing to WHNF not to force anything. Compare

>>> (error "uh-oh" :: (Int, Char)) `seq` True
*** Exception: uh-oh

with

>>> first succ (error "uh-oh" :: (Int, Char)) `seq` True
True

So in our case forcing the result of unsafeEvaluateCek to WHNF does not do anything AT ALL, no evaluation happens whatsoever.

Solution? Never use bimap, first and second over (,): if laziness is required, use something with a more suggestive name, if it's not, bimap will do the wrong thing and so you should use the strict version of the function.

And yeah, in case you're curious, the Functor instance for (,) a matches strictly on the spine of the tuple, just for some extra confusion.

I absolutely hate the reason why the Bifunctor instance for (,) was implemented differently.

[kwxm]

Solution? Never use bimap, first and second over (,)

So what should we do in unsafeEvaluateCek (and other places)? Do we really want the use of first in there for some reason, or should we do something else to make sure that it doesn't bite us unexpectedly?

[effectfully]

So what should we do in unsafeEvaluateCek (and other places)?

Drop bimap, first and second and use stuff like

first' :: (a -> c) -> (a, b) -> (c, b)
first' f (x, y) = (f x, y)

I did check that that makes tests pass.

[michaelpj]

Fixed

[kwxm]

Where's the PR that fixed this? I'm sure I saw a GitHub notification about this going past a while ago, but I can't find the PR now.

[kwxm]

Ah, I think it must have been #3877. I was misled by the fact that it says it's about tests.