Skip to content
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

Make StackSafeMonad extend Defer #3962

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

johnynek
Copy link
Contributor

Make StackSafeMonad extend Defer.

See the discussion in #3790 for more context.

The argument for why this is safe is given in the comments on StackSafeMonad. If they don't seem to make sense, please add a comment there.

cc @djspiewak @non

I think we can follow up with using an idea similar to shift from Kleisli to make an Applicative map2 abstraction. As a sketch:

trait Map2Strategy[F[_]] {
  type Lazy[_]
  def map2[A, B](fa: F[A], fb: Lazy[F[B]]): Lazy[F[B]]
}

and then we can either make Lazy[A] = Eval[A] or Lazy[A] = Id[A] depending on the F[_] which maybe we can reflectively select based on if the F[_] is a StackSafeMonad or not.

Or something...

Copy link
Member

@djspiewak djspiewak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like where this is going. I had a similar thought around the Lazy[_] idea but still trying to piece through how we can make it not-horrible.

Comment on lines +41 to +45
def shiftFunctor[F[_], A, B](fn: A => F[B])(implicit F: Functor[F]): A => F[B] =
F match {
case ssm: StackSafeMonad[F] @unchecked => { a => ssm.defer(fn(a)) }
case _ => fn
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's a way to make this more general. A => F[B] works extremely well for Kleisli and extremely not-well for almost anything else, since it will force boxing into a Function1 in cases where it just isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it shouldn't be public at all. I just took what was in Kleisli and made it more particular. I think the current Kleisli hack is adding cost for any Monad that isn't lazy (e.g. Option, or Either), and this function will help there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When in doubt, leave it private. Can always publicize it later if someone finds a second use.

core/src/main/scala/cats/instances/tailrec.scala Outdated Show resolved Hide resolved
core/src/main/scala/cats/StackSafeMonad.scala Outdated Show resolved Hide resolved
Accept @djspiewak's suggestions

Co-authored-by: Daniel Spiewak <djspiewak@gmail.com>
@johnynek
Copy link
Contributor Author

@djspiewak please take a look and see what you think with your changes merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants