From bbd8084ea988a1467ee1d7458c6b0bb10fc3c9bd Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 5 Feb 2021 12:07:22 +0100 Subject: [PATCH 1/3] Fix #9740: harden type checking for pattern match on objects We enforce that when pattern match on an object, the object should be a subtype of the scrutinee type. Reasons for doing so: - such code patterns usually implies hidden errors in the code - it's always safe/sound to reject the code We could check whether `equals` is overridden in the object, but - it complicates the protocol - overriding `equals` of object is also a bad practice - there is no sign that the slightly improved completeness is useful --- .../src/dotty/tools/dotc/typer/Typer.scala | 19 ++++++++++++++----- tests/neg/i9740.scala | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 tests/neg/i9740.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 059c4f83f64c..144259645701 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3780,11 +3780,20 @@ class Typer extends Namer withMode(Mode.GadtConstraintInference) { TypeComparer.constrainPatternType(tree.tpe, pt) } - val cmp = - untpd.Apply( - untpd.Select(untpd.TypedSplice(tree), nme.EQ), - untpd.TypedSplice(dummyTreeOfType(pt))) - typedExpr(cmp, defn.BooleanType) + + if tree.symbol.is(Module) && !(tree.tpe <:< pt) then + // We could check whether `equals` is overriden. + // Reasons for not doing so: + // - it complicates the protocol + // - such code patterns usually implies hidden errors in the code + // - it's safe/sound to reject the code + report.error(TypeMismatch(tree.tpe, pt, "\npattern type is incompatible with expected type"), tree.srcPos) + else + val cmp = + untpd.Apply( + untpd.Select(untpd.TypedSplice(tree), nme.EQ), + untpd.TypedSplice(dummyTreeOfType(pt))) + typedExpr(cmp, defn.BooleanType) case _ => private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol)(using Context): Unit = diff --git a/tests/neg/i9740.scala b/tests/neg/i9740.scala new file mode 100644 index 000000000000..2f342977ef5d --- /dev/null +++ b/tests/neg/i9740.scala @@ -0,0 +1,16 @@ +abstract class RecoveryCompleted +object RecoveryCompleted extends RecoveryCompleted + +abstract class TypedRecoveryCompleted +object TypedRecoveryCompleted extends TypedRecoveryCompleted + +class Test { + TypedRecoveryCompleted match { + case RecoveryCompleted => println("Recovery completed") // error + case TypedRecoveryCompleted => println("Typed recovery completed") + } + + def foo(x: TypedRecoveryCompleted) = x match + case RecoveryCompleted => // error + case TypedRecoveryCompleted => +} From ccc02581f77eda56f0206ff3e6e9ba0289e70995 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 5 Feb 2021 13:59:13 +0100 Subject: [PATCH 2/3] Widen type params in checking whether an object is matchable --- .../src/dotty/tools/dotc/typer/Typer.scala | 14 +++++++- tests/neg/i5077.scala | 32 +++++++++++++++++++ tests/pos/autoTuplingTest.scala | 2 +- tests/pos/i7516.scala | 2 +- tests/pos/i9740b.scala | 11 +++++++ tests/run/i5077.check | 1 - tests/run/i5077.scala | 13 ++++---- 7 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 tests/neg/i5077.scala create mode 100644 tests/pos/i9740b.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 144259645701..8a64fb2cf41b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3781,7 +3781,19 @@ class Typer extends Namer TypeComparer.constrainPatternType(tree.tpe, pt) } - if tree.symbol.is(Module) && !(tree.tpe <:< pt) then + // approximate type params with bounds + def approx = new ApproximatingTypeMap { + def apply(tp: Type) = tp.dealias match + case tp: TypeRef if !tp.symbol.isClass => + expandBounds(tp.info.bounds) + case _ => + mapOver(tp) + } + + if tree.symbol.is(Module) + && !(tree.tpe frozen_<:< pt) // fast track + && !(tree.tpe frozen_<:< approx(pt)) + then // We could check whether `equals` is overriden. // Reasons for not doing so: // - it complicates the protocol diff --git a/tests/neg/i5077.scala b/tests/neg/i5077.scala new file mode 100644 index 000000000000..bcae42b6a5c5 --- /dev/null +++ b/tests/neg/i5077.scala @@ -0,0 +1,32 @@ +trait Is[A] +case object IsInt extends Is[Int] +case object IsString extends Is[String] +case class C[A](is: Is[A], value: A) + +@main +def Test = { + val c_string: C[String] = C(IsString, "name") + val c_any: C[_] = c_string + val any: Any = c_string + + // Case 1: error + c_string match { + case C(IsInt, _) => println(s"An Int") // error + case C(IsString, s) => println(s"A String with length ${s.length}") + case _ => println("No match") + } + + // Case 2: Should match the second case and print the length of the string + c_any match { + case C(IsInt, i) if i < 10 => println(s"An Int less than 10") + case C(IsString, s) => println(s"A String with length ${s.length}") + case _ => println("No match") + } + + // Case 3: Same as above; should match the second case and print the length of the string + any match { + case C(IsInt, i) if i < 10 => println(s"An Int less than 10") + case C(IsString, s) => println(s"A String with length ${s.length}") + case _ => println("No match") + } +} \ No newline at end of file diff --git a/tests/pos/autoTuplingTest.scala b/tests/pos/autoTuplingTest.scala index 523411a1a410..389bbe6b1017 100644 --- a/tests/pos/autoTuplingTest.scala +++ b/tests/pos/autoTuplingTest.scala @@ -1,6 +1,6 @@ object autoTupling { - val x = Some(1, 2) + val x: Option[(Int, Int)] = Some(1, 2) x match { case Some(a, b) => a + b diff --git a/tests/pos/i7516.scala b/tests/pos/i7516.scala index 810474cd825b..c74567dff918 100644 --- a/tests/pos/i7516.scala +++ b/tests/pos/i7516.scala @@ -1,3 +1,3 @@ -val foo: Int => Int = Some(7) match +val foo: Int => Int = Option(7) match case Some(y) => x => y case None => identity[Int] diff --git a/tests/pos/i9740b.scala b/tests/pos/i9740b.scala new file mode 100644 index 000000000000..412e8a95dc27 --- /dev/null +++ b/tests/pos/i9740b.scala @@ -0,0 +1,11 @@ +sealed trait Exp[T] +case class IntExp(x: Int) extends Exp[Int] +case class StrExp(x: String) extends Exp[String] +object UnitExp extends Exp[Unit] + +class Foo[U <: Int, T <: U] { + def bar[A <: T](x: Exp[A]): Unit = x match + case IntExp(x) => + case StrExp(x) => + case UnitExp => +} \ No newline at end of file diff --git a/tests/run/i5077.check b/tests/run/i5077.check index 71fb2bf10e1c..5553bb1de1e9 100644 --- a/tests/run/i5077.check +++ b/tests/run/i5077.check @@ -1,3 +1,2 @@ A String with length 4 A String with length 4 -A String with length 4 diff --git a/tests/run/i5077.scala b/tests/run/i5077.scala index bf70cb0c0d19..44fd2cb7bf47 100644 --- a/tests/run/i5077.scala +++ b/tests/run/i5077.scala @@ -9,13 +9,12 @@ def Test = { val c_any: C[_] = c_string val any: Any = c_string - // Case 1: no error - // `IsInt.equals` might be overridden to match a value of `C[String]` - c_string match { - case C(IsInt, _) => println(s"An Int") // Can't possibly happen! - case C(IsString, s) => println(s"A String with length ${s.length}") - case _ => println("No match") - } + // Case 1: error, tested in tests/neg/i5077.scala + // c_string match { + // case C(IsInt, _) => println(s"An Int") // Can't possibly happen! + // case C(IsString, s) => println(s"A String with length ${s.length}") + // case _ => println("No match") + // } // Case 2: Should match the second case and print the length of the string c_any match { From b04150c708ddb1f67433e72d67bc768820096094 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 9 Feb 2021 15:35:16 +0100 Subject: [PATCH 3/3] Add test about F-Bounds --- tests/pos/i9740c.scala | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 tests/pos/i9740c.scala diff --git a/tests/pos/i9740c.scala b/tests/pos/i9740c.scala new file mode 100644 index 000000000000..968355711e19 --- /dev/null +++ b/tests/pos/i9740c.scala @@ -0,0 +1,16 @@ +sealed trait Exp[T] +case class IntExp(x: Int) extends Exp[Int] +case class StrExp(x: String) extends Exp[String] +object UnitExp extends Exp[Unit] + +trait Txn[T <: Txn[T]] +case class Obj(o: AnyRef) extends Txn[Obj] with Exp[AnyRef] + + +class Foo { + def bar[A <: Txn[A]](x: Exp[A]): Unit = x match + case IntExp(x) => + case StrExp(x) => + case UnitExp => + case Obj(o) => +}