Added Scala 3 support #5
Conversation
| _root_.cps.await[F, A](self) | ||
| } | ||
|
|
||
| implicit def catsEffectCpsMonadPureMemoization[F[_]](implicit F: Concurrent[F]): CpsMonadPureMemoization[F] = |
There was a problem hiding this comment.
As far as I understand, this construct is used conjointly with a given import to enable automatic colouring. I've already expressed my concerns against it, and I'd rather we avoided becoming enablers of (what I consider) a bad pattern.
So I'd like this instance to be removed... If people really want it, they could add it in userland.
There was a problem hiding this comment.
No, automatic coloring is enabled only if given cps.automaticColoring.tag is in scope. I.e. it is always enabled on userland unless you do not reexport automatic coloring (not memorization) in your code. So, define memorization here is safe.
There was a problem hiding this comment.
it is always enabled on userland unless you do not reexport automatic coloring (not memorization) in your code.
you mean it is always disabled in userland, right (unless the import is in scope which requires decision from the user) ?
Even if that's the case, I still oppose the auto-colouring pattern and wouldn't want this library (cats-effect-cps) to make it easier for people to enable.
There was a problem hiding this comment.
yes.
Hmm, a problem that memoization is a right description on functionality and disabling it potentially can break other functionality, not related to coloring at all.
The right way to disable automatic coloring will be to define another tag to force implicit resolution conflict. We can make AutomaticColoringTag be depended on monad, so you can reexport something like:
@implicitAmbiguous ("Automatic coloring for IO is disabled, use another library if you want automatic coloring")
given DisableAutomaticColoringForIO: cps.automaticColoring.AutomaticColoringTag[IO]
There was a problem hiding this comment.
Hmm, a problem that memoization is a right description on functionality and disabling it potentially can break other functionality, not related to coloring at all.
I see, thanks 😄 . out of curiosity, could you formulate an example of how memoization is used in contexts other than auto-colouring ? In particular, I want to ensure that its presence wouldn't break least-surprise.
There was a problem hiding this comment.
Right now, it's not used in dotty-cps-async for purposes other than coloring. (It's why I have written potentially). But imagine you, as a developer, building something which should logically works with memoization. For example, it can be a cache layer in some service or broadcasting events to listeners, where events are processed once, but the results are broadcasted. And then you are discovering that this is impossible to do if you import cps-async structures for IO via cats-effect-cps, because authors of cats-effect-cps dislike automatic coloring ;).
On the other side, it's your choice - of course, in your library you can do whatever you want ;)
// btw, I'm not happy with the current automatic coloring for effect monads and trying to explore now some other ways, which should be more concise for developers. Some of those approaches use memoization more explicitly than now. I will be able to say more a bit later when will have some results.
There was a problem hiding this comment.
@Baccata I believe the memoization is needed for correctly translating vals under some circumstances. This is one of the areas where monadless can sometimes change the meaning of the code. The trick is that dotty-cps-async isn't running the effects, but rather performing a transformation of the call graph. Thus, in some scenarios, it needs the ability to repeatedly sequence the same effect multiple times but without re-evaluating it.
There was a problem hiding this comment.
The trick is that dotty-cps-async isn't running the effects, but rather performing a transformation of the call graph.
Yeah that is pretty clear, and it's quite amazing that it's able to do that on a reasonably large subset of the language (including while loops, etc)
I believe the memoization is needed for correctly translating vals under some circumstances
I do understand (intuitively) that memoization is useful when auto-colouring is involved, but the author has indicated (in the message above yours) that memoization is, at the moment, only used in that very context. Do you have an example in mind of a possible transformation (even one not currently supported) that would benefit from memoization when the user has to manually apply colouring ? (I'm not being rhetorical here, genuinely curious as to whether you can think of one)
There was a problem hiding this comment.
I do understand (intuitively) that memoization is useful when auto-colouring is involved, but the author has indicated (in the message above yours) that memoization is, at the moment, only used in that very context. Do you have an example in mind of a possible transformation (even one not currently supported) that would benefit from memoization when the user has to manually apply colouring ? (I'm not being rhetorical here, genuinely curious as to whether you can think of one)
Ah, it would help if I could read. You're correct.
So I defined the instance because it fails to compile without it. That's the baseline, but beyond that, the scenarios I'm thinking about would have to involve something like val x = ioa.await followed by some weird construct which defies reconstitution into a flatMap. I'm really not sure what that would be though, if anything.
Either way, I'm not too worried about it. I agree we don't want autocoloring, but since that's controlled independently, implementing this abstraction seems harmless enough. Concurrent[F].memoize is safe to use.
There was a problem hiding this comment.
So I defined the instance because it fails to compile without it
That is a sufficient argument for me to accept the presence of the instance in this PR 👍
| "org.specs2" %% "specs2-core" % "4.12.1" % Test)) | ||
| libraryDependencies ++= { | ||
| if (isDotty.value) | ||
| Seq("com.github.rssh" %%% "dotty-cps-async" % "0.8.1") |
There was a problem hiding this comment.
Here are my thoughts (re-iterated here for the sake of visibility) : dotty-cps-async is a super cool library, but before we commit to depending on it, I'd like to know more about the status/plan for it. Is it sponsored commercially ? Is it a research project conducted by a small bunch of individuals for the sake of proving it's possible ? Are there (even loose) plans for it to be integrated into the Scala 3 standard library ? (or even moved over to the scala organisation). (@rssh could you provide insight ?)
My concern is that the AST transformation this library performs is not trivial by any mean. If it was to become unmaintained (for any reason whatsoever), we'd be in the awkward position of having to assume maintenance burden, burden which is increased by the currently poor state of tooling and documentation around metaprogramming in Scala 3.
Of course one could say "we could always give up on cats-effect-cps if that happens", but if people actually start to use it, I for one will experience the self-inflicted pressure to assume the maintenance cost.
I'm talking from experience here, as I'm now participating (with @keynmol) to the maintenance of https://github.com/eed3si9n/expecty, another macro-heavy library, because weaver depends on it and Eugene doesn't have the capacity to maintain it properly.
There was a problem hiding this comment.
1 - yes. Note, that the company behind is mine.
2 - not clear, what is 'research project', how you differ research project from non-research?
3 - out of my control. From my estimations of the scala-center processes - no, but better ask them directly ;)
4. - out of my control.
There was a problem hiding this comment.
I definitely agree that this is not a dependency we would take on lightly. I think it's okay to pull it into cats-effect-cps for now, particularly since we have giant "EXPERIMENTAL" flags all over everything, but before we pull this into CE core we're going to need to have a larger conversation.
My guess is that this is indeed something that both Scala Center and EPFL would be interested in. I'm making that guess for a couple of reasons:
- This dovetails very closely with some @odersky's current research into effect systems, and indeed provides a pathway for that research to be generalized to existing systems like Cats Effect
- Certain major companies which rely on Scala 2 will need this kind of functionality to even consider Scala 3
- There's a lot of community interest in this type of syntactic construct, which is an obvious motivator for not only SC but also other major partners such as VirtusLabs and 47 Degrees
Tldr, I would be very surprised if something like this didn't end up in the Scala 3 compiler at some point. When and in what form are more open questions. It would also be lovely to hear from more authoritative voices than mine on some of the future thinking around this.
There was a problem hiding this comment.
but before we pull this into CE core we're going to need to have a larger conversation.
Alright, gave a fair bit of thoughts to this. Given that the merge into CE would be conditioned by not-only the stabilisation of -Xasync on Scala2, but also by the gathering of enough evidence that this syntax is actually used, I think I'm okay to accept the dependency at this point in time, as it will give us more data points.
Will do! |
There's a workaround in here for rssh/dotty-cps-async#40, but otherwise it's fairly straightforward. It's going to need to change a lot once 0.9 releases, but in a good way (since the revised hierarchy should allow us to support
spawnand such).Fixes #1