Skip to content

Warn about unchecked type tests in primitive catch cases #19206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 = exprTpe.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
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