diff options
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/UnCurry.scala | 19 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Typers.scala | 14 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/TreeInfo.scala | 28 | ||||
-rw-r--r-- | test/files/neg/catch-all.check | 9 | ||||
-rw-r--r-- | test/files/neg/nonlocal-warning.check | 3 | ||||
-rw-r--r-- | test/files/neg/nonlocal-warning.scala | 11 |
6 files changed, 53 insertions, 31 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index f68cbfc141..ea93ad1bd4 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -205,11 +205,8 @@ abstract class UnCurry extends InfoTransform val keyDef = ValDef(key, New(ObjectClass.tpe)) val tryCatch = Try(body, pat -> rhs) - body foreach { - case Try(t, catches, _) if catches exists treeInfo.catchesThrowable => - unit.warning(body.pos, "catch block may intercept non-local return from " + meth) - case _ => - } + for (Try(t, catches, _) <- body ; cdef <- catches ; if treeInfo catchesThrowable cdef) + unit.warning(body.pos, "catch block may intercept non-local return from " + meth) Block(List(keyDef), tryCatch) } @@ -691,16 +688,16 @@ abstract class UnCurry extends InfoTransform else tree } - + def isThrowable(pat: Tree): Boolean = pat match { - case Typed(Ident(nme.WILDCARD), tpt) => + case Typed(Ident(nme.WILDCARD), tpt) => tpt.tpe =:= ThrowableClass.tpe - case Bind(_, pat) => + case Bind(_, pat) => isThrowable(pat) case _ => false } - + def isDefaultCatch(cdef: CaseDef) = isThrowable(cdef.pat) && cdef.guard.isEmpty def postTransformTry(tree: Try) = { @@ -764,10 +761,10 @@ abstract class UnCurry extends InfoTransform case tree: Try => postTransformTry(tree) - + case Apply(Apply(fn, args), args1) => treeCopy.Apply(tree, fn, args ::: args1) - + case Ident(name) => assert(name != tpnme.WILDCARD_STAR, tree) applyUnary() diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index cf9a07a7e4..9c6f6a0f99 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -5133,14 +5133,12 @@ trait Typers extends Modes with Adaptations with Tags { var block1 = typed(tree.block, pt) var catches1 = typedCases(tree.catches, ThrowableClass.tpe, pt) - for (cdef <- catches1 if cdef.guard.isEmpty) { - def warn(name: Name) = context.warning(cdef.pat.pos, s"This catches all Throwables. If this is really intended, use `case ${name.decoded} : Throwable` to clear this warning.") - def unbound(t: Tree) = t.symbol == null || t.symbol == NoSymbol - cdef.pat match { - case Bind(name, i @ Ident(_)) if unbound(i) => warn(name) - case i @ Ident(name) if unbound(i) => warn(name) - case _ => - } + for (cdef <- catches1; if treeInfo catchesThrowable cdef) { + val name = (treeInfo assignedNameOfPattern cdef).decoded + context.warning(cdef.pat.pos, + s"""|This catches all Throwables, which often has undesirable consequences. + |If intentional, use `case $name : Throwable` to clear this warning.""".stripMargin + ) } val finalizer1 = diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index db062d138f..d4a22886dd 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -397,19 +397,31 @@ abstract class TreeInfo { case _ => false } - /** Does this CaseDef catch Throwable? */ - def catchesThrowable(cdef: CaseDef) = catchesAllOf(cdef, ThrowableClass.tpe) + private def hasNoSymbol(t: Tree) = t.symbol == null || t.symbol == NoSymbol - /** Does this CaseDef catch everything of a certain Type? */ - def catchesAllOf(cdef: CaseDef, threshold: Type) = { - def unbound(t: Tree) = t.symbol == null || t.symbol == NoSymbol + /** If this CaseDef assigns a name to its top-level pattern, + * in the form 'expr @ pattern' or 'expr: pattern', returns + * the name. Otherwise, nme.NO_NAME. + * + * Note: in the case of Constant patterns such as 'case x @ "" =>', + * the pattern matcher eliminates the binding and inlines the constant, + * so as far as this method is likely to be able to determine, + * the name is NO_NAME. + */ + def assignedNameOfPattern(cdef: CaseDef): Name = cdef.pat match { + case Bind(name, _) => name + case Ident(name) => name + case _ => nme.NO_NAME + } + + /** Does this CaseDef catch Throwable? */ + def catchesThrowable(cdef: CaseDef) = ( cdef.guard.isEmpty && (unbind(cdef.pat) match { case Ident(nme.WILDCARD) => true - case i@Ident(name) => unbound(i) - case Typed(_, tpt) => (tpt.tpe != null) && (threshold <:< tpt.tpe) + case i@Ident(name) => hasNoSymbol(i) case _ => false }) - } + ) /** Is this pattern node a catch-all or type-test pattern? */ def isCatchCase(cdef: CaseDef) = cdef match { diff --git a/test/files/neg/catch-all.check b/test/files/neg/catch-all.check index aaf51480c3..2d58dd99a8 100644 --- a/test/files/neg/catch-all.check +++ b/test/files/neg/catch-all.check @@ -1,10 +1,13 @@ -catch-all.scala:2: warning: This catches all Throwables. If this is really intended, use `case _ : Throwable` to clear this warning. +catch-all.scala:2: warning: This catches all Throwables, which often has undesirable consequences. +If intentional, use `case _ : Throwable` to clear this warning. try { "warn" } catch { case _ => } ^ -catch-all.scala:4: warning: This catches all Throwables. If this is really intended, use `case x : Throwable` to clear this warning. +catch-all.scala:4: warning: This catches all Throwables, which often has undesirable consequences. +If intentional, use `case x : Throwable` to clear this warning. try { "warn" } catch { case x => } ^ -catch-all.scala:6: warning: This catches all Throwables. If this is really intended, use `case x : Throwable` to clear this warning. +catch-all.scala:6: warning: This catches all Throwables, which often has undesirable consequences. +If intentional, use `case x : Throwable` to clear this warning. try { "warn" } catch { case _: RuntimeException => ; case x => } ^ error: No warnings can be incurred under -Xfatal-warnings. diff --git a/test/files/neg/nonlocal-warning.check b/test/files/neg/nonlocal-warning.check index 5202df655a..67b3b10095 100644 --- a/test/files/neg/nonlocal-warning.check +++ b/test/files/neg/nonlocal-warning.check @@ -1,4 +1,5 @@ -nonlocal-warning.scala:4: warning: This catches all Throwables. If this is really intended, use `case x : Throwable` to clear this warning. +nonlocal-warning.scala:4: warning: This catches all Throwables, which often has undesirable consequences. +If intentional, use `case x : Throwable` to clear this warning. catch { case x => 11 } ^ nonlocal-warning.scala:2: warning: catch block may intercept non-local return from method foo diff --git a/test/files/neg/nonlocal-warning.scala b/test/files/neg/nonlocal-warning.scala index cc98bd631a..f908a86302 100644 --- a/test/files/neg/nonlocal-warning.scala +++ b/test/files/neg/nonlocal-warning.scala @@ -4,4 +4,15 @@ class Foo { catch { case x => 11 } 22 } + + val pf: PartialFunction[Throwable, Unit] = { + case x if false => () + } + + def bar(l: List[Int]): Int = { + try l foreach { _ => return 5 } + catch pf + finally println() + 22 + } } |