Merge pull request #5799 from dotty-staging/fix-lazyvals-try

Fix #1462: Move LiftTry as late as possible

Author: smarter

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -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
@@ -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

compiler/src/dotty/tools/dotc/transform/LazyVals.scala

@@ -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 */
@@ -435,6 +435,8 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
 }
 
 object LazyVals {
+  val name: String = "lazyVals"
+
   object lazyNme {
     import Names.TermName
     object RLazyVals {

compiler/src/dotty/tools/dotc/transform/LiftTry.scala

@@ -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)
 

tests/run/i1692.scala

@@ -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 {
@@ -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 {
@@ -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)
   }
@@ -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

tests/pending/run/t2333.scala renamed to tests/run/t2333.scala

@@ -11,6 +11,6 @@ class A {
 object Test {
     def main(a: Array[String]): Unit = {
         val a = new A
-        a.whatever
+        a.whatever()
     }
 }