avoid lifting modules (like Abort.type/Var.type, ...) #1291
Conversation
3379a70 to
623963e
Compare
kyo-kernel/shared/src/main/scala/kyo/kernel/internal/WeakFlat.scala
Outdated
Show resolved
Hide resolved
bae49d9 to
d4e0715
Compare
|
I don't like the overall approach since it pollutes the codebase and we can forget to add the marker to new code. How about renaming |
d4e0715 to
4363329
Compare
|
@fwbrasil reworked it as a macro, for |
| object CanLiftMacro: | ||
| inline given derived[A](using inline ng: NotGiven[A <:< (Any < Nothing)]): CanLift[A] = ${ liftImpl[A] } |
There was a problem hiding this comment.
circular references don't help with the macro:
NotGiven[A <:< (Any < Nothing)]is better than doing it in the macro.- the macro have to be isolated from
inline given CanLift[Nothing], hence theCanLiftMacro, then an export.
There was a problem hiding this comment.
why not inspect the A type for nesting in the macro code? There's also Expr.summon I think that can be used in the macro but avoiding another implicit search could be good for compilation times.
There was a problem hiding this comment.
why not inspect the
Atype
It was not working a all, with painful compile errors due to cyclic definitions.
Now it could be working and make PendingTest.scala crash with 32 compile errors:
[error] -- Error: /Users/jon/Documents/GitHub/kyo/kyo-kernel/shared/src/test/scala/kyo/kernel/PendingTest.scala:456:48
[error] 456 | TestEffect1(1).map(_ => Kyo.lift(TestEffect2("hello")))
[error] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error] |Type '<' may contain a nested effect computation.
if tpe <:< TypeRepr.of[Any < Nothing] then
report.errorAndAbort(s"""Type '${sym.name}' may contain a nested effect computation.
|
|This usually means you have a value of type 'X < S' where you need a plain value.
|To fix this, you can:
|
|1. Call .flatten to combine the nested effects:
| (x: (Int < S1) < S2).flatten // Result: Int < (S1 & S2)
|
|2. Split the computation into multiple statements:
| val x: Int < S1 = computeFirst()
| val y: Int < S2 = useResult(x)""".stripMargin)
end ifIt's better to not change this part now, and have a discussion when possible to plan the work on lift.
kyo-kernel/shared/src/main/scala/kyo/kernel/internal/CanLift.scala
Outdated
Show resolved
Hide resolved
2b8580e to
05391eb
Compare
|
With the current implementation
|
| v match | ||
| case kyo: Kyo[?, ?] => Nested(kyo) | ||
| case _ => v.asInstanceOf[A < S] | ||
|
|
||
| implicit inline def liftUnit[S1, S2](inline v: Unit < S1): Unit < S2 = ${ liftUnitImpl[S1, S2]('v) } | ||
| implicit inline def liftAnyVal[A <: AnyVal, S](inline v: A): A < S = v.asInstanceOf[A < S] |
There was a problem hiding this comment.
Do we need CanLift? Technically you could put an AnyVal around a computation, right?
There was a problem hiding this comment.
I am sorry, what do you mean?
There was a problem hiding this comment.
Maybe we should do a call on the rest, their are a lot of interesting things to do with lift.
Eg. optimizing all the Const[A <: AnyVal][?] could be a huge improvement (perf / bytecode). as far as I see, they are not using liftAnyVal, and it's use a lot in ArrowEffect.
|
Bench result, from #1300
the most improved case:
pipe-benchmark.jmh-result.json |
kyo-kernel/shared/src/main/scala/kyo/kernel/internal/CanLift.scala
Outdated
Show resolved
Hide resolved
…cala Co-authored-by: Adam Hearn <22334119+hearnadam@users.noreply.github.com>
…cala Co-authored-by: Adam Hearn <22334119+hearnadam@users.noreply.github.com>
3fb1f12 to
6410c5b
Compare
Plain solution about this problem. There is probably a better way.@johnhungerford that's what I understood from: "Could we use implicit evidence to exclude certain objects from lifting?", you tell me!- [x] add extends KyoModule to all the companion object of effectsUpdating
WeakFlat=>CanLiftwith a macro to not lift modules inkyo.that are not a case object.