Restructure Try node. #202

DarkDimius · 2014-10-28T20:08:36Z

Now Try node always has cases as handlers. In case handler is an Ident of
type Throwable => T than it's desugared to a CaseDef during parsing.

DarkDimius · 2014-10-28T21:10:17Z

@odersky please review

retronym · 2014-10-29T02:17:18Z

In Scala2, handlers of Throwable => T are not allowed:

scala> try { ??? } catch { case F }
<console>:1: error: '=>' expected but '}' found.
       try { ??? } catch { case F }
                                  ^

scala> try { ??? } catch F
<console>:9: error: value isDefinedAt is not a member of Throwable => Unit
              try { ??? } catch F
                                ^

Only PartialFunction-s are supported:

scala> val F: PartialFunction[Throwable, Unit] = { case _ => }
F: PartialFunction[Throwable,Unit] = <function1>

scala> try { ??? } catch F

Perhaps this is a conscious generalisation of the existing facility, though.

retronym · 2014-10-29T02:24:33Z

I also notice that we allow an expression that is implicit convertible to that type.

scala> try { ??? } catch ""

scala> try { ??? } catch "".reverse

The spec says:

The handler h is expected conform to type scala.PartialFunction[scala.Throwable, pt]

The implementation wrongly appears to assume s/conform to/be compatible with/.

I have lodged this as https://issues.scala-lang.org/browse/SI-8945.

retronym · 2014-10-29T02:26:55Z

src/dotty/tools/dotc/parsing/Parsers.scala

+                List(CaseDef(Ident(nme.DEFAULT_EXCEPTION_NAME), EmptyTree, Apply(handler, Ident(nme.DEFAULT_EXCEPTION_NAME)))),
+              finalizer)
+          }
+


Isn't the dotty parser designed as a desugaring-free zone?

odersky · 2014-10-29T11:12:22Z

I proposed the representation in the PR, but Jason is right. It is not good that the parser does this desugaring. The parser represents naked case blocks

 { case P1 => E1 ... case Pn => En }

as Match nodes with EmptyTree as the selector. How about we use the same representation for catch blocks? I.e. a Try would be represented as follows:

 case class Try(body: Tree, handler: Tree, finalizer: Tree)

and after desugaring we would make sure that the handler is always of the form

 Match(EmptyTree, cases)

What do people think about this?

retronym · 2014-10-29T11:30:08Z

SGTM

DarkDimius · 2014-10-29T11:50:40Z

@odersky In case Try after desugaring(and typeing) should always have handlers as cases, I believe it would be better to have this enforced by types.
We already have a untyped trees that are parsed but are gone after typechecking&desugaring. I propose to have a different tree representing Try nodes before and after desugaring. See my last commit.

What do you think?

Simplifies a lot handling of Try blocks in patmat and in tailrec.

…nlined

Box& unbox methods are defined in companion objects.

It doesn't exist. Also use symbol with 'ex' name as selector for exceptions

After erasure was always wrong(didn't include the type of handler). Now it's able to use both Closures and desugared Math nodes.

Now Try node always has cases as handlers. In case handler is an Ident of type Throwable => T than it's desugared to a CaseDef during parsing.

It's not used anymore as superseded by previous commit.

Instead of desugaring in parser desugaring is now done during desugaring.

DarkDimius · 2014-10-30T18:11:51Z

@odersky should be ready for review. #206 Conflicts with this as both change typer and I would prefer to get this in first.

odersky · 2014-10-31T16:38:23Z

src/dotty/tools/dotc/Run.scala

@@ -60,7 +61,10 @@ class Run(comp: Compiler)(implicit ctx: Context) {

  private def printTree(ctx: Context) = {
    val unit = ctx.compilationUnit
-    println(s"result of $unit after ${ctx.phase.prev}:")
+    val prevPhase = ctx.phase.prev // can be a mini-phase
+    val squahsedPhase = ctx.squashed(prevPhase)


odersky · 2014-10-31T16:45:51Z

Otherwise LGTM.

Restructure Try node.

retronym reviewed Oct 29, 2014
View reviewed changes

odersky mentioned this pull request Oct 29, 2014

Transform/flatten #203

Closed

DarkDimius force-pushed the no-closure-try branch from 026cd91 to 5ec0bdf Compare October 29, 2014 11:51

DarkDimius added 15 commits October 30, 2014 17:21

Extracting TypedCases to be reused for typedTry

1be3b2f

TailRec now handles Try with Ident-handler

e44e4aa

New kind of EmptyTree for indicating exception selector in Try blocks.

09c5ad4

Simplifies a lot handling of Try blocks in patmat and in tailrec.

Typer should leave inline exception handlers inline.

5613295

Fix TailRec to handle inline exception handlers and non-trivial not-i…

f2ea8dd

…nlined

Fix Erasure.Boxing isBox and isUnbox

6fe3a2d

Box& unbox methods are defined in companion objects.

Add Patmat to Context

049bcb5

Stop patmat from using selector pos for Try.

ba4fee7

It doesn't exist. Also use symbol with 'ex' name as selector for exceptions

Flag dotty deviation for return type which is part of cake.

f60f81f

Make typing of Try nodes idempotent

d923d70

Rewrite assignType for Try

6dcd16a

After erasure was always wrong(didn't include the type of handler). Now it's able to use both Closures and desugared Math nodes.

Restructure Try node.

e07e9a3

Now Try node always has cases as handlers. In case handler is an Ident of type Throwable => T than it's desugared to a CaseDef during parsing.

Get rid of ExceptionHandlerSel.

0c967f0

It's not used anymore as superseded by previous commit.

Postponing desugaring of Try nodes.

7ebc502

Instead of desugaring in parser desugaring is now done during desugaring.

Print whole TreeTransform name in Ycheck and Xprint

9d1b4bf

DarkDimius force-pushed the no-closure-try branch from a1333a7 to 9d1b4bf Compare October 30, 2014 16:22

merge fixes: extract typedCase to be reused in TreeChecker.

5540f13

odersky reviewed Oct 31, 2014
View reviewed changes

DarkDimius added a commit that referenced this pull request Nov 3, 2014

Merge pull request #202 from dotty-staging/no-closure-try

97a3732

Restructure Try node.

DarkDimius merged commit 97a3732 into scala:master Nov 3, 2014

DarkDimius deleted the no-closure-try branch November 3, 2014 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restructure Try node. #202

Restructure Try node. #202

DarkDimius commented Oct 28, 2014

DarkDimius commented Oct 28, 2014

retronym commented Oct 29, 2014

retronym commented Oct 29, 2014

retronym Oct 29, 2014

odersky commented Oct 29, 2014

retronym commented Oct 29, 2014

DarkDimius commented Oct 29, 2014

DarkDimius commented Oct 30, 2014

odersky Oct 31, 2014

odersky commented Oct 31, 2014

Restructure Try node. #202

Restructure Try node. #202

Conversation

DarkDimius commented Oct 28, 2014

DarkDimius commented Oct 28, 2014

retronym commented Oct 29, 2014

retronym commented Oct 29, 2014

retronym Oct 29, 2014

Choose a reason for hiding this comment

odersky commented Oct 29, 2014

retronym commented Oct 29, 2014

DarkDimius commented Oct 29, 2014

DarkDimius commented Oct 30, 2014

odersky Oct 31, 2014

Choose a reason for hiding this comment

odersky commented Oct 31, 2014