Scala Programming Language
  1. Scala Programming Language
  2. SI-1672

Tail calls within catch blocks not optimized

    Details

      Description

      scala> def foo = {
           |   def bar : Nothing = {
           |     try {
           |  throw new RuntimeException 
           |     } catch {
           |         case _ => bar
           |    }
           |   }
           |   bar
           | }
      foo: Nothing
      
      scala> foo
      java.lang.StackOverflowError
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9)
      	at .bar$$1(<console>:9...
      scala> 
      
      1. tail-recurse-catch.check
        0.7 kB
        Jira Admin
      2. tail-recurse-catch.scala
        0.4 kB
        Jira Admin

        Activity

        Hide
        Erik Engbrecht added a comment -

        I'll attach a test case for partest tomorrow...

        Show
        Erik Engbrecht added a comment - I'll attach a test case for partest tomorrow...
        Hide
        Iulian Dragos added a comment -

        This needs a bit more thinking, it turns out such recursive calls /were/ optimized some time ago, but a later commit 'fixed' it by skipping them. I'll need to find out why the exception was added.

        Show
        Iulian Dragos added a comment - This needs a bit more thinking, it turns out such recursive calls /were/ optimized some time ago, but a later commit 'fixed' it by skipping them. I'll need to find out why the exception was added.
        Hide
        Iulian Dragos added a comment -

        I think this is safe as long as there is no `finally`. Will do.

        Show
        Iulian Dragos added a comment - I think this is safe as long as there is no `finally`. Will do.
        Show
        Jason Zaugg added a comment - https://github.com/scala/scala/pull/1711
        Hide
        Erik Allik added a comment - - edited

        Should this be handled by the fix?

          @tailrec
          def retryOn[T](thCls: Class[_ <: Throwable], maxTimes: Int)(block: => T): T = {
            try block
            catch {
              case t: Throwable if thCls.isInstance(t) && maxTimes > 0 =>
                retryOn(thCls, maxTimes - 1)(block)
            }
          }
        

        ...because currently I'm getting a compile error.

        It works when the guard is turned into

        case ... => if (...) ... else throw t

        Show
        Erik Allik added a comment - - edited Should this be handled by the fix? @tailrec def retryOn[T](thCls: Class[_ <: Throwable], maxTimes: Int)(block: => T): T = { try block catch { case t: Throwable if thCls.isInstance(t) && maxTimes > 0 => retryOn(thCls, maxTimes - 1)(block) } } ...because currently I'm getting a compile error. It works when the guard is turned into case ... => if (...) ... else throw t
        Hide
        Adriaan Moors added a comment -

        You cases in the catch should be restricted to type tests. More complicated patterns lead to a different compilation strategy that I'm not sure we can tailcall optimize. You can see the trees (as source code) with the -Xprint:patmat option.

        Show
        Adriaan Moors added a comment - You cases in the catch should be restricted to type tests. More complicated patterns lead to a different compilation strategy that I'm not sure we can tailcall optimize. You can see the trees (as source code) with the -Xprint:patmat option.

          People

          • Assignee:
            Jason Zaugg
            Reporter:
            Erik Engbrecht
          • Votes:
            2 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development