Skip to content

Commit

Permalink
Pull - Scope: rewrites
Browse files Browse the repository at this point in the history
We add to Scope the following methods:
- A method to ask if this is the Root Scope.
- A method to get the depth (distance to the root scope)
- A method to search either a descendent or ancestor scope.

Pull: rewrite `goCloseScope` method to reduce code nesting:
- Split from `closeAndGo` the logic to get the Result
- Use cases with guards for the choices.
- Inline top-level parts of `closeAndGo` method.
  • Loading branch information
diesalbla committed Dec 29, 2020
1 parent 1ea8e42 commit 16e2d80
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 31 deletions.
48 changes: 22 additions & 26 deletions core/shared/src/main/scala/fs2/Pull.scala
Original file line number Diff line number Diff line change
Expand Up @@ -873,19 +873,16 @@ object Pull extends PullLowPriority {
}
}
}
val maybeCloseExtendedScope: F[Boolean] =
val maybeCloseExtendedScope: F[Option[Scope[F]]] =
// If we're opening a new top-level scope (aka, direct descendant of root),
// close the current extended top-level scope if it is defined.
if (scope.parent.isEmpty)
extendedTopLevelScope match {
case None => false.pure[F]
case Some(s) => s.close(ExitCase.Succeeded).rethrow.as(true)
}
else F.pure(false)
if (scope.isRoot && extendedTopLevelScope.isDefined)
extendedTopLevelScope.traverse_(_.close(ExitCase.Succeeded).rethrow).as(None)
else
F.pure(extendedTopLevelScope)

val vrun = new ViewRunner(view)
val tail = maybeCloseExtendedScope.flatMap { closedExtendedScope =>
val newExtendedScope = if (closedExtendedScope) None else extendedTopLevelScope
val tail = maybeCloseExtendedScope.flatMap { newExtendedScope =>
scope.open(useInterruption).rethrow.flatMap { childScope =>
go(childScope, newExtendedScope, translation, vrun, boundToScope(childScope.id))
}
Expand All @@ -894,14 +891,12 @@ object Pull extends PullLowPriority {
}

def goCloseScope(close: CloseScope, view: Cont[Unit, G, X]): F[End] = {
def closeAndGo(toClose: Scope[F]) =
toClose.close(close.exitCase).flatMap { r =>
toClose.openAncestor.flatMap { ancestor =>
val res = close.interruption match {
def closeResult(r: Either[Throwable, Unit], ancestor: Scope[F]): Result[Unit] =
close.interruption match {
case None => Result.fromEither(r)
case Some(Result.Interrupted(interruptedScopeId, err)) =>
def err1 = CompositeFailure.fromList(r.swap.toOption.toList ++ err.toList)
if (ancestor.findSelfOrAncestor(interruptedScopeId).isDefined)
if (ancestor.descendsFrom(interruptedScopeId))
// we still have scopes to interrupt, lets build interrupted tail
Result.Interrupted(interruptedScopeId, err1)
else
Expand All @@ -911,27 +906,28 @@ object Pull extends PullLowPriority {
case Some(err) => Result.Fail(err)
}
}
go(ancestor, extendedTopLevelScope, translation, endRunner, view(res))
}
}

val scopeToClose: F[Option[Scope[F]]] = scope
.findSelfOrAncestor(close.scopeId)
.pure[F]
.orElse(scope.findSelfOrChild(close.scopeId))
scopeToClose.flatMap {
case Some(toClose) =>
if (toClose.parent.isEmpty)
scope.findInLineage(close.scopeId).flatMap {
case Some(toClose) if toClose.isRoot =>
// Impossible - don't close root scope as a result of a `CloseScope` call
go(scope, extendedTopLevelScope, translation, endRunner, view(Result.unit))
else if (extendLastTopLevelScope && toClose.parent.flatMap(_.parent).isEmpty)

case Some(toClose) if extendLastTopLevelScope && toClose.level == 1 =>
// Request to close the current top-level scope - if we're supposed to extend
// it instead, leave the scope open and pass it to the continuation
extendedTopLevelScope.traverse_(_.close(ExitCase.Succeeded).rethrow) *>
toClose.openAncestor.flatMap { ancestor =>
go(ancestor, Some(toClose), translation, endRunner, view(Result.unit))
}
else closeAndGo(toClose)

case Some(toClose) =>
toClose.close(close.exitCase).flatMap { r =>
toClose.openAncestor.flatMap { ancestor =>
val res = closeResult(r, ancestor)
go(ancestor, extendedTopLevelScope, translation, endRunner, view(res))
}
}

case None =>
// scope already closed, continue with current scope
val result = close.interruption.getOrElse(Result.unit)
Expand Down
34 changes: 29 additions & 5 deletions core/shared/src/main/scala/fs2/internal/Scope.scala
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,25 @@ import fs2.internal.InterruptContext.InterruptionOutcome
*/
private[fs2] final class Scope[F[_]] private (
val id: Unique,
val parent: Option[Scope[F]],
val interruptible: Option[InterruptContext[F]],
private val parent: Option[Scope[F]],
interruptible: Option[InterruptContext[F]],
private val state: Ref[F, Scope.State[F]]
)(implicit val F: Compiler.Target[F]) { self =>

/** Registers supplied resource in this scope.
def isRoot: Boolean = parent.isEmpty

/** Gives the level or distance of this scope from the root,
* where the root has level 0, its children level 1, etc.
*/
def level: Int = {
@tailrec def go(scope: Scope[F], acc: Int): Int = scope.parent match {
case None => acc
case Some(s) => go(s, acc + 1)
}
go(this, 0)
}

/* Registers supplied resource in this scope.
* Returns false and makes no registration if this scope has been closed.
*/
private def register(resource: ScopedResource[F]): F[Boolean] =
Expand Down Expand Up @@ -283,8 +296,12 @@ private[fs2] final class Scope[F[_]] private (
go(self, Chain.empty)
}

/** @returns true if the given `scopeId` identifies an ancestor of this scope, or false otherwise.
*/
def descendsFrom(scopeId: Unique): Boolean = findSelfOrAncestor(scopeId).isDefined

/** Finds ancestor of this scope given `scopeId`. */
def findSelfOrAncestor(scopeId: Unique): Option[Scope[F]] = {
private def findSelfOrAncestor(scopeId: Unique): Option[Scope[F]] = {
@tailrec
def go(curr: Scope[F]): Option[Scope[F]] =
if (curr.id == scopeId) Some(curr)
Expand All @@ -296,8 +313,15 @@ private[fs2] final class Scope[F[_]] private (
go(self)
}

/** Looks for the scopeId in this scope lineage, that being either
* its ancestors or its descendants, but not lateral branches.
* (brothers, uncles, nephews, etc)
*/
def findInLineage(scopeId: Unique): F[Option[Scope[F]]] =
findSelfOrAncestor(scopeId).pure[F].orElse(findSelfOrChild(scopeId))

/** Finds scope in child hierarchy of current scope. */
def findSelfOrChild(scopeId: Unique): F[Option[Scope[F]]] = {
private def findSelfOrChild(scopeId: Unique): F[Option[Scope[F]]] = {
def go(scopes: Chain[Scope[F]]): F[Option[Scope[F]]] =
scopes.uncons match {
case None => F.pure(None)
Expand Down

0 comments on commit 16e2d80

Please sign in to comment.