-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implemented submarine error propagation for Handle
#619
base: main
Are you sure you want to change the base?
Implemented submarine error propagation for Handle
#619
Conversation
And also run prPR
cb59bec
to
4f19410
Compare
9140fc4
to
cb545b9
Compare
def allow[E]: AdHocSyntaxWired[E] = | ||
new AdHocSyntaxWired[E] | ||
|
||
final class AdHocSyntaxWired[E]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is Scala 3, I think with some clever use of inline
we could make all the allocations / boilerplate disappear from the generated code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried inline in 85e418b.
Combine with moving Inner class out of AdhocSyntaxWired, we completely
remove AdhocSyntaxWired footprint from generated bytecode (user's site)
before:
public final class mtl$minussubmarine$package$
implements Serializable {
public static final mtl$minussubmarine$package$ MODULE$ = new mtl$minussubmarine$package$();
private mtl$minussubmarine$package$() {
}
private Object writeReplace() {
return new ModuleSerializationProxy(mtl$minussubmarine$package$.class);
}
public EitherT<Eval, Throwable, String> test() {
return (EitherT)Handle$.MODULE$.allow() // `allow` return new AdHocSyntaxWired
.apply((Function1 & Serializable)contextual$1 -> {
Error$ error$ = (Error$)all$.MODULE$.toRaiseOps((Object)Error$.MODULE$);
return (EitherT)package.all$.MODULE$.toFunctorOps(RaiseOps$.MODULE$.raise$extension((Object)error$, (Raise)contextual$1), (Functor)EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval())).as((Object)"nope");
}, (ApplicativeError)EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval()))
.rescue((Function1 & Serializable)x$1 -> {
Error$ error$ = x$1;
if (Error$.MODULE$.equals(error$)) {
String string = (String)package.all$.MODULE$.catsSyntaxApplicativeId((Object)"error");
return (EitherT)ApplicativeIdOps$.MODULE$.pure$extension((Object)string, (Applicative)EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval()));
}
throw new MatchError((Object)error$);
});
}
}
after:
public final class mtl$minussubmarine$package$
implements Serializable {
public static final mtl$minussubmarine$package$ MODULE$ = new mtl$minussubmarine$package$();
private mtl$minussubmarine$package$() {
}
private Object writeReplace() {
return new ModuleSerializationProxy(mtl$minussubmarine$package$.class);
}
public EitherT<Eval, Throwable, String> test() {
Handle$ HandleCrossCompat_this = Handle$.MODULE$;
Handle$ HandleCrossCompat_this2 = Handle$.MODULE$;
MonadError x$2$proxy1 = EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval());
return (EitherT)new HandleCrossCompat.InnerWired((HandleCrossCompat)HandleCrossCompat_this2, (Function1 & Serializable)evidence$1 -> {
Error$ error$ = (Error$)all$.MODULE$.toRaiseOps((Object)Error$.MODULE$);
return (EitherT)package.all$.MODULE$.toFunctorOps(RaiseOps$.MODULE$.raise$extension((Object)error$, (Raise)evidence$1), (Functor)EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval())).as((Object)"nope");
}, (ApplicativeError)x$2$proxy1).rescue((Function1 & Serializable)x$1 -> {
Error$ error$ = x$1;
if (Error$.MODULE$.equals(error$)) {
String string = (String)package.all$.MODULE$.catsSyntaxApplicativeId((Object)"error");
return (EitherT)ApplicativeIdOps$.MODULE$.pure$extension((Object)string, (Applicative)EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval()));
}
throw new MatchError((Object)error$);
});
}
}
scala code for both:
//> using scala 3.nightly
//> using dep org.typelevel::cats-mtl:1.5.0-89-85e418b-20250308T182332Z-SNAPSHOT
//> using dep org.typelevel::cats-core:2.13.0
//> using options -explain
import cats.Eval
import cats.data.EitherT
import cats.mtl.Handle.*
import cats.mtl.syntax.all.*
import cats.syntax.all.*
object Error
type F[A] = EitherT[Eval, Throwable, A]
def test1 =
allow[Error.type]:
Error.raise[F, String].as("nope")
.rescue:
case Error => "error".pure[F]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another fancy way to do it is using polymorphic function 40a7772:
- inline def allow[E]: AdHocSyntaxWired[E] =
- new AdHocSyntaxWired[E]()
-
- private[mtl] final class AdHocSyntaxWired[E]:
- inline def apply[F[_], A](inline body: Handle[F, E] ?=> F[A]): InnerWired[F, E, A] =
- new InnerWired(body)
+ def allow[E]: [F[_], A] => (Handle[F, E] ?=> F[A]) => InnerWired[F, E, A] =
+ [F[_], A] => (body: Handle[F, E] ?=> F[A]) => InnerWired(body)
Generated code is a bit nicer compare to inline
solution:
public final class mtl$minussubmarine$package$
implements Serializable {
public static final mtl$minussubmarine$package$ MODULE$ = new mtl$minussubmarine$package$();
private mtl$minussubmarine$package$() {
}
private Object writeReplace() {
return new ModuleSerializationProxy(mtl$minussubmarine$package$.class);
}
public EitherT<Eval, Throwable, String> test1() {
return (EitherT)((HandleCrossCompat.InnerWired)Handle$.MODULE$.allow().apply((Function1 & Serializable)contextual$1 -> {
Error$ error$ = (Error$)all$.MODULE$.toRaiseOps((Object)Error$.MODULE$);
return (EitherT)package.all$.MODULE$.toFunctorOps(RaiseOps$.MODULE$.raise$extension((Object)error$, (Raise)contextual$1), (Functor)EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval())).as((Object)"nope");
})).rescue((Function1 & Serializable)x$1 -> {
Error$ error$ = x$1;
if (Error$.MODULE$.equals(error$)) {
String string = (String)package.all$.MODULE$.catsSyntaxApplicativeId((Object)"error");
return (EitherT)ApplicativeIdOps$.MODULE$.pure$extension((Object)string, (Applicative)EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval()));
}
throw new MatchError((Object)error$);
}, (ApplicativeError)EitherT$.MODULE$.catsDataMonadErrorForEitherT((Monad)Eval$.MODULE$.catsBimonadForEval()));
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! It looks like we are still allocating InnerWired
, do you think we can make that one go away too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! It looks like we are still allocating
InnerWired
, do you think we can make that one go away too?
I tried but couldn't find a way. opaque
doesn't work because opaque
doesn't work with context function.
I tried with value class by extending AnyVal
because of Value classes may not be a member of another class
error.
Maybe there is another way? Let me try some more.
btw, which approach do you prefer polymorphic function or inlining?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, which approach do you prefer polymorphic function or inlining?
I think the polymorphic function is allocating something? to call the apply
on?
Handle$.MODULE$.allow().apply
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right, let me investigate.
c4ce3fa
to
ca4d5e8
Compare
Co-authored-by: Arman Bilge <[email protected]>
ca4d5e8
to
74dda73
Compare
Combine with moving Inner class out of AdhocSyntaxWired, we completely remove AdhocSyntaxWired footprint from generated bytecode (user's site)
8eebab3
to
85e418b
Compare
And use assert.equals instead of ==
@scala.annotation.nowarn("msg=dubious usage of method hashCode with unit value") | ||
private[mtl] final class AdHocSyntaxTired[F[_], E](private val unit: Unit) extends AnyVal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, that's a weird warning 😅 instead of Unit
we could also use dummy: Byte
or something else.
This is a follow up of #489 on behalf of @djspiewak, the description below is copied from previous pr with some minor update of the examples.
This adds builder syntax to Handle which makes it possible to materialize an instance of Handle for any error type given an
ApplicativeThrow[F]
. Syntax is intentionally reminiscent oftry/catch
:Scala 2 example
scala 3 example
You can see some other examples in the tests. You can suspend as many different error types as you want and it all composes the way you would expect.
This effectively eliminates the need for
EitherT
orIorT
outside of tests (where both can still be useful for building ad-hoc harnesses that are less powerful thanIO
). Types infer quite nicely on both Scala 2 and Scala 3, though obviously Scala 3'susing
syntax makes this even nicer.