From 93ecb23b6026799038db6bcdc85426ed567bcdbe Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Thu, 21 Sep 2017 18:05:54 +0200 Subject: [PATCH 1/9] fix #3144: emit warnings for unchecked type patterns --- .../reporting/diagnostic/ErrorMessageID.java | 1 + .../dotc/reporting/diagnostic/messages.scala | 10 ++++++++++ .../tools/dotc/transform/patmat/Space.scala | 18 ++++++++++++++---- tests/patmat/3144.check | 2 ++ tests/patmat/3144.scala | 9 +++++++++ tests/patmat/enum-HList.check | 1 + tests/patmat/enum-Tree.check | 1 + tests/patmat/t10019.check | 2 ++ tests/patmat/t10019.scala | 14 ++++++++++++++ tests/patmat/t3683.check | 1 + tests/patmat/t3683a.check | 1 + 11 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 tests/patmat/3144.check create mode 100644 tests/patmat/3144.scala create mode 100644 tests/patmat/enum-HList.check create mode 100644 tests/patmat/enum-Tree.check create mode 100644 tests/patmat/t10019.check create mode 100644 tests/patmat/t10019.scala create mode 100644 tests/patmat/t3683.check diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index 2d653d581e3c..e98659c1663c 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -99,6 +99,7 @@ public enum ErrorMessageID { MissingReturnTypeWithReturnStatementID, NoReturnFromInlineID, ReturnOutsideMethodDefinitionID, + UncheckedTypePatternID, ; public int errorNumber() { diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index e45c581cea1a..9c4207dd62b0 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -880,6 +880,16 @@ object messages { |""" } + case class UncheckedTypePattern(msg: String)(implicit ctx: Context) + extends Message(UncheckedTypePatternID) { + val kind = "Unchecked Type Pattern" + + val explanation = + hl"""|Type arguments and type refinements are erased during compile time, thus it's + |impossible to check them at run-time. + |""" + } + case class MatchCaseUnreachable()(implicit ctx: Context) extends Message(MatchCaseUnreachableID) { val kind = s"""Match ${hl"case"} Unreachable""" diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index da53a106edad..4247dc7654cb 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -7,7 +7,7 @@ import Types._ import Contexts._ import Flags._ import ast.Trees._ -import ast.{tpd, untpd} +import ast.tpd import Decorators._ import Symbols._ import StdNames._ @@ -20,6 +20,7 @@ import ProtoTypes._ import transform.SymUtils._ import reporting.diagnostic.messages._ import config.Printers.{exhaustivity => debug} +import util.Positions.Position /** Space logic for checking exhaustivity and unreachability of pattern matching * @@ -403,21 +404,30 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { else Prod(pat.tpe.stripAnnots, fun.tpe.widen, fun.symbol, pats.map(project), irrefutable(fun)) case Typed(pat @ UnApply(_, _, _), _) => project(pat) - case Typed(expr, _) => Typ(erase(expr.tpe.stripAnnots), true) + case Typed(expr, tp) => Typ(erase(expr.tpe.stripAnnots)(tp.pos), true) case _ => debug.println(s"unknown pattern: $pat") Empty } /* Erase a type binding according to erasure semantics in pattern matching */ - def erase(tp: Type): Type = tp match { + def erase(tp: Type)(implicit pos: Position): Type = tp match { case tp @ AppliedType(tycon, args) => if (tycon.isRef(defn.ArrayClass)) tp.derivedAppliedType(tycon, args.map(erase)) - else tp.derivedAppliedType(tycon, args.map(t => WildcardType)) + else { + val ignoreWarning = args.forall(p => p.typeSymbol.is(BindDefinedType) || p.isInstanceOf[TypeBounds]) + if (!ignoreWarning) + ctx.warning(UncheckedTypePattern("type arguments are not unchecked since they are eliminated by erasure"), pos) + + tp.derivedAppliedType(tycon, args.map(t => WildcardType)) + } case OrType(tp1, tp2) => OrType(erase(tp1), erase(tp2)) case AndType(tp1, tp2) => AndType(erase(tp1), erase(tp2)) + case tp: RefinedType => + ctx.warning(UncheckedTypePattern("type refinement is not unchecked since it is eliminated by erasure"), pos) + tp.derivedRefinedType(erase(tp.parent), tp.refinedName, WildcardType) case _ => tp } diff --git a/tests/patmat/3144.check b/tests/patmat/3144.check new file mode 100644 index 000000000000..ea1dd615503f --- /dev/null +++ b/tests/patmat/3144.check @@ -0,0 +1,2 @@ +2: Unchecked Type Pattern +7: Unchecked Type Pattern diff --git a/tests/patmat/3144.scala b/tests/patmat/3144.scala new file mode 100644 index 000000000000..2aea42bfaaeb --- /dev/null +++ b/tests/patmat/3144.scala @@ -0,0 +1,9 @@ +sealed trait Foo[T] +case class Bar[T](s: String) + +object Test { + def shouldError[T](foo: Foo[T]): String = + foo match { + case bar: Bar[T] => bar.s + } +} \ No newline at end of file diff --git a/tests/patmat/enum-HList.check b/tests/patmat/enum-HList.check new file mode 100644 index 000000000000..592726c00c72 --- /dev/null +++ b/tests/patmat/enum-HList.check @@ -0,0 +1 @@ +2: Unchecked Type Pattern diff --git a/tests/patmat/enum-Tree.check b/tests/patmat/enum-Tree.check new file mode 100644 index 000000000000..2b8a99d90ec2 --- /dev/null +++ b/tests/patmat/enum-Tree.check @@ -0,0 +1 @@ +8: Unchecked Type Pattern \ No newline at end of file diff --git a/tests/patmat/t10019.check b/tests/patmat/t10019.check new file mode 100644 index 000000000000..49f5b152d1fb --- /dev/null +++ b/tests/patmat/t10019.check @@ -0,0 +1,2 @@ +2: Pattern Match Exhaustivity: (List(_, _: _*), Nil), (List(_, _: _*), List(_, _, _: _*)), (Nil, List(_, _: _*)), (List(_, _, _: _*), List(_, _: _*)) +11: Pattern Match Exhaustivity: (Foo(None), Foo(_)) diff --git a/tests/patmat/t10019.scala b/tests/patmat/t10019.scala new file mode 100644 index 000000000000..b34e453b8bab --- /dev/null +++ b/tests/patmat/t10019.scala @@ -0,0 +1,14 @@ +object Bug { + def foo[T](t1: List[T], t2: List[T]) = (t1, t2) match { + case (Nil, Nil) => () + case (List(_), List(_)) => () + } +} + +object Bug2 { + sealed case class Foo(e: Option[Int]) + + def loop(s: Foo, t: Foo): Nothing = (s,t) match { + case (Foo(Some(_)), _) => ??? + } +} diff --git a/tests/patmat/t3683.check b/tests/patmat/t3683.check new file mode 100644 index 000000000000..4964e454d9d0 --- /dev/null +++ b/tests/patmat/t3683.check @@ -0,0 +1 @@ +7: Unchecked Type Pattern \ No newline at end of file diff --git a/tests/patmat/t3683a.check b/tests/patmat/t3683a.check index fdcafc5961b9..804a435f514d 100644 --- a/tests/patmat/t3683a.check +++ b/tests/patmat/t3683a.check @@ -1 +1,2 @@ +8: Unchecked Type Pattern 14: Pattern Match Exhaustivity: XX() From 287a37718599a7bb818f5476335f8a45b6334036 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Thu, 21 Sep 2017 21:33:01 +0200 Subject: [PATCH 2/9] fix typos --- compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 4247dc7654cb..3ce79f461cb6 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -417,7 +417,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { else { val ignoreWarning = args.forall(p => p.typeSymbol.is(BindDefinedType) || p.isInstanceOf[TypeBounds]) if (!ignoreWarning) - ctx.warning(UncheckedTypePattern("type arguments are not unchecked since they are eliminated by erasure"), pos) + ctx.warning(UncheckedTypePattern("type arguments are not checked since they are eliminated by erasure"), pos) tp.derivedAppliedType(tycon, args.map(t => WildcardType)) } @@ -426,7 +426,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { case AndType(tp1, tp2) => AndType(erase(tp1), erase(tp2)) case tp: RefinedType => - ctx.warning(UncheckedTypePattern("type refinement is not unchecked since it is eliminated by erasure"), pos) + ctx.warning(UncheckedTypePattern("type refinement is not checked since it is eliminated by erasure"), pos) tp.derivedRefinedType(erase(tp.parent), tp.refinedName, WildcardType) case _ => tp } From a413dfc587854a44278e53fb1c18bde11c326329 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Fri, 22 Sep 2017 12:18:34 +0200 Subject: [PATCH 3/9] respect @unchecked in type patterns --- .../dotty/tools/dotc/transform/patmat/Space.scala | 12 +++++++----- tests/patmat/3144b.scala | 6 ++++++ 2 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 tests/patmat/3144b.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 3ce79f461cb6..8524a7e06379 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -20,7 +20,6 @@ import ProtoTypes._ import transform.SymUtils._ import reporting.diagnostic.messages._ import config.Printers.{exhaustivity => debug} -import util.Positions.Position /** Space logic for checking exhaustivity and unreachability of pattern matching * @@ -404,20 +403,23 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { else Prod(pat.tpe.stripAnnots, fun.tpe.widen, fun.symbol, pats.map(project), irrefutable(fun)) case Typed(pat @ UnApply(_, _, _), _) => project(pat) - case Typed(expr, tp) => Typ(erase(expr.tpe.stripAnnots)(tp.pos), true) + case Typed(expr, tpt) => + val unchecked = expr.tpe.hasAnnotation(ctx.definitions.UncheckedAnnot) + def warn(msg: String): Unit = if (!unchecked) ctx.warning(UncheckedTypePattern(msg), tpt.pos) + Typ(erase(expr.tpe.stripAnnots)(warn), true) case _ => debug.println(s"unknown pattern: $pat") Empty } /* Erase a type binding according to erasure semantics in pattern matching */ - def erase(tp: Type)(implicit pos: Position): Type = tp match { + def erase(tp: Type)(implicit warn: String => Unit): Type = tp match { case tp @ AppliedType(tycon, args) => if (tycon.isRef(defn.ArrayClass)) tp.derivedAppliedType(tycon, args.map(erase)) else { val ignoreWarning = args.forall(p => p.typeSymbol.is(BindDefinedType) || p.isInstanceOf[TypeBounds]) if (!ignoreWarning) - ctx.warning(UncheckedTypePattern("type arguments are not checked since they are eliminated by erasure"), pos) + warn("type arguments are not checked since they are eliminated by erasure") tp.derivedAppliedType(tycon, args.map(t => WildcardType)) } @@ -426,7 +428,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { case AndType(tp1, tp2) => AndType(erase(tp1), erase(tp2)) case tp: RefinedType => - ctx.warning(UncheckedTypePattern("type refinement is not checked since it is eliminated by erasure"), pos) + warn("type refinement is not checked since it is eliminated by erasure") tp.derivedRefinedType(erase(tp.parent), tp.refinedName, WildcardType) case _ => tp } diff --git a/tests/patmat/3144b.scala b/tests/patmat/3144b.scala new file mode 100644 index 000000000000..621f84d0e340 --- /dev/null +++ b/tests/patmat/3144b.scala @@ -0,0 +1,6 @@ +class Test { + def f(x: Any): Int = x match { + case xs: List[Int] @unchecked => xs.head + case _ => 0 + } +} From 83a1e3ce266b72215f9b99c33e1917b582df1194 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Fri, 22 Sep 2017 12:23:32 +0200 Subject: [PATCH 4/9] simplify checkable --- .../dotty/tools/dotc/transform/patmat/Space.scala | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 8524a7e06379..6d78a82d3870 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -733,12 +733,10 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { } def checkable(tree: Match): Boolean = { - def isCheckable(tp: Type): Boolean = tp match { - case AnnotatedType(tp, annot) => - (ctx.definitions.UncheckedAnnot != annot.symbol) && isCheckable(tp) - case _ => - // Possible to check everything, but be compatible with scalac by default - ctx.settings.YcheckAllPatmat.value || + // Possible to check everything, but be compatible with scalac by default + def isCheckable(tp: Type): Boolean = + !tp.hasAnnotation(defn.UncheckedAnnot) && ( + ctx.settings.YcheckAllPatmat.value || tp.typeSymbol.is(Sealed) || tp.isInstanceOf[OrType] || (tp.isInstanceOf[AndType] && { @@ -749,7 +747,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { tp.typeSymbol.is(Enum) || canDecompose(tp) || (defn.isTupleType(tp) && tp.dealias.argInfos.exists(isCheckable(_))) - } + ) val Match(sel, cases) = tree val res = isCheckable(sel.tpe.widen.dealiasKeepAnnots) From 2c2c47a10ec8e18baf3100920955fa27b9f3cd75 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Fri, 22 Sep 2017 14:49:46 +0200 Subject: [PATCH 5/9] handle @unchecked inside type arguments --- compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 6 +++++- tests/patmat/3144b.scala | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 6d78a82d3870..1e6c4d29798d 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -417,7 +417,11 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { case tp @ AppliedType(tycon, args) => if (tycon.isRef(defn.ArrayClass)) tp.derivedAppliedType(tycon, args.map(erase)) else { - val ignoreWarning = args.forall(p => p.typeSymbol.is(BindDefinedType) || p.isInstanceOf[TypeBounds]) + val ignoreWarning = args.forall { p => + p.typeSymbol.is(BindDefinedType) || + p.hasAnnotation(defn.UncheckedAnnot) || + p.isInstanceOf[TypeBounds] + } if (!ignoreWarning) warn("type arguments are not checked since they are eliminated by erasure") diff --git a/tests/patmat/3144b.scala b/tests/patmat/3144b.scala index 621f84d0e340..075b69ebf766 100644 --- a/tests/patmat/3144b.scala +++ b/tests/patmat/3144b.scala @@ -1,6 +1,8 @@ class Test { def f(x: Any): Int = x match { case xs: List[Int] @unchecked => xs.head + case xs: List[Int @unchecked] => xs.head + case xs: Array[List[Int]] => 3 case _ => 0 } } From 986dab14ccaf8a3b3d5fd6040509ad02258288d5 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Fri, 22 Sep 2017 14:50:25 +0200 Subject: [PATCH 6/9] add missing check file --- tests/patmat/3144b.check | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 tests/patmat/3144b.check diff --git a/tests/patmat/3144b.check b/tests/patmat/3144b.check new file mode 100644 index 000000000000..92ef3aeaf6de --- /dev/null +++ b/tests/patmat/3144b.check @@ -0,0 +1,2 @@ +5: Unchecked Type Pattern +4: Match case Unreachable From c53d6ec35b1b16b05fd4a6e2ff79f188b0471cf0 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Fri, 22 Sep 2017 15:12:54 +0200 Subject: [PATCH 7/9] address review: update test case --- tests/patmat/3144b.check | 4 ++-- tests/patmat/3144b.scala | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/patmat/3144b.check b/tests/patmat/3144b.check index 92ef3aeaf6de..9eec9d940d08 100644 --- a/tests/patmat/3144b.check +++ b/tests/patmat/3144b.check @@ -1,2 +1,2 @@ -5: Unchecked Type Pattern -4: Match case Unreachable +4: Unchecked Type Pattern +10: Unchecked Type Pattern diff --git a/tests/patmat/3144b.scala b/tests/patmat/3144b.scala index 075b69ebf766..63445339461a 100644 --- a/tests/patmat/3144b.scala +++ b/tests/patmat/3144b.scala @@ -1,6 +1,11 @@ class Test { def f(x: Any): Int = x match { case xs: List[Int] @unchecked => xs.head + case xs: Array[List[Int]] => 3 + case _ => 0 + } + + def g(x: Any): Int = x match { case xs: List[Int @unchecked] => xs.head case xs: Array[List[Int]] => 3 case _ => 0 From daa5b84313ffbab0f64f3a12168b414cbc6527d8 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Wed, 27 Sep 2017 18:26:00 +0200 Subject: [PATCH 8/9] address reviews --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 9c4207dd62b0..6da65f77d534 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -882,11 +882,13 @@ object messages { case class UncheckedTypePattern(msg: String)(implicit ctx: Context) extends Message(UncheckedTypePatternID) { - val kind = "Unchecked Type Pattern" + val kind = "Pattern Match Exhaustivity" val explanation = hl"""|Type arguments and type refinements are erased during compile time, thus it's |impossible to check them at run-time. + | + |You can either replace the type arguments by `_` or use `@unchecked`. |""" } From 608a17c8bb76fc85c281e5f69b436dd973bf02a3 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Wed, 27 Sep 2017 22:16:09 +0200 Subject: [PATCH 9/9] update patmat check files after message change --- tests/patmat/3144.check | 4 ++-- tests/patmat/3144b.check | 4 ++-- tests/patmat/enum-HList.check | 2 +- tests/patmat/enum-Tree.check | 2 +- tests/patmat/t3683.check | 2 +- tests/patmat/t3683a.check | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/patmat/3144.check b/tests/patmat/3144.check index ea1dd615503f..207dd36e1d9a 100644 --- a/tests/patmat/3144.check +++ b/tests/patmat/3144.check @@ -1,2 +1,2 @@ -2: Unchecked Type Pattern -7: Unchecked Type Pattern +2: Pattern Match Exhaustivity +7: Pattern Match Exhaustivity diff --git a/tests/patmat/3144b.check b/tests/patmat/3144b.check index 9eec9d940d08..d4bd280466da 100644 --- a/tests/patmat/3144b.check +++ b/tests/patmat/3144b.check @@ -1,2 +1,2 @@ -4: Unchecked Type Pattern -10: Unchecked Type Pattern +4: Pattern Match Exhaustivity +10: Pattern Match Exhaustivity diff --git a/tests/patmat/enum-HList.check b/tests/patmat/enum-HList.check index 592726c00c72..414335be5ea7 100644 --- a/tests/patmat/enum-HList.check +++ b/tests/patmat/enum-HList.check @@ -1 +1 @@ -2: Unchecked Type Pattern +2: Pattern Match Exhaustivity diff --git a/tests/patmat/enum-Tree.check b/tests/patmat/enum-Tree.check index 2b8a99d90ec2..84053e20646d 100644 --- a/tests/patmat/enum-Tree.check +++ b/tests/patmat/enum-Tree.check @@ -1 +1 @@ -8: Unchecked Type Pattern \ No newline at end of file +8: Pattern Match Exhaustivity \ No newline at end of file diff --git a/tests/patmat/t3683.check b/tests/patmat/t3683.check index 4964e454d9d0..2d090c590d8c 100644 --- a/tests/patmat/t3683.check +++ b/tests/patmat/t3683.check @@ -1 +1 @@ -7: Unchecked Type Pattern \ No newline at end of file +7: Pattern Match Exhaustivity \ No newline at end of file diff --git a/tests/patmat/t3683a.check b/tests/patmat/t3683a.check index 804a435f514d..c39820a1a0be 100644 --- a/tests/patmat/t3683a.check +++ b/tests/patmat/t3683a.check @@ -1,2 +1,2 @@ -8: Unchecked Type Pattern +8: Pattern Match Exhaustivity 14: Pattern Match Exhaustivity: XX()