From dc3fd62912ef37f1f108fbf0c4ed2a5a47b7e73e Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 29 Jul 2015 19:17:17 +1000 Subject: [backport] Cleanup code generation by avoiding redundant blocks Don't bother adding `{ ...; () }` if we can use the original tree(s) instead, e.g. if the last tree in `...` conforms to `Unit`. This makes the debug output of the macro a little easier to read. (cherry picked from commit de641dc265f34b06e17dfa7c64be00219f72b670) --- .../scala/async/internal/AsyncTransform.scala | 12 ++--- .../scala/scala/async/internal/ExprBuilder.scala | 52 ++++++++++++++++------ 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/src/main/scala/scala/async/internal/AsyncTransform.scala b/src/main/scala/scala/async/internal/AsyncTransform.scala index 47a2704..b894e61 100644 --- a/src/main/scala/scala/async/internal/AsyncTransform.scala +++ b/src/main/scala/scala/async/internal/AsyncTransform.scala @@ -68,12 +68,12 @@ trait AsyncTransform { for ((state, flds) <- assignsOf) { val assigns = flds.map { fld => val fieldSym = fld.symbol - Block( - List( - asyncBase.nullOut(global)(Expr[String](Literal(Constant(fieldSym.name.toString))), Expr[Any](Ident(fieldSym))).tree - ), - Assign(gen.mkAttributedStableRef(fieldSym.owner.thisType, fieldSym), mkZero(fieldSym.info)) - ) + val assign = Assign(gen.mkAttributedStableRef(fieldSym.owner.thisType, fieldSym), mkZero(fieldSym.info)) + asyncBase.nullOut(global)(Expr[String](Literal(Constant(fieldSym.name.toString))), Expr[Any](Ident(fieldSym))).tree + match { + case Literal(Constant(value: Unit)) => assign + case x => Block(x :: Nil, assign) + } } val asyncState = asyncBlock.asyncStates.find(_.state == state).get asyncState.stats = assigns ++ asyncState.stats diff --git a/src/main/scala/scala/async/internal/ExprBuilder.scala b/src/main/scala/scala/async/internal/ExprBuilder.scala index 2dd485d..d107ed8 100644 --- a/src/main/scala/scala/async/internal/ExprBuilder.scala +++ b/src/main/scala/scala/async/internal/ExprBuilder.scala @@ -33,8 +33,13 @@ trait ExprBuilder { var stats: List[Tree] + def statsAnd(trees: List[Tree]): List[Tree] = { + val body = adaptToUnit(stats) + Try(body, Nil, adaptToUnit(trees)) :: Nil + } + final def allStats: List[Tree] = this match { - case a: AsyncStateWithAwait => stats :+ a.awaitable.resultValDef + case a: AsyncStateWithAwait => statsAnd(a.awaitable.resultValDef :: Nil) case _ => stats } @@ -51,8 +56,9 @@ trait ExprBuilder { def nextStates: List[Int] = List(nextState) - def mkHandlerCaseForState[T: WeakTypeTag]: CaseDef = - mkHandlerCase(state, stats :+ mkStateTree(nextState, symLookup)) + def mkHandlerCaseForState[T: WeakTypeTag]: CaseDef = { + mkHandlerCase(state, statsAnd(mkStateTree(nextState, symLookup) :: Nil)) + } override val toString: String = s"AsyncState #$state, next = $nextState" @@ -86,10 +92,10 @@ trait ExprBuilder { val tryGetOrCallOnComplete = if (futureSystemOps.continueCompletedFutureOnSameThread) If(futureSystemOps.isCompleted(Expr[futureSystem.Fut[_]](awaitable.expr)).tree, - Block(ifIsFailureTree[T](futureSystemOps.getCompleted[Any](Expr[futureSystem.Fut[Any]](awaitable.expr)).tree) :: Nil, literalUnit), - Block(callOnComplete :: Nil, Return(literalUnit))) + adaptToUnit(ifIsFailureTree[T](futureSystemOps.getCompleted[Any](Expr[futureSystem.Fut[Any]](awaitable.expr)).tree) :: Nil), + Block(toList(callOnComplete), Return(literalUnit))) else - Block(callOnComplete :: Nil, Return(literalUnit)) + Block(toList(callOnComplete), Return(literalUnit)) mkHandlerCase(state, stats ++ List(mkStateTree(onCompleteState, symLookup), tryGetOrCallOnComplete)) } @@ -109,11 +115,11 @@ trait ExprBuilder { */ def ifIsFailureTree[T: WeakTypeTag](tryReference: => Tree) = If(futureSystemOps.tryyIsFailure(Expr[futureSystem.Tryy[T]](tryReference)).tree, - Block(futureSystemOps.completeProm[T]( + Block(toList(futureSystemOps.completeProm[T]( Expr[futureSystem.Prom[T]](symLookup.memberRef(name.result)), Expr[futureSystem.Tryy[T]]( TypeApply(Select(tryReference, newTermName("asInstanceOf")), - List(TypeTree(futureSystemOps.tryType[T]))))).tree :: Nil, + List(TypeTree(futureSystemOps.tryType[T]))))).tree), Return(literalUnit)), Block(List(tryGetTree(tryReference)), mkStateTree(nextState, symLookup)) ) @@ -381,12 +387,12 @@ trait ExprBuilder { val t = Expr[Throwable](Ident(name.t)) val complete = futureSystemOps.completeProm[T]( Expr[futureSystem.Prom[T]](symLookup.memberRef(name.result)), futureSystemOps.tryyFailure[T](t)).tree - Block(complete :: Nil, Return(literalUnit)) + Block(toList(complete), Return(literalUnit)) })), EmptyTree) def forever(t: Tree): Tree = { val labelName = name.fresh("while$") - LabelDef(labelName, Nil, Block(t :: Nil, Apply(Ident(labelName), Nil))) + LabelDef(labelName, Nil, Block(toList(t), Apply(Ident(labelName), Nil))) } /** @@ -404,7 +410,7 @@ trait ExprBuilder { def onCompleteHandler[T: WeakTypeTag]: Tree = { val onCompletes = initStates.flatMap(_.mkOnCompleteHandler[T]).toList forever { - Block(resumeFunTree :: Nil, literalUnit) + adaptToUnit(toList(resumeFunTree)) } } } @@ -421,12 +427,32 @@ trait ExprBuilder { Assign(symLookup.memberRef(name.state), Literal(Constant(nextState))) private def mkHandlerCase(num: Int, rhs: List[Tree]): CaseDef = - mkHandlerCase(num, Block(rhs, literalUnit)) + mkHandlerCase(num, adaptToUnit(rhs)) + + private def tpeOf(t: Tree): Type = t match { + case _ if t.tpe != null => t.tpe + case Try(body, Nil, _) => tpeOf(body) + case _ => NoType + } + + private def adaptToUnit(rhs: List[Tree]): Block = { + rhs match { + case init :+ last if tpeOf(last) <:< definitions.UnitTpe => + Block(init, last) + case _ => + Block(rhs, literalUnit) + } + } private def mkHandlerCase(num: Int, rhs: Tree): CaseDef = CaseDef(Literal(Constant(num)), EmptyTree, rhs) - def literalUnit = Literal(Constant(())) + def literalUnit = Literal(Constant(())) // a def to avoid sharing trees + + def toList(tree: Tree): List[Tree] = tree match { + case Block(stats, Literal(Constant(value))) if value == () => stats + case _ => tree :: Nil + } def literalNull = Literal(Constant(null)) } -- cgit v1.2.3 From a23cfa3d3359aa5ee4e72de340344b6b1b15c3d0 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 29 Jul 2015 19:22:00 +1000 Subject: [backport] Avoid dead code warnings in generated code. If we blindly splicing `{..$stats, ..$generatedCode}`, and the last expression in `$stats` is of type `Nothing`, we'll incur a dead code warning when typechecking the block. This commit: - introduces a helper method to augment user-written stats with synthetic code - Emit a try/finally in that code (so we advance the state, even if we are about to exit the state machine in the async-block global exception handler - Hide `Nothing` typed expressions from the dead code analysis by wrapping them in an `expr: Any` Fixes #150 (the part reported in the comments, not the original ticket.) --- .../scala/scala/async/internal/ExprBuilder.scala | 7 +++++- src/test/scala/scala/async/package.scala | 26 +++++++++++++++++++ src/test/scala/scala/async/run/WarningsSpec.scala | 29 +++++++++++++++++++--- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/main/scala/scala/async/internal/ExprBuilder.scala b/src/main/scala/scala/async/internal/ExprBuilder.scala index d107ed8..ae20642 100644 --- a/src/main/scala/scala/async/internal/ExprBuilder.scala +++ b/src/main/scala/scala/async/internal/ExprBuilder.scala @@ -34,7 +34,12 @@ trait ExprBuilder { var stats: List[Tree] def statsAnd(trees: List[Tree]): List[Tree] = { - val body = adaptToUnit(stats) + val body = stats match { + case init :+ last if tpeOf(last) =:= definitions.NothingTpe => + adaptToUnit(init :+ Typed(last, TypeTree(definitions.AnyTpe))) + case _ => + adaptToUnit(stats) + } Try(body, Nil, adaptToUnit(trees)) :: Nil } diff --git a/src/test/scala/scala/async/package.scala b/src/test/scala/scala/async/package.scala index 974a989..31f3d09 100644 --- a/src/test/scala/scala/async/package.scala +++ b/src/test/scala/scala/async/package.scala @@ -42,6 +42,32 @@ package object async { m.mkToolBox(options = compileOptions) } + import scala.tools.nsc._, reporters._ + def mkGlobal(compileOptions: String = ""): Global = { + val source = """ + | class Test { + | def test = { + | import scala.async.Async._, scala.concurrent._, ExecutionContext.Implicits.global + | async { + | val opt = await(async(Option.empty[String => Future[Unit]])) + | opt match { + | case None => + | throw new RuntimeException("case a") + | case Some(f) => + | await(f("case b")) + | } + | } + | } + | } + | """.stripMargin + val settings = new Settings() + settings.processArgumentString(compileOptions) + settings.usejavacp.value = true + settings.embeddedDefaults(getClass.getClassLoader) + val reporter = new StoreReporter + new Global(settings, reporter) + } + def scalaBinaryVersion: String = { val PreReleasePattern = """.*-(M|RC).*""".r val Pattern = """(\d+\.\d+)\..*""".r diff --git a/src/test/scala/scala/async/run/WarningsSpec.scala b/src/test/scala/scala/async/run/WarningsSpec.scala index 3a7843a..f0b414a 100644 --- a/src/test/scala/scala/async/run/WarningsSpec.scala +++ b/src/test/scala/scala/async/run/WarningsSpec.scala @@ -7,10 +7,8 @@ package run import org.junit.Test -import scala.async.internal.AsyncId -import scala.concurrent.Await -import scala.concurrent.duration._ import scala.language.{postfixOps, reflectiveCalls} +import scala.tools.nsc.reporters.StoreReporter class WarningsSpec { @@ -32,4 +30,29 @@ class WarningsSpec { """.stripMargin }) } + + @Test + def noDeadCodeWarning() { + val global = mkGlobal("-cp ${toolboxClasspath} -Yrangepos -Ywarn-dead-code -Xfatal-warnings") + val source = """ + | class Test { + | def test = { + | import scala.async.Async._, scala.concurrent._, ExecutionContext.Implicits.global + | async { + | val opt = await(async(Option.empty[String => Future[Unit]])) + | opt match { + | case None => + | throw new RuntimeException("case a") + | case Some(f) => + | await(f("case b")) + | } + | } + | } + |} + """.stripMargin + val run = new global.Run + val sourceFile = global.newSourceFile(source) + run.compileSources(sourceFile :: Nil) + assert(!global.reporter.hasErrors, global.reporter.asInstanceOf[StoreReporter].infos) + } } -- cgit v1.2.3 From 3654f2225f81273f1e14d89e3def8638f99e457d Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 29 Jul 2015 19:36:36 +1000 Subject: [backport] Avoid dead code warning with async(throw T) By declararing the parameter of `async` as by-name. Fixes #150 (the bug in the original ticket.) (cherry picked from commit 4b1dbeef9ec73612867afc5dd9c925faa8cbc30d) --- src/main/scala/scala/async/Async.scala | 2 +- src/main/scala/scala/async/internal/AsyncId.scala | 2 +- src/test/scala/scala/async/run/WarningsSpec.scala | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/main/scala/scala/async/Async.scala b/src/main/scala/scala/async/Async.scala index 17a63a4..5e2d62d 100644 --- a/src/main/scala/scala/async/Async.scala +++ b/src/main/scala/scala/async/Async.scala @@ -42,7 +42,7 @@ object Async { * Run the block of code `body` asynchronously. `body` may contain calls to `await` when the results of * a `Future` are needed; this is translated into non-blocking code. */ - def async[T](body: T)(implicit execContext: ExecutionContext): Future[T] = macro internal.ScalaConcurrentAsync.asyncImpl[T] + def async[T](body: => T)(implicit execContext: ExecutionContext): Future[T] = macro internal.ScalaConcurrentAsync.asyncImpl[T] /** * Non-blocking await the on result of `awaitable`. This may only be used directly within an enclosing `async` block. diff --git a/src/main/scala/scala/async/internal/AsyncId.scala b/src/main/scala/scala/async/internal/AsyncId.scala index f9d8c0b..c92ebf4 100644 --- a/src/main/scala/scala/async/internal/AsyncId.scala +++ b/src/main/scala/scala/async/internal/AsyncId.scala @@ -13,7 +13,7 @@ object AsyncId extends AsyncBase { lazy val futureSystem = IdentityFutureSystem type FS = IdentityFutureSystem.type - def async[T](body: T) = macro asyncIdImpl[T] + def async[T](body: => T) = macro asyncIdImpl[T] def asyncIdImpl[T: c.WeakTypeTag](c: Context)(body: c.Expr[T]): c.Expr[T] = asyncImpl[T](c)(body)(c.literalUnit) } diff --git a/src/test/scala/scala/async/run/WarningsSpec.scala b/src/test/scala/scala/async/run/WarningsSpec.scala index f0b414a..3b16899 100644 --- a/src/test/scala/scala/async/run/WarningsSpec.scala +++ b/src/test/scala/scala/async/run/WarningsSpec.scala @@ -31,6 +31,25 @@ class WarningsSpec { }) } + @Test + // https://github.com/scala/async/issues/74 + def noDeadCodeWarningForAsyncThrow() { + val global = mkGlobal("-cp ${toolboxClasspath} -Yrangepos -Ywarn-dead-code -Xfatal-warnings") + // was: "a pure expression does nothing in statement position; you may be omitting necessary parentheses" + val source = + """ + | class Test { + | import scala.async.Async._ + | import scala.concurrent.ExecutionContext.Implicits.global + | async { throw new Error() } + | } + """.stripMargin + val run = new global.Run + val sourceFile = global.newSourceFile(source) + run.compileSources(sourceFile :: Nil) + assert(!global.reporter.hasErrors, global.reporter.asInstanceOf[StoreReporter].infos) + } + @Test def noDeadCodeWarning() { val global = mkGlobal("-cp ${toolboxClasspath} -Yrangepos -Ywarn-dead-code -Xfatal-warnings") -- cgit v1.2.3 From 62bb8cfd59818c3a6923ad292c22bc805257ae89 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 29 Jul 2015 19:37:49 +1000 Subject: [backport] Stop test compiler before code generation This avoids leaving .class files in the working directory after running the test. (cherry picked from commit 5bb93b0b7357259eb588437a45063bf43595028a) --- src/test/scala/scala/async/run/WarningsSpec.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/scala/scala/async/run/WarningsSpec.scala b/src/test/scala/scala/async/run/WarningsSpec.scala index 3b16899..00c6466 100644 --- a/src/test/scala/scala/async/run/WarningsSpec.scala +++ b/src/test/scala/scala/async/run/WarningsSpec.scala @@ -34,7 +34,7 @@ class WarningsSpec { @Test // https://github.com/scala/async/issues/74 def noDeadCodeWarningForAsyncThrow() { - val global = mkGlobal("-cp ${toolboxClasspath} -Yrangepos -Ywarn-dead-code -Xfatal-warnings") + val global = mkGlobal("-cp ${toolboxClasspath} -Yrangepos -Ywarn-dead-code -Xfatal-warnings -Ystop-after:refchecks") // was: "a pure expression does nothing in statement position; you may be omitting necessary parentheses" val source = """ @@ -51,8 +51,8 @@ class WarningsSpec { } @Test - def noDeadCodeWarning() { - val global = mkGlobal("-cp ${toolboxClasspath} -Yrangepos -Ywarn-dead-code -Xfatal-warnings") + def noDeadCodeWarningInMacroExpansion() { + val global = mkGlobal("-cp ${toolboxClasspath} -Yrangepos -Ywarn-dead-code -Xfatal-warnings -Ystop-after:refchecks") val source = """ | class Test { | def test = { -- cgit v1.2.3 From f9e170e3187f83c5d00e9ea8128fe2edf2bd371c Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 30 Jul 2015 14:45:37 +1000 Subject: [backport] Make nsc.Global based tests work under SBT And remove unused code. (cherry picked from commit 7238bc1982cb1d87157c650115a2ae92a58430c9) --- src/test/scala/scala/async/package.scala | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/test/scala/scala/async/package.scala b/src/test/scala/scala/async/package.scala index 31f3d09..b355825 100644 --- a/src/test/scala/scala/async/package.scala +++ b/src/test/scala/scala/async/package.scala @@ -44,26 +44,12 @@ package object async { import scala.tools.nsc._, reporters._ def mkGlobal(compileOptions: String = ""): Global = { - val source = """ - | class Test { - | def test = { - | import scala.async.Async._, scala.concurrent._, ExecutionContext.Implicits.global - | async { - | val opt = await(async(Option.empty[String => Future[Unit]])) - | opt match { - | case None => - | throw new RuntimeException("case a") - | case Some(f) => - | await(f("case b")) - | } - | } - | } - | } - | """.stripMargin val settings = new Settings() settings.processArgumentString(compileOptions) - settings.usejavacp.value = true + val initClassPath = settings.classpath.value settings.embeddedDefaults(getClass.getClassLoader) + if (initClassPath == settings.classpath.value) + settings.usejavacp.value = true // not running under SBT, try to use the Java claspath instead val reporter = new StoreReporter new Global(settings, reporter) } -- cgit v1.2.3