Skip to content

Fix #4866: do not lift try with no catch block #4872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/LiftTry.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ import util.Store
* is lifted to
*
* { def liftedTree$n() = try body catch handler; liftedTree$n() }
*
* However, don't lift try's without catch expressions (try-finally).
* 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
* (copied from https://github.com/scala/scala/pull/922).
*/
class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase =>
import ast.tpd._
Expand Down Expand Up @@ -58,7 +64,7 @@ class LiftTry extends MiniPhase with IdentityDenotTransformer { thisPhase =>
liftingCtx(false)

override def transformTry(tree: Try)(implicit ctx: Context): Tree =
if (needLift) {
if (needLift && tree.cases.nonEmpty) {
ctx.debuglog(i"lifting tree at ${tree.pos}, current owner = ${ctx.owner}")
val fn = ctx.newSymbol(
ctx.owner, LiftedTreeName.fresh(), Synthetic | Method,
Expand Down
13 changes: 13 additions & 0 deletions tests/run/i1692.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ class LazyNullable(a: => Int) {

private[this] val d = "D"
lazy val l3 = d + d // null out d (Scalac require single use?)

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

object LazyNullable2 {
Expand Down Expand Up @@ -47,6 +50,10 @@ class LazyNotNullable {
class Inner {
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 @@ -87,6 +94,9 @@ object Test {
assert(lz.l3 == "DD")
assertNull("d")

assert(lz.l4 == "E")
assertNull("e")

assert(LazyNullable2.l0 == "A")
assert(readField("a", LazyNullable2) == null)
}
Expand Down Expand Up @@ -124,6 +134,9 @@ 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: 2 additions & 0 deletions tests/run/i4866.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Foo #lifted: 0
FooLifted #lifted: 1
19 changes: 19 additions & 0 deletions tests/run/i4866.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Test that try-finally aren't lifted, but try-catch are.

class Foo {
val field = try 1 finally ()
}

class FooLifted {
val field = try 1 catch { case e: Exception => () } finally ()
}

object Test extends App {
def numLifted(o: Object) = {
def isLifted(m: java.lang.reflect.Method) = m.getName.startsWith("lifted")
o.getClass.getDeclaredMethods.count(isLifted)
}

println("Foo #lifted: " + numLifted(new Foo))
println("FooLifted #lifted: " + numLifted(new FooLifted))
}