[Arrow 2.0] Effect without suspending shift

[nomisRev]

This is proposal is second proposal alongside #2782 after some discussions with @raulraja based on some other experimental / explatory implementations.

Design choices

Removal of suspend from shift (Why CancellationException?)

Both EagerEffectScope and EffectScope implement shift using a sub type of CancellationException since Arrow 1.x.x (before it was using ControlThrowable). The reason is to maintain correct semantics with other Kotlin Coroutine code, and to respect semantics of language constructs.

Effect

Effect requires shift to rely on CancellationException so that the we have proper short-circuit semantics when mixed with Kotlin Coroutines, such that following bracket laws are respected. See kotlin.coroutines.cancellation.

effect<String, Int> {
  bracketCase(
    acquire = { 1 },
    use = { i: Int ->
      i shouldBe 1
      shift<Int>("error")
    },
    releaseCase = { _, exitCase -> println(exitCase) } // ExitCase.Cancelled
  )
}.toEither() // Either.Left("error")

EagerEffect

To not break semantics of language constructs we use CancellationException in EagerEffect. Otherwise, without relying on CancellationException finally would never be called, and promise.await() would hang forever in the example below. Reference to an old impl/prototype here.

val promise = CompletableDeferred<Int>()
eagerEffect {
  try {
    shift<Int>(s)
  } finally {
    require(promise.complete(i))
  }
}.fold(::identity) { fail("Should never come here") } shouldBe s
promise.await() shouldBe i

Due to both implementations of EffectScope and EagerEffectScope throw CancellationException (in Arrow 1.x.x series) the shift function does not require suspend perse. So we can unify it under a single Shift, and we do not longer require @RestrictSuspension.

This changes the typealias of Effect, and EagerEffect to only differ in suspend, and means that we can now write truly polymorphic code when we're inside the context of Shift, since we can require suspend only when we rely on its capabilities. (I'm using extension functions below to replace/emulate the not yet released context receivers.)

public typealias Effect<R, A> = suspend Shift<R>.() -> A
public typealias EagerEffect<R, A> = Shift<R>.() -> A

fun Shift<String>.error(): Int =
  shift("error")

suspend fun Shift<String>.parallel(): Int =
  parZip({ error() }, { error() }) { x, y -> x + y }

Additional information:

• CancellationException vs ControlThrowable
• Edge cases, and Structured Concurrency

Inline fold

Since we now have a single Shift, we can now also implement a single fold which is inline so it allows for suspend to pass through! Below is the definition of our base implementation, and we can derive all implementations for Effect, EagerEffect, Either, etc from it.

public inline fun <R, A, B> fold(
  @BuilderInference program: Shift<R>.() -> A,
  error: (error: Throwable) -> B,
  recover: (shifted: R) -> B,
  transform: (value: A) -> B,
): B

What is interesting here is that fold is inline, so depending on the caller it will allow suspend or not.
This however means we can now also implement our either DSL as inline, which means we no longer need to differentiate between suspend and eager.

public inline fun <E, A> either(@BuilderInference block: Shift<E>.() -> A): Either<E, A>

This allows us to remove an additional complexity pain-point from Arrow. Since there is quite some confusion as to why we need eager and suspend operator fun invoke. This new fold implementation removes that limitation completely, and automatically works correctly depending on the callers context.

Catch / Attempt

As an added benefit we can now implement catch for inline for Either, alongside catch for Effect and EagerEffect. Because the catch implementation for Either is inline it still allows you to use suspend if desired.

public inline fun <E2, E, A> Either<E, A>.catch(block: Shift<E2>.(E) -> A): Either<E2, A> = TODO()

val error: Either<String, Int> = Either.Left("error")

val example2: Either<Nothing, Int> = error.catch { it.length }

suspend fun example(): Either<List<Char>, Int> =
  error.catch { str ->
    delay(100)
    shift(str.toList())
  }

Conclusion

Because we removed suspend from shift we are able to:

1. Unify EagerShift and Shift into a single type
2. inline fold to unify internal implementation, and remove all internal complexity of low-level coroutines
3. inline monadic builders, supporting both regular and suspending code seamlessly
4. Implement catch for suspending, and eager data types.

Are there any downsides of removing suspend from shift ??
=> Non suspending function throwing (CancellationException)
=> ... ?

[raulraja]

Thanks, @nomisRev I think this is a significant improvement in both runtime and declarations. 👍 ❤️

[JavierSegoviaCordoba]

Really great job 👏

[nomisRev]

This PR takes precedence over #2782 due to some new findings. I converted it back into a DRAFT because it needs some documentation before it can be merged.

All code is complete, and all tests are present. So the code can be reviewed.

I want to challenge the EffectScope<E> to Shift<E> renaming. It was something that was discussed offline, but I want to open it up here. Shift is not very descriptive about what happens. Ideally some1 should be able to grasp what is happening when looking at this code:

