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.

Sign up for GitHub

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

MVar: add "tryRead" fixes #732 #739

Merged
merged 2 commits into from
Apr 29, 2020
Merged

MVar: add "tryRead" fixes #732 #739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9399999
MVar: add "tryRead" and "swap" fixes #732
kubum Feb 8, 2020
78cc281
Add Scala compiler plugin: "sbt-silencer"
kubum Feb 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ val CompileTime = config("CompileTime").hide
val SimulacrumVersion = "1.0.0"
val CatsVersion = "2.1.1"
val DisciplineScalatestVersion = "1.0.1"
val SilencerVersion = "1.6.0"
val customScalaJSVersion = Option(System.getenv("SCALAJS_VERSION"))

addCommandAlias("ci", ";scalafmtSbtCheck ;scalafmtCheckAll ;test ;mimaReportBinaryIssues; doc")
Expand Down Expand Up @@ -239,6 +240,11 @@ lazy val core = crossProject(JSPlatform, JVMPlatform)
"org.typelevel" %%% "cats-laws" % CatsVersion % Test,
"org.typelevel" %%% "discipline-scalatest" % DisciplineScalatestVersion % Test
),
libraryDependencies ++= Seq(
compilerPlugin(("com.github.ghik" % "silencer-plugin" % SilencerVersion).cross(CrossVersion.full)),
("com.github.ghik" % "silencer-lib" % SilencerVersion % CompileTime).cross(CrossVersion.full),
("com.github.ghik" % "silencer-lib" % SilencerVersion % Test).cross(CrossVersion.full)
),
libraryDependencies ++= {
CrossVersion.partialVersion(scalaVersion.value) match {
case Some((2, v)) if v <= 12 =>
Expand Down Expand Up @@ -330,9 +336,6 @@ lazy val siteSettings = Seq(
Map("permalink" -> "/", "title" -> "Home", "section" -> "home", "position" -> "0")
)
),
micrositeConfigYaml := ConfigYml(
yamlCustomProperties = Map("plugins" -> List("jekyll-relative-links"))
),
micrositeCompilingDocsTool := WithMdoc,
mdocIn := (sourceDirectory in Compile).value / "mdoc",
fork in mdoc := true,
Expand Down
81 changes: 67 additions & 14 deletions core/shared/src/main/scala/cats/effect/concurrent/MVar.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
package cats.effect
package concurrent

import cats.effect.concurrent.MVar.TransformedMVar
import cats.effect.concurrent.MVar.{TransformedMVar, TransformedMVar2}
import cats.effect.internals.{MVarAsync, MVarConcurrent}
import cats.~>
import com.github.ghik.silencer.silent

/**
* A mutable location, that is either empty or contains
* a value of type `A`.
* @define mVarDescription A mutable location, that is either empty or contains a value of type `A`.
*
* It has the following fundamental atomic operations:
*
Expand All @@ -37,6 +37,8 @@ import cats.~>
* - [[read]] which reads the current value without touching it,
* assuming there is one, or otherwise it waits until a value
* is made available via `put`
* - `tryRead` returns a variable if it exists. Implemented in the successor [[MVar2]]
* - `swap` takes a value, replaces it and returns the taken value. Implemented in the successor [[MVar2]]
* - [[isEmpty]] returns true if currently empty
*
* The `MVar` is appropriate for building synchronization
Expand All @@ -52,7 +54,13 @@ import cats.~>
* Inspired by `Control.Concurrent.MVar` from Haskell and
* by `scalaz.concurrent.MVar`.
*/
abstract class MVar[F[_], A] {
sealed private[concurrent] trait MVarDocumentation extends Any {}

/**
* $mVarDescription
*/
@deprecated("`MVar` is now deprecated in favour of a new generation `MVar2` with `tryRead` and `swap` support", "2.2.0")
abstract class MVar[F[_], A] extends MVarDocumentation {

/**
* Returns `true` if the `MVar` is empty and can receive a `put`, or
Expand Down Expand Up @@ -120,6 +128,39 @@ abstract class MVar[F[_], A] {
new TransformedMVar(this, f)
}

/**
* $mVarDescription
*
* The `MVar2` is the successor of `MVar` with [[tryRead]] and [[swap]]. It was implemented separately only to maintain
* binary compatibility with `MVar`.
Copy link
Member

Choose a reason for hiding this comment

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

We want to provide good ScalaDocs. I wonder what we can do here, because people are going to be looking at the description of MVar2, which has this sentence that has technical implementation details and with no link to MVar. And then if they'll take a look at the ScalaDoc for MVar, they'll see it as being deprecated.

Copy link
Author

Choose a reason for hiding this comment

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

Good point. It seems there are two ways:

  • Reference both. The downside of it is that tools (IDE helpers) won't give the relevant docs immediately.
  • Copy docs from MVar to MVar2. The downside is maintaining the two copies.

Are there something else? What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

You can define reusable documentation via @define in ScalaDoc.

And definitions get inherited, so if you @define something on MVar, it should then be available in MVar2. And a trick would be to do something like ...

/** 
  * @define mvarDescription An MVar is a mutable location that can be empty or 
  *         contain a value, asynchronously blocking reads when empty and 
  *         blocking writes when full.
  * 
  *         Use-cases:
  *          - bla bla ...
  */
private[concurrent] trait MVarDocs extends Any {}

/** 
  * $mvarDescription
  */
abstract class MVar[F[_], A] extends MVarDocs

/** 
  * $mvarDescription
  */
abstract class MVar2[F[_], A] extends MVar[F, A]

It's just a template value, so you can add your warning before or after it.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, it is a great suggestion, thank you. I just added it.

I have tried to avoid the redundant trait, but scaladocs kept crashing with expansion errors.

*/
@silent("deprecated")
abstract class MVar2[F[_], A] extends MVar[F, A] {

/**
* Replaces a value in MVar and returns the old value.

* @param newValue is a new value
* @return the value taken
*/
def swap(newValue: A): F[A]

/**
* Returns the value without waiting or modifying.
*
* This operation is atomic.
*
* @return an Option holding the current value, None means it was empty
*/
def tryRead: F[Option[A]]

/**
* Modify the context `F` using transformation `f`.
*/
override def mapK[G[_]](f: F ~> G): MVar2[G, A] =
new TransformedMVar2(this, f)
}

/** Builders for [[MVar]]. */
object MVar {

Expand Down Expand Up @@ -156,7 +197,7 @@ object MVar {
* @param F is a [[Concurrent]] constraint, needed in order to
* describe cancelable operations
*/
def empty[F[_], A](implicit F: Concurrent[F]): F[MVar[F, A]] =
def empty[F[_], A](implicit F: Concurrent[F]): F[MVar2[F, A]] =
F.delay(MVarConcurrent.empty)

/**
Expand All @@ -172,7 +213,7 @@ object MVar {
*
* @see [[empty]] for creating cancelable MVars
*/
def uncancelableEmpty[F[_], A](implicit F: Async[F]): F[MVar[F, A]] =
def uncancelableEmpty[F[_], A](implicit F: Async[F]): F[MVar2[F, A]] =
F.delay(MVarAsync.empty)

/**
Expand All @@ -187,7 +228,7 @@ object MVar {
* @param F is a [[Concurrent]] constraint, needed in order to
* describe cancelable operations
*/
def of[F[_], A](initial: A)(implicit F: Concurrent[F]): F[MVar[F, A]] =
def of[F[_], A](initial: A)(implicit F: Concurrent[F]): F[MVar2[F, A]] =
F.delay(MVarConcurrent(initial))

/**
Expand All @@ -204,31 +245,31 @@ object MVar {
*
* @see [[of]] for creating cancelable MVars
*/
def uncancelableOf[F[_], A](initial: A)(implicit F: Async[F]): F[MVar[F, A]] =
def uncancelableOf[F[_], A](initial: A)(implicit F: Async[F]): F[MVar2[F, A]] =
F.delay(MVarAsync(initial))

/**
* Like [[of]] but initializes state using another effect constructor
*/
def in[F[_], G[_], A](initial: A)(implicit F: Sync[F], G: Concurrent[G]): F[MVar[G, A]] =
def in[F[_], G[_], A](initial: A)(implicit F: Sync[F], G: Concurrent[G]): F[MVar2[G, A]] =
F.delay(MVarConcurrent(initial))

/**
* Like [[empty]] but initializes state using another effect constructor
*/
def emptyIn[F[_], G[_], A](implicit F: Sync[F], G: Concurrent[G]): F[MVar[G, A]] =
def emptyIn[F[_], G[_], A](implicit F: Sync[F], G: Concurrent[G]): F[MVar2[G, A]] =
F.delay(MVarConcurrent.empty)

/**
* Like [[uncancelableOf]] but initializes state using another effect constructor
*/
def uncancelableIn[F[_], G[_], A](initial: A)(implicit F: Sync[F], G: Async[G]): F[MVar[G, A]] =
def uncancelableIn[F[_], G[_], A](initial: A)(implicit F: Sync[F], G: Async[G]): F[MVar2[G, A]] =
F.delay(MVarAsync(initial))

/**
* Like [[uncancelableEmpty]] but initializes state using another effect constructor
*/
def uncancelableEmptyIn[F[_], G[_], A](implicit F: Sync[F], G: Async[G]): F[MVar[G, A]] =
def uncancelableEmptyIn[F[_], G[_], A](implicit F: Sync[F], G: Async[G]): F[MVar2[G, A]] =
F.delay(MVarAsync.empty)

/**
Expand All @@ -241,15 +282,15 @@ object MVar {
*
* @see documentation for [[MVar.of]]
*/
def of[A](a: A): F[MVar[F, A]] =
def of[A](a: A): F[MVar2[F, A]] =
MVar.of(a)(F)

/**
* Builds an empty `MVar`.
*
* @see documentation for [[MVar.empty]]
*/
def empty[A]: F[MVar[F, A]] =
def empty[A]: F[MVar2[F, A]] =
MVar.empty(F)
}

Expand All @@ -262,4 +303,16 @@ object MVar {
override def tryTake: G[Option[A]] = trans(underlying.tryTake)
override def read: G[A] = trans(underlying.read)
}

final private[concurrent] class TransformedMVar2[F[_], G[_], A](underlying: MVar2[F, A], trans: F ~> G)
extends MVar2[G, A] {
override def isEmpty: G[Boolean] = trans(underlying.isEmpty)
override def put(a: A): G[Unit] = trans(underlying.put(a))
override def tryPut(a: A): G[Boolean] = trans(underlying.tryPut(a))
override def take: G[A] = trans(underlying.take)
override def tryTake: G[Option[A]] = trans(underlying.tryTake)
override def read: G[A] = trans(underlying.read)
override def tryRead: G[Option[A]] = trans(underlying.tryRead)
override def swap(newValue: A): G[A] = trans(underlying.swap(newValue))
}
}
22 changes: 18 additions & 4 deletions core/shared/src/main/scala/cats/effect/internals/MVarAsync.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,18 @@ package cats.effect
package internals

import java.util.concurrent.atomic.AtomicReference
import cats.effect.concurrent.MVar

import cats.effect.concurrent.MVar2
import cats.effect.internals.Callback.rightUnit

import scala.annotation.tailrec
import scala.collection.immutable.Queue

/**
* [[MVar]] implementation for [[Async]] data types.
*/
final private[effect] class MVarAsync[F[_], A] private (initial: MVarAsync.State[A])(implicit F: Async[F])
extends MVar[F, A] {
extends MVar2[F, A] {
import MVarAsync._

/** Shared mutable state. */
Expand Down Expand Up @@ -55,6 +57,18 @@ final private[effect] class MVarAsync[F[_], A] private (initial: MVarAsync.State
def read: F[A] =
F.async(unsafeRead)

def tryRead: F[Option[A]] = F.delay {
stateRef.get match {
case WaitForTake(value, _) => Some(value)
case WaitForPut(_, _) => None
}
}

def swap(newValue: A): F[A] =
F.flatMap(take) { oldValue =>
F.map(put(newValue))(_ => oldValue)
}

def isEmpty: F[Boolean] =
F.delay {
stateRef.get match {
Expand Down Expand Up @@ -226,11 +240,11 @@ final private[effect] class MVarAsync[F[_], A] private (initial: MVarAsync.State
private[effect] object MVarAsync {

/** Builds an [[MVarAsync]] instance with an `initial` value. */
def apply[F[_], A](initial: A)(implicit F: Async[F]): MVar[F, A] =
def apply[F[_], A](initial: A)(implicit F: Async[F]): MVar2[F, A] =
new MVarAsync[F, A](State(initial))

/** Returns an empty [[MVarAsync]] instance. */
def empty[F[_], A](implicit F: Async[F]): MVar[F, A] =
def empty[F[_], A](implicit F: Async[F]): MVar2[F, A] =
new MVarAsync[F, A](State.empty)

/**
Expand Down
23 changes: 19 additions & 4 deletions core/shared/src/main/scala/cats/effect/internals/MVarConcurrent.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,18 @@ package cats.effect
package internals

import java.util.concurrent.atomic.AtomicReference
import cats.effect.concurrent.MVar

import cats.effect.concurrent.MVar2
import cats.effect.internals.Callback.rightUnit

import scala.annotation.tailrec

/**
* [[MVar]] implementation for [[Concurrent]] data types.
*/
final private[effect] class MVarConcurrent[F[_], A] private (initial: MVarConcurrent.State[A])(
implicit F: Concurrent[F]
) extends MVar[F, A] {
) extends MVar2[F, A] {
import MVarConcurrent._

/** Shared mutable state. */
Expand Down Expand Up @@ -58,6 +60,14 @@ final private[effect] class MVarConcurrent[F[_], A] private (initial: MVarConcur
val read: F[A] =
F.cancelable(unsafeRead)

def tryRead =
F.delay {
stateRef.get match {
case WaitForTake(value, _) => Some(value)
case WaitForPut(_, _) => None
}
}

def isEmpty: F[Boolean] =
F.delay {
stateRef.get match {
Expand All @@ -66,6 +76,11 @@ final private[effect] class MVarConcurrent[F[_], A] private (initial: MVarConcur
}
}

def swap(newValue: A): F[A] =
F.flatMap(take) { oldValue =>
F.map(put(newValue))(_ => oldValue)
}

@tailrec private def unsafeTryPut(a: A): F[Boolean] =
stateRef.get match {
case WaitForTake(_, _) => F.pure(false)
Expand Down Expand Up @@ -280,11 +295,11 @@ final private[effect] class MVarConcurrent[F[_], A] private (initial: MVarConcur
private[effect] object MVarConcurrent {

/** Builds an [[MVarConcurrent]] instance with an `initial` value. */
def apply[F[_], A](initial: A)(implicit F: Concurrent[F]): MVar[F, A] =
def apply[F[_], A](initial: A)(implicit F: Concurrent[F]): MVar2[F, A] =
new MVarConcurrent[F, A](State(initial))

/** Returns an empty [[MVarConcurrent]] instance. */
def empty[F[_], A](implicit F: Concurrent[F]): MVar[F, A] =
def empty[F[_], A](implicit F: Concurrent[F]): MVar2[F, A] =
new MVarConcurrent[F, A](State.empty)

/**
Expand Down
Loading