Update Raise.xOrAccumulate #2958
Conversation
Kover Report
|
nomisRev
force-pushed
the
sv-update-raiseaccumulate
branch
from
a88a9d0
to
1129b23
Compare
nomisRev
force-pushed
the
sv-update-raiseaccumulate
branch
from
1129b23
to
9b3889c
Compare
nomisRev
force-pushed
the
sv-update-raiseaccumulate
branch
from
70e831c
to
0239aaa
Compare
nomisRev
force-pushed
the
sv-update-raiseaccumulate
branch
from
0239aaa
to
5ea2cf4
Compare
| @@ -1,4 +1,4 @@ | |||
| @file:OptIn(ExperimentalTypeInference::class, ExperimentalContracts::class) | |||
There was a problem hiding this comment.
I got confused how we should use them, because they resulted in a couple of false-negatives of "Unreachable code" 😕
In this case of catch we should be able to tell the compiler that the second lambda "recovers" from the Nothing of the first lambda. It's a bit strange since the inferred returned type is Int of the catch lambda. So I guess callsInPlace assumes not-wrapped in try?
The second one is more complex.. since either is also callsInPlace and the last statement is an exception, but it's not throwing but forever suspending... And it exists early due to raise 😅
Not sure how we should deal with this best. WDYT @serras?
There was a problem hiding this comment.
I also noticed that ResultKt only specifies AT_MOST_ONCE and never EXACTLY_ONCE. EXACTLY_ONCE only appears in very little places in Kotlin StdLib 🤔 I also asked this question in KotlinLang #compiler if there is any guidelines for this.
There was a problem hiding this comment.
Thanks for the thorough investigation, I never noticed that before. I assumed that this would help the compiler inline or optimize the code better, but it seems that it's reaching wrong conclusions...
There was a problem hiding this comment.
I am going to merge this PR, but I am continuing my search for a clear answer on this in Kotlin Slack. Re-asked my question in #library-development, https://kotlinlang.slack.com/archives/C8C4JTXR7/p1678213597746259.
It seems that AT_MOST_ONCE doesn't suffer from the false-negative so I am going to re-add that back before merging.
|
Not sure why you closed @raulraja? |
| val error = mutableListOf<R>() | ||
| val results = ArrayList<B>(list.collectionSizeOrDefault(10)) | ||
| for (item in list) { | ||
| fold<NonEmptyList<R>, B, Unit>( | ||
| { transform(RaiseAccumulate(this), item) }, | ||
| { errors -> error.addAll(errors) }, | ||
| { results.add(it) } | ||
| ) | ||
| } | ||
| return error.toNonEmptyListOrNull()?.let { raise(it) } ?: results |
There was a problem hiding this comment.
The fact that this can be defined now once and for all in a nice way fills me with joy :)
This PR updates
RaiseAccumulateDSL with changes from #2952.Nel<E>andEin the same DSL in Raise DSLIterable.mapOrAccumulate