Merge pull request #922 from dragos/issue/fix-SI-6092

SI-6092 don't leak in LazyAnnotationInfo, don't lift try{}finally{}

4 files changed, 55 insertions, 6 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
index de0650b2ea..efc3d25ed0 100644
--- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala
+++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -33,6 +33,14 @@ import language.postfixOps
  *  - convert implicit method types to method types
  *  - convert non-trivial catches in try statements to matches
  *  - convert non-local returns to throws with enclosing try statements.
+ *  - convert try-catch expressions in contexts where there might be values on the stack to
+ *      a local method and a call to it (since an exception empties the evaluation stack):
+ *      
+ *      meth(x_1,..., try { x_i } catch { ..}, .. x_b0) ==> 
+ *        {
+ *          def liftedTry$1 = try { x_i } catch { .. }
+ *          meth(x_1, .., liftedTry$1(), .. )
+ *        }
  */
 /*</export> */
 abstract class UnCurry extends InfoTransform
@@ -634,6 +642,13 @@ abstract class UnCurry extends InfoTransform
           case ret @ Return(_) if (isNonLocalReturn(ret)) =>
             withNeedLift(true) { super.transform(ret) }
 
+          case Try(_, Nil, _) => 
+            // try-finally does not need lifting: lifting is needed only for try-catch
+            // expressions that are evaluated in a context where the stack might not be empty.
+            // `finally` does not attempt to continue evaluation after an exception, so the fact
+            // that values on the stack are 'lost' does not matter
+            super.transform(tree)
+
           case Try(block, catches, finalizer) =>
             if (needTryLift || shouldBeLiftedAnyway(tree)) transform(liftTree(tree))
             else super.transform(tree)
diff --git a/src/reflect/scala/reflect/internal/AnnotationInfos.scala b/src/reflect/scala/reflect/internal/AnnotationInfos.scala
index c283ae408e..2bcc95b9a8 100644
--- a/src/reflect/scala/reflect/internal/AnnotationInfos.scala
+++ b/src/reflect/scala/reflect/internal/AnnotationInfos.scala
@@ -160,12 +160,7 @@ trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable =>
    */
   final class LazyAnnotationInfo(lazyInfo: => AnnotationInfo) extends AnnotationInfo {
     private var forced = false
-    private lazy val forcedInfo =
-      try {
-        val result = lazyInfo
-        if (result.pos == NoPosition) result setPos pos
-        result
-      } finally forced = true
+    private lazy val forcedInfo = try lazyInfo finally forced = true
 
     def atp: Type                               = forcedInfo.atp
     def args: List[Tree]                        = forcedInfo.args
diff --git a/test/files/run/nullable-lazyvals.check b/test/files/run/nullable-lazyvals.check
new file mode 100644
index 0000000000..4db5783257
--- /dev/null
+++ b/test/files/run/nullable-lazyvals.check
@@ -0,0 +1,3 @@
+
+param1: null
+param2: null
diff --git a/test/files/run/nullable-lazyvals.scala b/test/files/run/nullable-lazyvals.scala
new file mode 100644
index 0000000000..c201e74e75
--- /dev/null
+++ b/test/files/run/nullable-lazyvals.scala
@@ -0,0 +1,36 @@
+
+/** Test that call-by-name parameters are set to null if
+ *  they are used only to initialize a lazy value, after the
+ *  value has been initialized.
+ */
+
+class Foo(param1: => Object, param2: => String) {
+	lazy val field1 = param1
+	lazy val field2 = try param2 finally println("")
+}
+
+object Test extends App {
+	val foo = new Foo(new Object, "abc")
+
+	foo.field1
+	foo.field2
+
+	for (f <- foo.getClass.getDeclaredFields) {
+		f.setAccessible(true)
+		if (f.getName.startsWith("param")) {
+			println("%s: %s".format(f.getName, f.get(foo)))
+		}
+	}
+
+	// test that try-finally does not generated a liftedTry
+	// helper. This would already fail the first part of the test,
+	// but this check will help diganose it (if the single access to a
+	// private field does not happen directly in the lazy val, it won't
+	// be nulled).
+	for (f <- foo.getClass.getDeclaredMethods) {
+		f.setAccessible(true)
+		if (f.getName.startsWith("lifted")) {
+			println("not expected: %s".format(f))
+		}
+	}
+}