context(Shift<Error>)
suspend fun example(): Int = shift(Error)

Naming that is more explicit about errors can be more descriptive:

context(Raise<Error>)
suspend fun example(): Int = raise(Error)

or

context(CanFail<Error>)
suspend fun example(): Int = fail(Error)

It's a bit imprecise because RaiseError/CanFail can be used also for short-circuiting but not error paths perse.
99% of the use-cases will use it for errors... Similar to how MonadError might be used in Scala/Haskell for it's short-circuit behaviour in non-failure settings.

Open to all suggestions, or alternative names. I didn't suggest ShortCircuit because I felt it was non-descriptive like Shift.

[sksamuel]

RaiseError is more clear to the non-FP user than Shift.

[i-walker]

Since this was raised a couple of times, we could either favour failure-settings and keep only Raise or have Raise as an type-alias since Shift does encompass both scenarios failure-settings or just utilising short-circuit behaviour with non-failure settings.
If the type-alias isn't an option, maybe too verbose or documentation wise.
I'd prefer Shift over Raise for the best case.
We can also hold a pool in Github just like we did for bind?
wdyt cc @raulraja , @nomisRev @serras @arrow-kt/maintainers

[nomisRev]

@i-walker we've started a small poll on KotlinLang, but we can easily hold other polls and accumulate the data.

I think we've been able to gather some good feedback already, https://kotlinlang.slack.com/archives/C5UPMM0A0/p1661170633105679

[raulraja]

Looking great and getting even simpler 👏 @nomisRev!

[raulraja]

👏 nice work with catch everyone!, this is slick

[raulraja]

These impls are so much nicer and simpler now 👏

[raulraja]

These may be an exception to what I said above, but it's also not a value class since it takes more than one arg.

[nomisRev]

Sam issue, it can be implemented for StateShift<State, R> if we have such a type. (Local prototype, shared with you on Slack). Can share gist here for everyone that is interested, otherwise I'll raise it as a follow-up PR.

[i-walker]

Can you send in a gist link, please? 😄

[raulraja]

Should these functions be inline?

[nomisRev]

Does the compiler take that into account if there is no lambda to inline?

[raulraja]

I believe so based on the decompiled bytecode:

inline suspend fun foo(): Int = 1

suspend fun main() {
    println(foo())
}

decompiles inlining

byte var2 = 1;
System.out.println(var2);

in the code below.

public final class KPreludeKt {
   @Nullable
   public static final Object foo(@NotNull Continuation $completion) {
      int $i$f$foo = false;
      return Boxing.boxInt(1);
   }

   private static final Object foo$$forInline(Continuation $completion) {
      int $i$f$foo = false;
      return 1;
   }

   @Nullable
   public static final Object main(@NotNull Continuation $completion) {
      int $i$f$foo = false;
      byte var2 = 1;
      System.out.println(var2);
      return Unit.INSTANCE;
   }

   // $FF: synthetic method
   public static void main(String[] args) {
      RunSuspendKt.runSuspend((Function1)2.INSTANCE);
   }
}

And when it's not inlined:

suspend fun foo2(): Int = 1

suspend fun main() {
    println(foo2())
}

then it expands with the regular overhead of continuation wrapping at var10000 = foo2($continuation);

public final class KPreludeNotInlineKt {
   @Nullable
   public static final Object foo2(@NotNull Continuation $completion) {
      return Boxing.boxInt(1);
   }

   @Nullable
   public static final Object main(@NotNull Continuation var0) {
      main.1 $continuation;
      label20: {
         if (var0 instanceof main.1) {
            $continuation = (main.1)var0;
            if (($continuation.label & Integer.MIN_VALUE) != 0) {
               $continuation.label -= Integer.MIN_VALUE;
               break label20;
            }
         }

         $continuation = new main.1(var0);
      }

      Object $result = $continuation.result;
      Object var4 = IntrinsicsKt.getCOROUTINE_SUSPENDED();
      Object var10000;
      switch ($continuation.label) {
         case 0:
            ResultKt.throwOnFailure($result);
            $continuation.label = 1;
            var10000 = foo2($continuation);
            if (var10000 == var4) {
               return var4;
            }
            break;
         case 1:
            ResultKt.throwOnFailure($result);
            var10000 = $result;
            break;
         default:
            throw new IllegalStateException("call to 'resume' before 'invoke' with coroutine");
      }

      int var1 = ((Number)var10000).intValue();
      System.out.println(var1);
      return Unit.INSTANCE;
   }

   // $FF: synthetic method
   public static void main(String[] args) {
      RunSuspendKt.runSuspend((Function1)2.INSTANCE);
   }
}

[raulraja]

great work @nomisRev !

[nomisRev]

Oops. There was only a single approval. @serras do we need to protect the arrow-2 branch with same rules as main?

If there are any additional comments, feel free to post them here or in a new issue and we'll tackle them in as subsequent PR. 🙏