Warn about catching non-local returns.

Almost every time someone is shooting themself in the foot
by catching a non-local return, it is apparent from the
structure of the AST that they are doing so.  Warn them.

5 files changed, 32 insertions, 5 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
index 72dd8acad7..fc61997cd5 100644
--- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala
+++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -204,6 +204,12 @@ 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 _ =>
+        }
+
         Block(List(keyDef), tryCatch)
       }
     }
diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala
index 1b4c1b2877..e92cfba1c5 100644
--- a/src/reflect/scala/reflect/internal/TreeInfo.scala
+++ b/src/reflect/scala/reflect/internal/TreeInfo.scala
@@ -402,11 +402,15 @@ abstract class TreeInfo {
   def catchesThrowable(cdef: CaseDef) = catchesAllOf(cdef, ThrowableClass.tpe)
 
   /** Does this CaseDef catch everything of a certain Type? */
-  def catchesAllOf(cdef: CaseDef, threshold: Type) =
-    isDefaultCase(cdef) || (cdef.guard.isEmpty && (unbind(cdef.pat) match {
-      case Typed(Ident(nme.WILDCARD), tpt)  => (tpt.tpe != null) && (threshold <:< tpt.tpe)
-      case _                                => false
-    }))
+  def catchesAllOf(cdef: CaseDef, threshold: Type) = {
+    def unbound(t: Tree) = t.symbol == null || t.symbol == NoSymbol
+    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 _                         => false
+    })
+  }
 
   /** Is this pattern node a catch-all or type-test pattern? */
   def isCatchCase(cdef: CaseDef) = cdef match {
diff --git a/test/files/neg/nonlocal-warning.check b/test/files/neg/nonlocal-warning.check
new file mode 100644
index 0000000000..efb3efaaa2
--- /dev/null
+++ b/test/files/neg/nonlocal-warning.check
@@ -0,0 +1,9 @@
+nonlocal-warning.scala:4: warning: This catches all Throwables. If this is really intended, 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
+  def foo(l: List[Int]): Int = {
+                               ^
+error: No warnings can be incurred under -Xfatal-warnings.
+two warnings found
+one error found
diff --git a/test/files/neg/nonlocal-warning.flags b/test/files/neg/nonlocal-warning.flags
new file mode 100644
index 0000000000..e8fb65d50c
--- /dev/null
+++ b/test/files/neg/nonlocal-warning.flags
@@ -0,0 +1 @@
+-Xfatal-warnings
\ No newline at end of file
diff --git a/test/files/neg/nonlocal-warning.scala b/test/files/neg/nonlocal-warning.scala
new file mode 100644
index 0000000000..cc98bd631a
--- /dev/null
+++ b/test/files/neg/nonlocal-warning.scala
@@ -0,0 +1,7 @@
+class Foo {
+  def foo(l: List[Int]): Int = {
+    try l foreach { _ => return 5 }
+    catch { case x => 11 }
+    22
+  }
+}