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

FreeC implementation #1048

Merged
merged 19 commits into from
Nov 9, 2018
Merged

FreeC implementation #1048

merged 19 commits into from
Nov 9, 2018

Conversation

nomisRev
Copy link
Member

This PR is the starting point of a FS2 port or FS2 inspired stream lib.

In discussion with @raulraja we had the idea to move this to arrow-free as a replacement for the current Free implementation since we can then provide instance up to MonadDefer (and later also Sync).

If we are to move FreeC to arrow-free I'd like to improve the Result and ViewL code. Does any1 know if we could do something with inline classes to create sealed class wrappers without runtime overhead? I am assuming since they're going to be nested that they'll auto-box?

@codecov
Copy link

codecov bot commented Oct 14, 2018

Codecov Report

Merging #1048 into master will decrease coverage by 0.11%.
The diff coverage is 36.36%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1048      +/-   ##
============================================
- Coverage     42.39%   42.28%   -0.12%     
- Complexity      750      755       +5     
============================================
  Files           360      365       +5     
  Lines          9989    10219     +230     
  Branches       1082     1147      +65     
============================================
+ Hits           4235     4321      +86     
- Misses         5439     5562     +123     
- Partials        315      336      +21
Impacted Files Coverage Δ Complexity Δ
...core/arrow-free/src/main/kotlin/arrow/free/Free.kt 57.14% <ø> (ø) 0 <0> (ø) ⬇️
...-streams/src/main/kotlin/arrow/effects/ExitCode.kt 0% <0%> (ø) 0 <0> (?)
...re/arrow-core/src/main/kotlin/arrow/core/Option.kt 59.67% <0%> (ø) 22 <0> (ø) ⬇️
.../src/main/kotlin/arrow/streams/CompositeFailure.kt 0% <0%> (ø) 0 <0> (?)
...rc/main/kotlin/arrow/streams/internal/instances.kt 34.78% <34.78%> (ø) 0 <0> (?)
...ms/src/main/kotlin/arrow/streams/internal/FreeC.kt 39.89% <39.89%> (ø) 4 <4> (?)
...ms/src/main/kotlin/arrow/streams/internal/Token.kt 50% <50%> (ø) 1 <1> (?)
...ics/src/main/kotlin/arrow/optics/instances/mapk.kt 86.36% <0%> (-4.55%) 0% <0%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5088f9b...f82dac6. Read the comment docs.

@raulraja
Copy link
Member

@nomisRev AFAIK inline classes properties of erasing the wrapper are not applied to any kind of generic container and they only work with primitives but @pakoito may know more.

Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

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

Great start!

@@ -191,7 +191,7 @@ fun <T> Option<T>.getOrElse(default: () -> T): T = fold({ default() }, ::identit
*
* @param alternative the default option if this is empty.
*/
inline fun <A, B : A> OptionOf<B>.orElse(alternative: () -> Option<B>): Option<B> = if (fix().isEmpty()) alternative() else fix()
inline fun <A> OptionOf<A>.orElse(alternative: () -> Option<A>): Option<A> = if (fix().isEmpty()) alternative() else fix()
Copy link
Member

Choose a reason for hiding this comment

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

nice one

}

internal object EitherToTry : FunctionK<EitherPartialOf<Throwable>, ForTry> {
override fun <A> invoke(fa: Kind<EitherPartialOf<Throwable>, A>): Kind<ForTry, A> =
Copy link
Member

Choose a reason for hiding this comment

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

We have Try#toEither. Perhaps this implementation can be exposed in the core as Either<Throwable, A>.toTry()

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah sure. Btw should we add FunctionK instances like this one to Arrow? Or is there a ticket for this that I missed?

Copy link
Member

Choose a reason for hiding this comment

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

@nomisRev they won't hurt, but it isn't a priority. Only the ones between Try/Either/Option and maybe ListK may be more interesting for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pakoito I could quickly add them in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Just a friendly reminder that in 0.8.0 the use of ListK and wrappers of one type arguments are rarely seen in user code unless they have polymorphic functions that expect values in the kind hierarchy.

Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

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

Awesome stuff! 👏

@@ -0,0 +1,14 @@
dependencies {
Copy link
Member

Choose a reason for hiding this comment

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

🎉 it's alive!

import arrow.typeclasses.MonadError

/**
* Free Monad with Catch (and Interruption).
Copy link
Member

Choose a reason for hiding this comment

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

}
)

/** @higherkind doesn't respect access modifier **/
Copy link
Member

Choose a reason for hiding this comment

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

once it is migrated to the meta processor you may be able to get that info

this.fix().map(f)
}

internal fun <F> FreeC.Companion.applicative(): Applicative<FreeCPartialOf<F>> = object : FreeCApplicative<F> { }
Copy link
Member

Choose a reason for hiding this comment

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

Are these handwritten because they are internal? After 0.8.0 we should be able to encode internal instances in the ExtensionProcessor so all the syntax they export it's also internal.

EQ_EITHER = Eq { a, b -> a.run(Try.monadError()) == b.run(Try.monadError()) }
))

testLaws(MonadDeferLaws.laws(
Copy link
Member

Choose a reason for hiding this comment

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

I thought the laws where encoded such as if you just test the MonadDeferLaws it will include all others in its hierarchy.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, they're. This is left over from when I was debugging combinators in smaller sets at a time :D

}
}

"Running an suspended value"{
Copy link
Member

Choose a reason for hiding this comment

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

s/an/a

}

internal object EitherToTry : FunctionK<EitherPartialOf<Throwable>, ForTry> {
override fun <A> invoke(fa: Kind<EitherPartialOf<Throwable>, A>): Kind<ForTry, A> =
Copy link
Member

Choose a reason for hiding this comment

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

Just a friendly reminder that in 0.8.0 the use of ListK and wrappers of one type arguments are rarely seen in user code unless they have polymorphic functions that expect values in the kind hierarchy.

@raulraja
Copy link
Member

@nomisRev This is ready to go once you rebase from master

@nomisRev
Copy link
Member Author

nomisRev commented Nov 9, 2018

@raulraja could you review again? Also API / encoding of FreeC. I think this is about as good as it gets and if everyone agrees I'll move it to arrow-free.

Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

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

This is excellent Simon! Kudos!, Our Free impl is now one step closer to have more superpowers than the scala cats one which had limited us In Freestyle big time

}
is Fail<*, R> -> free
is Interrupted<*, R, *> -> free
else -> throw AssertionError("Unreachable")
Copy link
Member

Choose a reason for hiding this comment

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

In the event that this is ever thrown we should indicate in the exception message that this is potentially caused by a bug and provide a link to report it in the arrow issue tracker.

@raulraja raulraja merged commit 2039fd8 into master Nov 9, 2018
@raulraja raulraja deleted the fs2-port-poc branch November 9, 2018 16:20
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