Skip to content

Commit b35eff9

Browse files
DarkDimiusDarkDimius
authored
Merge pull request #1315 from nicolasstucki/optimize-try-cases
Fix #856: Handle try/catch cases as catch cases if possible.
2 parents 7e6968c + b0ebe6a commit b35eff9

File tree

6 files changed

+297
-75
lines changed

6 files changed

+297
-75
lines changed

‎src/dotty/tools/dotc/Compiler.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ class Compiler {
5757
new TailRec, // Rewrite tail recursion to loops
5858
new LiftTry, // Put try expressions that might execute on non-empty stacks into their own methods
5959
new ClassOf), // Expand `Predef.classOf` calls.
60-
List(new PatternMatcher, // Compile pattern matches
60+
List(new TryCatchPatterns, // Compile cases in try/catch
61+
new PatternMatcher, // Compile pattern matches
6162
new ExplicitOuter, // Add accessors to outer classes from nested ones.
6263
new ExplicitSelf, // Make references to non-trivial self types explicit as casts
6364
new CrossCastAnd, // Normalize selections involving intersection types.

‎src/dotty/tools/dotc/transform/PatternMatcher.scala

Lines changed: 2 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package dotty.tools.dotc
22
package transform
33

4+
import scala.language.postfixOps
5+
46
import TreeTransforms._
57
import core.Denotations._
68
import core.SymDenotations._
@@ -53,19 +55,6 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
5355
translated.ensureConforms(tree.tpe)
5456
}
5557

56-
57-
override def transformTry(tree: tpd.Try)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = {
58-
val selector =
59-
ctx.newSymbol(ctx.owner, ctx.freshName("ex").toTermName, Flags.Synthetic | Flags.Case, defn.ThrowableType, coord = tree.pos)
60-
val sel = Ident(selector.termRef).withPos(tree.pos)
61-
val rethrow = tpd.CaseDef(EmptyTree, EmptyTree, Throw(ref(selector)))
62-
val newCases = tpd.CaseDef(
63-
Bind(selector, Underscore(selector.info).withPos(tree.pos)),
64-
EmptyTree,
65-
transformMatch(tpd.Match(sel, tree.cases ::: rethrow :: Nil)))
66-
cpy.Try(tree)(tree.expr, newCases :: Nil, tree.finalizer)
67-
}
68-
6958
class Translator(implicit ctx: Context) {
7059

7160
def translator = {
@@ -1264,27 +1253,6 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
12641253
t
12651254
}
12661255

1267-
/** Is this pattern node a catch-all or type-test pattern? */
1268-
def isCatchCase(cdef: CaseDef) = cdef match {
1269-
case CaseDef(Typed(Ident(nme.WILDCARD), tpt), EmptyTree, _) =>
1270-
isSimpleThrowable(tpt.tpe)
1271-
case CaseDef(Bind(_, Typed(Ident(nme.WILDCARD), tpt)), EmptyTree, _) =>
1272-
isSimpleThrowable(tpt.tpe)
1273-
case _ =>
1274-
isDefaultCase(cdef)
1275-
}
1276-
1277-
private def isSimpleThrowable(tp: Type)(implicit ctx: Context): Boolean = tp match {
1278-
case tp @ TypeRef(pre, _) =>
1279-
val sym = tp.symbol
1280-
(pre == NoPrefix || pre.widen.typeSymbol.isStatic) &&
1281-
(sym.derivesFrom(defn.ThrowableClass)) && /* bq */ !(sym is Flags.Trait)
1282-
case _ =>
1283-
false
1284-
}
1285-
1286-
1287-
12881256
/** Implement a pattern match by turning its cases (including the implicit failure case)
12891257
* into the corresponding (monadic) extractors, and combining them with the `orElse` combinator.
12901258
*
@@ -1335,46 +1303,6 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
13351303
Block(List(ValDef(selectorSym, sel)), combined)
13361304
}
13371305

1338-
// return list of typed CaseDefs that are supported by the backend (typed/bind/wildcard)
1339-
// we don't have a global scrutinee -- the caught exception must be bound in each of the casedefs
1340-
// there's no need to check the scrutinee for null -- "throw null" becomes "throw new NullPointerException"
1341-
// try to simplify to a type-based switch, or fall back to a catch-all case that runs a normal pattern match
1342-
// unlike translateMatch, we type our result before returning it
1343-
/*def translateTry(caseDefs: List[CaseDef], pt: Type, pos: Position): List[CaseDef] =
1344-
// if they're already simple enough to be handled by the back-end, we're done
1345-
if (caseDefs forall isCatchCase) caseDefs
1346-
else {
1347-
val swatches = { // switch-catches
1348-
val bindersAndCases = caseDefs map { caseDef =>
1349-
// generate a fresh symbol for each case, hoping we'll end up emitting a type-switch (we don't have a global scrut there)
1350-
// if we fail to emit a fine-grained switch, have to do translateCase again with a single scrutSym (TODO: uniformize substitution on treemakers so we can avoid this)
1351-
val caseScrutSym = freshSym(pos, pureType(defn.ThrowableType))
1352-
(caseScrutSym, propagateSubstitution(translateCase(caseScrutSym, pt)(caseDef), EmptySubstitution))
1353-
}
1354-
1355-
for(cases <- emitTypeSwitch(bindersAndCases, pt).toList
1356-
if cases forall isCatchCase; // must check again, since it's not guaranteed -- TODO: can we eliminate this? e.g., a type test could test for a trait or a non-trivial prefix, which are not handled by the back-end
1357-
cse <- cases) yield /*fixerUpper(matchOwner, pos)*/(cse).asInstanceOf[CaseDef]
1358-
}
1359-
1360-
val catches = if (swatches.nonEmpty) swatches else {
1361-
val scrutSym = freshSym(pos, pureType(defn.ThrowableType))
1362-
val casesNoSubstOnly = caseDefs map { caseDef => (propagateSubstitution(translateCase(scrutSym, pt)(caseDef), EmptySubstitution))}
1363-
1364-
val exSym = freshSym(pos, pureType(defn.ThrowableType), "ex")
1365-
1366-
List(
1367-
CaseDef(
1368-
Bind(exSym, Ident(??? /*nme.WILDCARD*/)), // TODO: does this need fixing upping?
1369-
EmptyTree,
1370-
combineCasesNoSubstOnly(ref(exSym), scrutSym, casesNoSubstOnly, pt, matchOwner, Some((scrut: Symbol) => Throw(ref(exSym))))
1371-
)
1372-
)
1373-
}
1374-
1375-
/*typer.typedCases(*/catches/*, defn.ThrowableType, WildcardType)*/
1376-
}*/
1377-
13781306
/** The translation of `pat if guard => body` has two aspects:
13791307
* 1) the substitution due to the variables bound by patterns
13801308
* 2) the combination of the extractor calls using `flatMap`.

‎src/dotty/tools/dotc/transform/TryCatchPatterns.scala

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package dotty.tools.dotc
2+
package transform
3+
4+
import core.Symbols._
5+
import core.StdNames._
6+
import ast.Trees._
7+
import core.Types._
8+
import dotty.tools.dotc.core.Decorators._
9+
import dotty.tools.dotc.core.Flags
10+
import dotty.tools.dotc.core.Contexts.Context
11+
import dotty.tools.dotc.transform.TreeTransforms.{MiniPhaseTransform, TransformerInfo}
12+
import dotty.tools.dotc.util.Positions.Position
13+
14+
/** Compiles the cases that can not be handled by primitive catch cases as a common pattern match.
15+
*
16+
* The following code:
17+
* ```
18+
* try { <code> }
19+
* catch {
20+
* <tryCases> // Cases that can be handled by catch
21+
* <patternMatchCases> // Cases starting with first one that can't be handled by catch
22+
* }
23+
* ```
24+
* will become:
25+
* ```
26+
* try { <code> }
27+
* catch {
28+
* <tryCases>
29+
* case e => e match {
30+
* <patternMatchCases>
31+
* }
32+
* }
33+
* ```
34+
*
35+
* Cases that are not supported include:
36+
* - Applies and unapplies
37+
* - Idents
38+
* - Alternatives
39+
* - `case _: T =>` where `T` is not `Throwable`
40+
*
41+
*/
42+
class TryCatchPatterns extends MiniPhaseTransform {
43+
import dotty.tools.dotc.ast.tpd._
44+
45+
def phaseName: String = "tryCatchPatterns"
46+
47+
override def runsAfter = Set(classOf[ElimRepeated])
48+
49+
override def checkPostCondition(tree: Tree)(implicit ctx: Context): Unit = tree match {
50+
case Try(_, cases, _) =>
51+
cases.foreach {
52+
case CaseDef(Typed(_, _), guard, _) => assert(guard.isEmpty, "Try case should not contain a guard.")
53+
case CaseDef(Bind(_, _), guard, _) => assert(guard.isEmpty, "Try case should not contain a guard.")
54+
case c =>
55+
assert(isDefaultCase(c), "Pattern in Try should be Bind, Typed or default case.")
56+
}
57+
case _ =>
58+
}
59+
60+
override def transformTry(tree: Try)(implicit ctx: Context, info: TransformerInfo): Tree = {
61+
val (tryCases, patternMatchCases) = tree.cases.span(isCatchCase)
62+
val fallbackCase = mkFallbackPatterMatchCase(patternMatchCases, tree.pos)
63+
cpy.Try(tree)(cases = tryCases ++ fallbackCase)
64+
}
65+
66+
/** Is this pattern node a catch-all or type-test pattern? */
67+
private def isCatchCase(cdef: CaseDef)(implicit ctx: Context): Boolean = cdef match {
68+
case CaseDef(Typed(Ident(nme.WILDCARD), tpt), EmptyTree, _) => isSimpleThrowable(tpt.tpe)
69+
case CaseDef(Bind(_, Typed(Ident(nme.WILDCARD), tpt)), EmptyTree, _) => isSimpleThrowable(tpt.tpe)
70+
case _ => isDefaultCase(cdef)
71+
}
72+
73+
private def isSimpleThrowable(tp: Type)(implicit ctx: Context): Boolean = tp match {
74+
case tp @ TypeRef(pre, _) =>
75+
(pre == NoPrefix || pre.widen.typeSymbol.isStatic) && // Does not require outer class check
76+
!tp.symbol.is(Flags.Trait) && // Traits not supported by JVM
77+
tp.derivesFrom(defn.ThrowableClass)
78+
case _ =>
79+
false
80+
}
81+
82+
private def mkFallbackPatterMatchCase(patternMatchCases: List[CaseDef], pos: Position)(
83+
implicit ctx: Context, info: TransformerInfo): Option[CaseDef] = {
84+
if (patternMatchCases.isEmpty) None
85+
else {
86+
val exName = ctx.freshName("ex").toTermName
87+
val fallbackSelector =
88+
ctx.newSymbol(ctx.owner, exName, Flags.Synthetic | Flags.Case, defn.ThrowableType, coord = pos)
89+
val sel = Ident(fallbackSelector.termRef).withPos(pos)
90+
val rethrow = CaseDef(EmptyTree, EmptyTree, Throw(ref(fallbackSelector)))
91+
Some(CaseDef(
92+
Bind(fallbackSelector, Underscore(fallbackSelector.info).withPos(pos)),
93+
EmptyTree,
94+
transformFollowing(Match(sel, patternMatchCases ::: rethrow :: Nil)))
95+
)
96+
}
97+
}
98+
99+
}

