Warn about unchecked type tests in primitive catch cases

Author: hamzaremmal

compiler/src/dotty/tools/dotc/reporting/trace.scala

@@ -106,7 +106,7 @@ trait TraceSyntax:
         finalize(trailing(res))
         res
       catch
-        case ex: runtime.NonLocalReturnControl[T] =>
+        case ex: runtime.NonLocalReturnControl[T @unchecked] =>
           finalize(trailing(ex.value))
           throw ex
         case ex: Throwable =>

compiler/src/dotty/tools/dotc/transform/Erasure.scala

@@ -823,6 +823,11 @@ object Erasure {
       }
     }
 
+    override def typedBind(tree: untpd.Bind, pt: Type)(using Context): Bind =
+      atPhase(erasurePhase):
+        checkBind(promote(tree))
+      super.typedBind(tree, pt)
+
     /** Besides normal typing, this method does uncurrying and collects parameters
      *  to anonymous functions of arity > 22.
      */

compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala

@@ -16,6 +16,7 @@ import reporting.*
 import config.Printers.{ transforms => debug }
 
 import patmat.Typ
+import dotty.tools.dotc.util.SrcPos
 
 /** This transform normalizes type tests and type casts,
  *  also replacing type tests with singleton argument type with reference equality check
@@ -358,11 +359,8 @@ object TypeTestsCasts {
         if (sym.isTypeTest) {
           val argType = tree.args.head.tpe
           val isTrusted = tree.hasAttachment(PatternMatcher.TrustedTypeTestKey)
-          val isUnchecked = expr.tpe.widenTermRefExpr.hasAnnotation(defn.UncheckedAnnot)
-          if !isTrusted && !isUnchecked then
-            val whyNot = whyUncheckable(expr.tpe, argType, tree.span)
-            if whyNot.nonEmpty then
-              report.uncheckedWarning(UncheckedTypePattern(argType, whyNot), expr.srcPos)
+          if !isTrusted then
+            checkTypePattern(expr.tpe, argType, expr.srcPos)
           transformTypeTest(expr, argType,
             flagUnrelated = enclosingInlineds.isEmpty) // if test comes from inlined code, dont't flag it even if it always false
         }
@@ -381,6 +379,19 @@ object TypeTestsCasts {
     interceptWith(expr)
   }
 
+  /** After PatternMatcher, only Bind nodes are present in simple try-catch trees
+   *  See i19013
+   */
+  def checkBind(tree: Bind)(using Context) =
+    checkTypePattern(defn.ThrowableType, tree.body.tpe, tree.srcPos)
+
+  private def checkTypePattern(exprTpe: Type, castTpe: Type, pos: SrcPos)(using Context) =
+    val isUnchecked = castTpe.widenTermRefExpr.hasAnnotation(defn.UncheckedAnnot)
+    if !isUnchecked then
+      val whyNot = whyUncheckable(exprTpe, castTpe, pos.span)
+      if whyNot.nonEmpty then
+        report.uncheckedWarning(UncheckedTypePattern(castTpe, whyNot), pos)
+
   private def effectiveClass(tp: Type)(using Context): Symbol =
     if tp.isRef(defn.PairClass) then effectiveClass(erasure(tp))
     else if tp.isRef(defn.AnyValClass) then defn.AnyClass

library/src/scala/util/control/NonLocalReturns.scala

@@ -42,7 +42,7 @@ object NonLocalReturns {
     val returner = new ReturnThrowable[T]
     try op(using returner)
     catch {
-      case ex: ReturnThrowable[T] =>
+      case ex: ReturnThrowable[T @unchecked] =>
        if (ex.eq(returner)) ex.result else throw ex
     }
   }

tests/neg/i4812.check

@@ -34,6 +34,12 @@
    |           the type test for A cannot be checked at runtime because it's a local class
    |
    | longer explanation available when compiling with `-explain`
+-- [E092] Pattern Match Error: tests/neg/i4812.scala:80:11 -------------------------------------------------------------
+80 |      case prev: A => prev.elem // error: the type test for A cannot be checked at runtime
+   |           ^
+   |           the type test for A cannot be checked at runtime because it's a local class
+   |
+   | longer explanation available when compiling with `-explain`
 -- [E092] Pattern Match Error: tests/neg/i4812.scala:96:11 -------------------------------------------------------------
 96 |      case x: B => // error: the type test for B cannot be checked at runtime
    |           ^

tests/neg/i4812.scala

@@ -77,7 +77,7 @@ object Test:
     class A(val elem: T)
     val p = prev
     (p: @unchecked) match
-      case prev: A => prev.elem
+      case prev: A => prev.elem // error: the type test for A cannot be checked at runtime
       case _       => prev = new A(x); x
 
   def test9 =

tests/pos/i19013-a.scala

@@ -0,0 +1,9 @@
+//> using options -Xfatal-warnings
+
+def handle[E <: Exception](f: => Unit): Option[E] =
+  try
+    f
+    None
+  catch case e: E @unchecked => Some(e)
+
+val r: RuntimeException = handle[RuntimeException](throw new Exception()).get

tests/pos/i19013-b.scala

@@ -0,0 +1,11 @@
+//> using options -Xfatal-warnings
+
+case class CustomException(x: Any) extends Exception("")
+
+def handle[E](f: => Unit): Option[E] =
+  try
+    f
+    None
+  catch case CustomException(e: E @unchecked ) => Some(e)
+
+val r: RuntimeException = handle[RuntimeException](throw new Exception()).get

tests/warn/i19013-a.check

@@ -0,0 +1,6 @@
+-- [E092] Pattern Match Unchecked Warning: tests/warn/i19013-a.scala:5:13 ----------------------------------------------
+5 |  catch case e: E => Some(e) // warn
+  |             ^^^^
+  |   the type test for E cannot be checked at runtime because it refers to an abstract type member or type parameter
+  |
+  | longer explanation available when compiling with `-explain`

tests/warn/i19013-a.scala

@@ -0,0 +1,7 @@
+def handle[E <: Exception](f: => Unit): Option[E] =
+  try
+    f
+    None
+  catch case e: E => Some(e) // warn
+
+val r: RuntimeException = handle[RuntimeException](throw new Exception()).get

tests/warn/i19013-b.check

@@ -0,0 +1,6 @@
+-- [E092] Pattern Match Unchecked Warning: tests/warn/i19013-b.scala:7:29 ----------------------------------------------
+7 |  catch case CustomException(e: E) => Some(e) // warn
+  |                             ^
+  |   the type test for E cannot be checked at runtime because it refers to an abstract type member or type parameter
+  |
+  | longer explanation available when compiling with `-explain`

tests/warn/i19013-b.scala

@@ -0,0 +1,9 @@
+case class CustomException(x: Any) extends Exception("")
+
+def handle[E](f: => Unit): Option[E] =
+  try
+    f
+    None
+  catch case CustomException(e: E) => Some(e) // warn
+
+val r: RuntimeException = handle[RuntimeException](throw new Exception()).get