Skip to content

Commit

Permalink
Warn about unchecked type tests in primitive catch cases
Browse files Browse the repository at this point in the history
  • Loading branch information
hamzaremmal committed Dec 5, 2023
1 parent a3854cb commit 7879bfe
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 8 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/reporting/trace.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down
5 changes: 5 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
21 changes: 16 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion library/src/scala/util/control/NonLocalReturns.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i4812.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
9 changes: 9 additions & 0 deletions tests/pos/i19013-a.scala
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions tests/pos/i19013-b.scala
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions tests/warn/i19013-a.check
Original file line number Diff line number Diff line change
@@ -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`
7 changes: 7 additions & 0 deletions tests/warn/i19013-a.scala
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions tests/warn/i19013-b.check
Original file line number Diff line number Diff line change
@@ -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`
9 changes: 9 additions & 0 deletions tests/warn/i19013-b.scala
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 7879bfe

Please sign in to comment.