From dfacadd04160f242b5bedd5cc4430eaf446ebf6a Mon Sep 17 00:00:00 2001 From: Hamza Remmal <56235032+hamzaremmal@users.noreply.github.com> Date: Tue, 5 Dec 2023 18:38:44 +0100 Subject: [PATCH] Warn about unchecked type tests in primitive catch cases --- .../dotty/tools/dotc/reporting/trace.scala | 2 +- .../dotty/tools/dotc/transform/Erasure.scala | 5 +++++ .../tools/dotc/transform/TypeTestsCasts.scala | 21 ++++++++++++++----- .../scala/util/control/NonLocalReturns.scala | 2 +- tests/pos/i19013-a.scala | 9 ++++++++ tests/pos/i19013-b.scala | 11 ++++++++++ tests/warn/i19013-a.check | 6 ++++++ tests/warn/i19013-a.scala | 7 +++++++ tests/warn/i19013-b.check | 6 ++++++ tests/warn/i19013-b.scala | 9 ++++++++ 10 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 tests/pos/i19013-a.scala create mode 100644 tests/pos/i19013-b.scala create mode 100644 tests/warn/i19013-a.check create mode 100644 tests/warn/i19013-a.scala create mode 100644 tests/warn/i19013-b.check create mode 100644 tests/warn/i19013-b.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/trace.scala b/compiler/src/dotty/tools/dotc/reporting/trace.scala index 8e8d3efb8b40..fbbc3d990969 100644 --- a/compiler/src/dotty/tools/dotc/reporting/trace.scala +++ b/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 => diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index cf99711d92c2..8582420d64ee 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/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. */ diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala index bac731c5f41b..fb90ea4abfea 100644 --- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala +++ b/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 = 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 diff --git a/library/src/scala/util/control/NonLocalReturns.scala b/library/src/scala/util/control/NonLocalReturns.scala index c7e600b4c028..f59b4c22a4c2 100644 --- a/library/src/scala/util/control/NonLocalReturns.scala +++ b/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 } } diff --git a/tests/pos/i19013-a.scala b/tests/pos/i19013-a.scala new file mode 100644 index 000000000000..0a6a5e2d1179 --- /dev/null +++ b/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 diff --git a/tests/pos/i19013-b.scala b/tests/pos/i19013-b.scala new file mode 100644 index 000000000000..a4ec71654ff5 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/tests/warn/i19013-a.check b/tests/warn/i19013-a.check new file mode 100644 index 000000000000..2b5c4ada7ebf --- /dev/null +++ b/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` \ No newline at end of file diff --git a/tests/warn/i19013-a.scala b/tests/warn/i19013-a.scala new file mode 100644 index 000000000000..1c83c7173c86 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/tests/warn/i19013-b.check b/tests/warn/i19013-b.check new file mode 100644 index 000000000000..f2cee8b58615 --- /dev/null +++ b/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` diff --git a/tests/warn/i19013-b.scala b/tests/warn/i19013-b.scala new file mode 100644 index 000000000000..73d342f4f683 --- /dev/null +++ b/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