Skip to content

Commit

Permalink
Merge pull request scala#5799 from dotty-staging/fix-lazyvals-try
Browse files Browse the repository at this point in the history
Fix scala#1462: Move LiftTry as late as possible
  • Loading branch information
smarter authored Jan 28, 2019
2 parents 818fdbc + 3cc6e21 commit 36740bf
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 13 deletions.
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class Compiler {
new ExtensionMethods, // Expand methods of value classes with extension methods
new ShortcutImplicits, // Allow implicit functions without creating closures
new ByNameClosures, // Expand arguments to by-name parameters to closures
new LiftTry, // Put try expressions that might execute on non-empty stacks into their own methods
new HoistSuperArgs, // Hoist complex arguments of supercalls to enclosing scope
new ClassOf, // Expand `Predef.classOf` calls.
new RefChecks) :: // Various checks mostly related to abstract members and overriding
Expand Down Expand Up @@ -97,9 +96,10 @@ class Compiler {
List(new Constructors, // Collect initialization code in primary constructors
// Note: constructors changes decls in transformTemplate, no InfoTransformers should be added after it
new FunctionalInterfaces, // Rewrites closures to implement @specialized types of Functions.
new Instrumentation, // Count closure allocations under -Yinstrument-closures
new GetClass) :: // Rewrites getClass calls on primitive types.
List(new LinkScala2Impls, // Redirect calls to trait methods defined by Scala 2.x, so that they now go to their implementations
new Instrumentation, // Count closure allocations under -Yinstrument-closures
new GetClass, // Rewrites getClass calls on primitive types.
new LiftTry) :: // Put try expressions that might execute on non-empty stacks into their own methods their implementations
List(new LinkScala2Impls, // Redirect calls to trait methods defined by Scala 2.x, so that they now go to
new LambdaLift, // Lifts out nested functions to class scope, storing free variables in environments
// Note: in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here
new ElimStaticThis) :: // Replace `this` references to static objects by global identifiers
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/LazyVals.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
class OffsetInfo(var defs: List[Tree], var ord:Int)
private[this] val appendOffsetDefs = mutable.Map.empty[Symbol, OffsetInfo]

override def phaseName: String = "lazyVals"
override def phaseName: String = LazyVals.name

/** List of names of phases that should have finished processing of tree
* before this phase starts processing same tree */
Expand Down Expand Up @@ -435,6 +435,8 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
}

object LazyVals {
val name: String = "lazyVals"

object lazyNme {
import Names.TermName
object RLazyVals {
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/LiftTry.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ import util.Store
class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase =>
import ast.tpd._

/** the following two members override abstract members in Transform */
val phaseName: String = "liftTry"

// See tests/run/t2333.scala for an example where running after the group of LazyVals matters
override def runsAfterGroupsOf: Set[String] = Set(LazyVals.name)

private var NeedLift: Store.Location[Boolean] = _
private def needLift(implicit ctx: Context): Boolean = ctx.store(NeedLift)

Expand Down
13 changes: 7 additions & 6 deletions tests/run/i1692.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class LazyNullable(a: => Int) {

private [this] val e = "E"
lazy val l4 = try e finally () // null out e

private[this] val i = "I"
// null out i even though the try ends up lifted, because the LazyVals phase runs before the LiftTry phase
lazy val l5 = try i catch { case e: Exception => () }
}

object LazyNullable2 {
Expand Down Expand Up @@ -51,9 +55,6 @@ class LazyNotNullable {
lazy val l8 = h
}

private[this] val i = "I"
// not nullable because try is lifted, so i is used outside lazy val initializer
lazy val l9 = try i catch { case e: Exception => () }
}

trait LazyTrait {
Expand Down Expand Up @@ -97,6 +98,9 @@ object Test {
assert(lz.l4 == "E")
assertNull("e")

assert(lz.l5 == "I")
assertNull("i")

assert(LazyNullable2.l0 == "A")
assert(readField("a", LazyNullable2) == null)
}
Expand Down Expand Up @@ -134,9 +138,6 @@ object Test {
assert(inner.l8 == "H")
assertNotNull("LazyNotNullable$$h") // fragile: test will break if compiler generated names change

assert(lz.l9 == "I")
assertNotNull("i")

val fromTrait = new LazyTrait {}
assert(fromTrait.l0 == "A")
assert(readField("LazyTrait$$a", fromTrait) != null) // fragile: test will break if compiler generated names change
Expand Down
2 changes: 1 addition & 1 deletion tests/pending/run/t2333.scala → tests/run/t2333.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ class A {
object Test {
def main(a: Array[String]): Unit = {
val a = new A
a.whatever
a.whatever()
}
}

0 comments on commit 36740bf

Please sign in to comment.