Merge pull request #5423 from lrytz/sd229

Fixes for DelayedInit classes capturing values

Fix scala/scala-dev#229

5 files changed, 240 insertions, 12 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala
index 8d362f13dd..daf645fd20 100644
--- a/src/compiler/scala/tools/nsc/transform/Constructors.scala
+++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala
@@ -527,7 +527,7 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
             super.transform(tree)
           else if (canBeSupplanted(tree.symbol))
             gen.mkAttributedIdent(parameter(tree.symbol)) setPos tree.pos
-          else if (tree.symbol.outerSource == clazz)
+          else if (tree.symbol.outerSource == clazz && !isDelayedInitSubclass)
             gen.mkAttributedIdent(parameterNamed(nme.OUTER)) setPos tree.pos
           else
             super.transform(tree)
diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
index 10d9c5627b..798cfcd072 100644
--- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
+++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
@@ -104,8 +104,31 @@ abstract class LambdaLift extends InfoTransform {
     /** Buffers for lifted out classes and methods */
     private val liftedDefs = new LinkedHashMap[Symbol, List[Tree]]
 
+    val delayedInitDummies = new mutable.HashMap[Symbol, Symbol]
+
+    /**
+     * For classes capturing locals, LambdaLift uses `local.logicallyEnclosingMember` to decide
+     * whether an access to the local is re-written to the field or constructor parameter. If the
+     * access is in a constructor statement, the constructor parameter is used.
+     *
+     * For DelayedInit subclasses, constructor statements end up in the synthetic init method
+     * instead of the constructor itself, so the access should go to the field. This method changes
+     * `logicallyEnclosingMember` in this case to return a temprorary symbol corresponding to that
+     * method.
+     */
+    private def logicallyEnclosingMember(sym: Symbol): Symbol = {
+      if (sym.isLocalDummy) {
+        val enclClass = sym.enclClass
+        if (enclClass.isSubClass(DelayedInitClass))
+          delayedInitDummies.getOrElseUpdate(enclClass, enclClass.newMethod(nme.delayedInit))
+        else
+          enclClass.primaryConstructor
+      } else if (sym.isMethod || sym.isClass || sym == NoSymbol) sym
+      else logicallyEnclosingMember(sym.owner)
+    }
+
     private def isSameOwnerEnclosure(sym: Symbol) =
-      sym.owner.logicallyEnclosingMember == currentOwner.logicallyEnclosingMember
+      logicallyEnclosingMember(sym.owner) == logicallyEnclosingMember(currentOwner)
 
     /** Mark symbol `sym` as being free in `enclosure`, unless `sym`
      *  is defined in `enclosure` or there is a class between `enclosure`s owner
@@ -139,9 +162,9 @@ abstract class LambdaLift extends InfoTransform {
      */
     private def markFree(sym: Symbol, enclosure: Symbol): Boolean = {
 //      println(s"mark free: ${sym.fullLocationString} marked free in $enclosure")
-      (enclosure == sym.owner.logicallyEnclosingMember) || {
-        debuglog("%s != %s".format(enclosure, sym.owner.logicallyEnclosingMember))
-        if (enclosure.isPackageClass || !markFree(sym, enclosure.skipConstructor.owner.logicallyEnclosingMember)) false
+      (enclosure == logicallyEnclosingMember(sym.owner)) || {
+        debuglog("%s != %s".format(enclosure, logicallyEnclosingMember(sym.owner)))
+        if (enclosure.isPackageClass || !markFree(sym, logicallyEnclosingMember(enclosure.skipConstructor.owner))) false
         else {
           val ss = symSet(free, enclosure)
           if (!ss(sym)) {
@@ -184,14 +207,14 @@ abstract class LambdaLift extends InfoTransform {
             if (sym == NoSymbol) {
               assert(name == nme.WILDCARD)
             } else if (sym.isLocalToBlock) {
-              val owner = currentOwner.logicallyEnclosingMember
+              val owner = logicallyEnclosingMember(currentOwner)
               if (sym.isTerm && !sym.isMethod) markFree(sym, owner)
               else if (sym.isMethod) markCalled(sym, owner)
                 //symSet(called, owner) += sym
             }
           case Select(_, _) =>
             if (sym.isConstructor && sym.owner.isLocalToBlock)
-              markCalled(sym, currentOwner.logicallyEnclosingMember)
+              markCalled(sym, logicallyEnclosingMember(currentOwner))
           case _ =>
         }
         super.traverse(tree)
@@ -283,17 +306,18 @@ abstract class LambdaLift extends InfoTransform {
 
     private def proxy(sym: Symbol) = {
       def searchIn(enclosure: Symbol): Symbol = {
-        if (enclosure eq NoSymbol) throw new IllegalArgumentException("Could not find proxy for "+ sym.defString +" in "+ sym.ownerChain +" (currentOwner= "+ currentOwner +" )")
-        debuglog("searching for " + sym + "(" + sym.owner + ") in " + enclosure + " " + enclosure.logicallyEnclosingMember)
+        if (enclosure eq NoSymbol)
+          throw new IllegalArgumentException("Could not find proxy for "+ sym.defString +" in "+ sym.ownerChain +" (currentOwner= "+ currentOwner +" )")
+        debuglog("searching for " + sym + "(" + sym.owner + ") in " + enclosure + " " + logicallyEnclosingMember(enclosure))
 
         val proxyName = proxyNames.getOrElse(sym, sym.name)
-        val ps = (proxies get enclosure.logicallyEnclosingMember).toList.flatten find (_.name == proxyName)
+        val ps = (proxies get logicallyEnclosingMember(enclosure)).toList.flatten find (_.name == proxyName)
         ps getOrElse searchIn(enclosure.skipConstructor.owner)
       }
       debuglog("proxy %s from %s has logical enclosure %s".format(
         sym.debugLocationString,
         currentOwner.debugLocationString,
-        sym.owner.logicallyEnclosingMember.debugLocationString)
+        logicallyEnclosingMember(sym.owner).debugLocationString)
       )
 
       if (isSameOwnerEnclosure(sym)) sym
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index ac025e50ae..f870ecfc15 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -2166,7 +2166,6 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     final def logicallyEnclosingMember: Symbol =
       if (isLocalDummy) enclClass.primaryConstructor
       else if (isMethod || isClass || this == NoSymbol) this
-      else if (this == NoSymbol) { devWarningDumpStack("NoSymbol.logicallyEnclosingMember", 15); this }
       else owner.logicallyEnclosingMember
 
     /** The top-level class containing this symbol. */
diff --git a/test/files/run/t9697.check b/test/files/run/t9697.check
new file mode 100644
index 0000000000..2a4f01c14f
--- /dev/null
+++ b/test/files/run/t9697.check
@@ -0,0 +1 @@
+warning: there were 9 deprecation warnings (since 2.11.0); re-run with -deprecation for details
diff --git a/test/files/run/t9697.scala b/test/files/run/t9697.scala
new file mode 100644
index 0000000000..eb8e44f8fc
--- /dev/null
+++ b/test/files/run/t9697.scala
@@ -0,0 +1,204 @@
+object log {
+  val b = new collection.mutable.StringBuilder
+  def apply(s: Any): Unit = b.append(s)
+  def check(s: String) = {
+    val bs = b.toString
+    assert(s == bs, bs)
+    b.clear()
+  }
+}
+
+package t9697 {
+  abstract class WA extends DelayedInit {
+    override def delayedInit(x: => Unit): Unit = x
+    val waField = "4"
+  }
+
+  class C {
+    def b(s: String) = log(s)
+    val cField = "1"
+
+    {
+      val dummyLocal = "2"
+      new WA {
+        val anonField = "3"
+        b(cField)
+        b(dummyLocal)
+        b(anonField)
+        b(waField)
+      }
+    }
+  }
+}
+
+package sd229 {
+  class Broken {
+    def is(ee: AnyRef) = {
+      new Delayed {
+        log(ee)
+      }
+    }
+  }
+
+  class Delayed extends DelayedInit {
+    def delayedInit(x: => Unit): Unit = x
+  }
+}
+
+
+// already fixed in 2.11.8, crashes in 2.10.6
+package t4683a {
+  class A { log("a") }
+  class B { log("b") }
+  class Bug extends DelayedInit {
+    log("bug")
+    def foo(a: A): B = new B
+    def delayedInit(init: => Unit): Unit = init
+  }
+}
+
+// already fixed in 2.12.0-RC1, crashes in 2.11.8
+package t4683b {
+  class Entity extends DelayedInit {
+    def delayedInit(x: => Unit): Unit = x
+    
+    class Field
+    
+    protected def EntityField[T <: Entity: reflect.ClassTag] = new Field
+    
+    def find[T <: Entity: reflect.ClassTag] {
+      Nil.map(dbo => {
+        class EntityHolder extends Entity {
+          val entity = EntityField[T]
+        }
+      })
+      log("find")
+    }
+  }
+}
+
+package t4683c {
+  trait T extends DelayedInit {
+    def delayedInit(body: => Unit) = {
+      log("init")
+      body
+    }
+  }
+}
+
+package t4683d {
+  class C extends DelayedInit {
+    def delayedInit(body: => Unit): Unit = body
+  }
+  class Injector {
+    def test: Object = {
+      val name = "k"
+      class crash extends C {
+        log(name)
+      }
+      new crash()
+    }
+  }
+}
+
+package t4683e {
+  class DelayedInitTest {
+    def a = log("uh")
+    class B extends DelayedInit {
+      a
+      def delayedInit(body: => Unit): Unit = body
+    }
+  }
+}
+
+package t4683f {
+  class Foo extends DelayedInit {
+    log("fooInit")
+    def delayedInit(newBody: => Unit): Unit = {
+      log("delayedInit")
+      inits = {
+        val f = () => newBody
+        if (inits == null) {
+          log("initsNull")
+          List(f)
+        } else
+          f :: inits
+      }
+    }
+    def foo = log("foo")
+    var inits: List[() => Unit] = Nil
+  }
+  
+  class Bar extends Foo {
+    log("barInit")
+    def bar = foo
+    def newBaz: Foo = new Baz 
+    private class Baz extends Foo {
+      log("bazInit")
+      bar
+    }
+  }
+}
+
+package t4683g {
+  trait MatExpWorld { self =>
+    class T extends Runner { val expWorld: self.type = self }
+  }
+  
+  trait Runner extends DelayedInit {
+    def delayedInit(init: => Unit): Unit = init
+    val expWorld: MatExpWorld
+  }
+}
+
+
+object Test extends App {
+  new t9697.C()
+  log.check("1234")
+
+  new sd229.Broken().is("hi")
+  log.check("hi")
+
+  val a: t4683a.A = new t4683a.A
+  var b: t4683a.B = null
+  new t4683a.Bug {
+    val b = foo(a)
+  }
+  log.check("abugb")
+
+  new t4683b.Entity().find[t4683b.Entity]
+  log.check("find")
+
+  val f = (p1: Int) => new t4683c.T { log(p1) }
+  f(5)
+  log.check("init5")
+
+  new t4683d.Injector().test
+  log.check("k")
+
+  val dit = new t4683e.DelayedInitTest()
+  new dit.B()
+  log.check("uh")
+
+  val fuu = new t4683f.Foo
+  log.check("delayedInitinitsNull")
+  fuu.inits.foreach(_.apply())
+  log.check("fooInit")
+  assert(fuu.inits == Nil) // the (delayed) initializer of Foo sets the inits field to Nil
+
+  val brr = new t4683f.Bar
+  log.check("delayedInitinitsNulldelayedInit") // delayedInit is called once for each constructor
+  brr.inits.foreach(_.apply())
+  log.check("barInitfooInit")
+  assert(brr.inits == Nil)
+
+  val bzz = brr.newBaz
+  log.check("delayedInitinitsNulldelayedInit")
+  bzz.inits.foreach(_.apply())
+  log.check("bazInitfoofooInit")
+  assert(bzz.inits == Nil)
+
+  val mew = new t4683g.MatExpWorld { }
+  val mt = new mew.T
+  assert(mt.expWorld == mew)
+}