Skip to content
Browse files

SI-6479 Don't lift try exprs in label arguments.

The new pattern matcher uses label jumps to GOTO
the next case. Uncurry treated these like regular
method arguments, and performed the liftedTree()
transformation, which ensures that try expressions
are only used in a statement position. Even try
in statement position of a block used as such an argument
are subject to the same transform.

This transform stems from the JVM limitation,
that try/catch does not leave a value on the stack.
See b194446.

This commit changes Uncurry to avoid this transform
for arguments to label jumps. This avoids needlessly
indirect code, and enables tail call elimination in
more cases.

As an example, Scala 2.10.0 transforms the last
method of the enclosed test case to:

      try {
        case <synthetic> val x1: Int = 1;
        case5(){
          if (2.==(x1))
            {
              val x2: Int = x1;
              matchEnd4({
                {
                  def liftedTree2(): Unit = try {
                    throw new scala.runtime.NonLocalReturnControl[Unit](nonLocalReturnKey1, ())
                  } catch {
                    case (e @ (_: ClassNotFoundException)) => ()
                  };
                  liftedTree2()
                };
                TailrecAfterTryCatch.this.bad()
              })
            }
          else
            case6()
        };
        case6(){
          matchEnd4(throw new MatchError(x1))
        };
        matchEnd4(x: Unit){
          x
        }
      } catch {
        case (ex @ (_: scala.runtime.NonLocalReturnControl[Unit @unchecked])) => if (ex.key().eq(nonLocalReturnKey1))
          ex.value()
        else
          throw ex
      }

After this patch:

    @scala.annotation.tailrec final def bad(): Unit = {
      case <synthetic> val x1: Int = 1;
      case5(){
        if (2.==(x1))
          {
            <synthetic> val x2: Int = x1;
            matchEnd4({
              try {
                return ()
              } catch {
                case (e @ (_: ClassNotFoundException)) => ()
              };
              TailrecAfterTryCatch.this.bad()
            })
          }
        else
          case6()
      };
      case6(){
        matchEnd4(throw new MatchError(x1))
      };
      matchEnd4(x: Unit){
        x
      }
    }
  • Loading branch information...
1 parent 5d65772 commit 9cc61f310ef5f80e598956bee20156b7bc472e84 @retronym retronym committed
Showing with 60 additions and 2 deletions.
  1. +4 −2 src/compiler/scala/tools/nsc/transform/UnCurry.scala
  2. +56 −0 test/files/pos/t6479.scala
View
6 src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -621,11 +621,13 @@ abstract class UnCurry extends InfoTransform
case Apply(fn, args) =>
if (fn.symbol == Object_synchronized && shouldBeLiftedAnyway(args.head))
transform(treeCopy.Apply(tree, fn, List(liftTree(args.head))))
- else
- withNeedLift(true) {
+ else {
+ val needLift = needTryLift || !fn.symbol.isLabel // SI-6749, no need to lift in args to label jumps.
+ withNeedLift(needLift) {
val formals = fn.tpe.paramTypes
treeCopy.Apply(tree, transform(fn), transformTrees(transformArgs(tree.pos, fn.symbol, args, formals)))
}
+ }
case Assign(Select(_, _), _) =>
withNeedLift(true) { super.transform(tree) }
View
56 test/files/pos/t6479.scala
@@ -0,0 +1,56 @@
+object TailrecAfterTryCatch {
+
+ @annotation.tailrec
+ final def good1() {
+ 1 match {
+ case 2 => {
+ try {
+ // return
+ } catch {
+ case e: ClassNotFoundException =>
+ }
+ good1()
+ }
+ }
+ }
+
+ @annotation.tailrec
+ final def good2() {
+ //1 match {
+ // case 2 => {
+ try {
+ return
+ } catch {
+ case e: ClassNotFoundException =>
+ }
+ good2()
+ // }
+ //}
+ }
+
+ @annotation.tailrec
+ final def good3() {
+ val 1 = 2
+ try {
+ return
+ } catch {
+ case e: ClassNotFoundException =>
+ }
+ good3()
+ }
+
+ @annotation.tailrec
+ final def bad() {
+ 1 match {
+ case 2 => {
+ try {
+ return
+ } catch {
+ case e: ClassNotFoundException =>
+ }
+ bad()
+ }
+ }
+ }
+
+}

0 comments on commit 9cc61f3

Please sign in to comment.
Something went wrong with that request. Please try again.