Skip to content

Fix #1462: Move LiftTry as late as possible #5799

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
Jan 28, 2019
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: 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()
}
}