‎tests/neg/tryPatternMatchError.scala

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import java.io.IOException
2+
import java.lang.NullPointerException
3+
import java.lang.IllegalArgumentException
4+
5+
object IAE {
6+
def unapply(e: Exception): Option[String] =
7+
if (e.isInstanceOf[IllegalArgumentException]) Some(e.getMessage)
8+
else None
9+
}
10+
11+
object EX extends Exception
12+
13+
trait ExceptionTrait extends Exception
14+
15+
object Test {
16+
def main(args: Array[String]): Unit = {
17+
var a: Int = 1
18+
try {
19+
throw new IllegalArgumentException()
20+
} catch {
21+
case e: IOException if e.getMessage == null =>
22+
case e: NullPointerException =>
23+
case e: IndexOutOfBoundsException =>
24+
case _: NoSuchElementException =>
25+
case _: ExceptionTrait =>
26+
case _: NoSuchElementException if a <= 1 =>
27+
case _: NullPointerException | _:IOException =>
28+
case `a` => // This case should probably emmit an error
29+
case e: Int => // error
30+
case EX =>
31+
case IAE(msg) =>
32+
case e: IllegalArgumentException =>
33+
}
34+
}
35+
}

‎tests/run/tryPatternMatch.check

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
success 1
2+
success 2
3+
success 3
4+
success 4
5+
success 5
6+
success 6
7+
success 7
8+
success 8
9+
success 9.1
10+
success 9.2
11+
IllegalArgumentException: abc
12+
IllegalArgumentException
13+
NullPointerException | IOException
14+
NoSuchElementException
15+
EX
16+
InnerException
17+
NullPointerException
18+
ExceptionTrait
19+
ClassCastException
20+
TimeoutException escaped

0 commit comments

Comments
 (0)