From d1957e58392466223f6a17625794ad5249557cff Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Wed, 20 Oct 2021 16:13:04 +0200 Subject: [PATCH] Don't follow BaseType of abstract binders in MT reduction Fix #11982 and the associated soundness problem. The issue with the behavior on master arises from the fact that type binder of match types might change as context gets more precise, which results in a single match type reducing in two different ways. This issue comes from the fact that subtyping looks into base types, and is thus able to match a type such as `T <: Tuple2[Int, Int]` against a pattern `case Tuple2[a, b]`, even if the best solutions for `a` and `b` in the current context are not guaranteed to be the best solution in more precise contexts (such as at call site in the added test case). --- .../dotty/tools/dotc/core/TypeComparer.scala | 11 +++-- .../dotty/tools/dotc/typer/Implicits.scala | 2 +- .../dotty/tools/dotc/CompilationTests.scala | 1 + tests/neg/11982.scala | 40 +++++++++++++++++++ tests/neg/6570-1.scala | 24 ++++++++++- tests/neg/6570.scala | 6 +-- tests/pos/11982-a/119_1.scala | 9 +++++ tests/pos/11982-a/119_2.scala | 5 +++ tests/pos/13491.scala | 3 +- .../typeclass-derivation1.scala | 12 ++++++ 10 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 tests/neg/11982.scala create mode 100644 tests/pos/11982-a/119_1.scala create mode 100644 tests/pos/11982-a/119_2.scala rename tests/{run => run-custom-args}/typeclass-derivation1.scala (89%) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 67cfb12d67be..70d0a400aac3 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -744,8 +744,13 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling } def tryBaseType(cls2: Symbol) = { + val allowBaseType = caseLambda.eq(NoType) || (tp1 match { + case tp: TypeRef if tp.symbol.isClass => true + case AppliedType(tycon: TypeRef, _) if tycon.symbol.isClass => true + case _ => false + }) val base = nonExprBaseType(tp1, cls2) - if (base.exists && (base `ne` tp1)) + if (base.exists && base.ne(tp1) && allowBaseType) isSubType(base, tp2, if (tp1.isRef(cls2)) approx else approx.addLow) || base.isInstanceOf[OrType] && fourthTry // if base is a disjunction, this might have come from a tp1 type that @@ -764,7 +769,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling || narrowGADTBounds(tp1, tp2, approx, isUpper = true)) && (tp2.isAny || GADTusage(tp1.symbol)) - isSubType(hi1, tp2, approx.addLow) || compareGADT || tryLiftedToThis1 + caseLambda.eq(NoType) && isSubType(hi1, tp2, approx.addLow) || compareGADT || tryLiftedToThis1 case _ => // `Mode.RelaxedOverriding` is only enabled when checking Java overriding // in explicit nulls, and `Null` becomes a bottom type, which allows @@ -2536,7 +2541,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling override def apply(x: Boolean, t: Type) = x && { t match { - case tp: TypeRef if tp.symbol.isAbstractOrParamType => false + case tp: TypeRef if !tp.symbol.isClass => false case _: SkolemType | _: TypeVar | _: TypeParamRef => false case _ => foldOver(x, t) } diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 4fae9264939d..11f942df31d2 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1137,7 +1137,7 @@ trait Implicits: /** The expected type where parameters and uninstantiated typevars are replaced by wildcard types */ val wildProto: Type = if argument.isEmpty then wildApprox(pt) - else ViewProto(wildApprox(argument.tpe.widen), wildApprox(pt)) + else ViewProto(wildApprox(argument.tpe.widen.normalized), wildApprox(pt)) // Not clear whether we need to drop the `.widen` here. All tests pass with it in place, though. val isNotGiven: Boolean = wildProto.classSymbol == defn.NotGivenClass diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index bfaf10751fde..6b2af4c1ff85 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -198,6 +198,7 @@ class CompilationTests { @Test def runAll: Unit = { implicit val testGroup: TestGroup = TestGroup("runAll") aggregateTests( + compileFile("tests/run-custom-args/typeclass-derivation1.scala", defaultOptions.without(yCheckOptions: _*)), compileFile("tests/run-custom-args/tuple-cons.scala", allowDeepSubtypes), compileFile("tests/run-custom-args/i5256.scala", allowDeepSubtypes), compileFile("tests/run-custom-args/fors.scala", defaultOptions.and("-source", "future")), diff --git a/tests/neg/11982.scala b/tests/neg/11982.scala new file mode 100644 index 000000000000..dd7a2b9b055e --- /dev/null +++ b/tests/neg/11982.scala @@ -0,0 +1,40 @@ +// testCompilation 11982.scala +type Head[X] = X match { + case Tuple2[a, b] => a +} + +object Unpair { + def unpair[X <: Tuple2[Any, Any]]: Head[X] = 1 // error + unpair[Tuple2["msg", 42]]: "msg" +} + + +type Head2[X] = X match { + case Tuple2[Tuple2[a, b], Tuple2[c, d]] => a +} + +object Unpair2 { + def unpair[X <: Tuple2[Tuple2[Any, Any], Tuple2[Any, Any]]]: Head2[X] = 1 // error + unpair[Tuple2[Tuple2["msg", 42], Tuple2[41, 40]]]: "msg" +} + + +type Head3[X] = X match { + case Tuple2[a, b] => a +} + +object Unpair3 { + def unpair[X <: Tuple2[Any, Any]]: Head3[Tuple2[X, X]] = (1, 2) // error + unpair[Tuple2["msg", 42]]: ("msg", 42) +} + +trait Foo[+A, +B] + +type Head4[X] = X match { + case Foo[Foo[a, b], Foo[c, d]] => a +} + +object Unpair4 { + def unpair[X <: Foo[Any, Any]]: Head4[Foo[X, X]] = 1 // error + unpair[Foo["msg", 42]]: "msg" +} diff --git a/tests/neg/6570-1.scala b/tests/neg/6570-1.scala index a8cdf6410ed5..417f4e477779 100644 --- a/tests/neg/6570-1.scala +++ b/tests/neg/6570-1.scala @@ -21,10 +21,32 @@ trait Root[A] { class Asploder extends Root[Cov[Box[Int & String]]] { def thing = new Trait1 {} // error + // ^ + // Found: Object with Trait1 {...} + // Required: N[Box[Int & String]] + // + // Note: a match type could not be fully reduced: + // + // trying to reduce N[Box[Int & String]] + // failed since selector Box[Int & String] + // is uninhabited (there are no values of that type). } object Main { - def foo[T <: Cov[Box[Int]]](c: Root[T]): Trait2 = c.thing + def foo[T <: Cov[Box[Int]]](c: Root[T]): Trait2 = c.thing // error + // ^^^^^^^ + // Found: M[T] + // Required: Trait2 + // + // where: T is a type in method foo with bounds <: Cov[Box[Int]] + // + // + // Note: a match type could not be fully reduced: + // + // trying to reduce M[T] + // failed since selector T + // does not match case Cov[x] => N[x] + // and cannot be shown to be disjoint from it either. def explode = foo(new Asploder) diff --git a/tests/neg/6570.scala b/tests/neg/6570.scala index d0b55fe739de..f90c9b65014b 100644 --- a/tests/neg/6570.scala +++ b/tests/neg/6570.scala @@ -21,7 +21,7 @@ object UpperBoundParametricVariant { trait Child[A <: Cov[Int]] extends Root[A] // we reduce `M[T]` to `Trait2`, even though we cannot be certain of that - def foo[T <: Cov[Int]](c: Child[T]): Trait2 = c.thing + def foo[T <: Cov[Int]](c: Child[T]): Trait2 = c.thing // error class Asploder extends Child[Cov[String & Int]] { def thing = new Trait1 {} // error @@ -42,7 +42,7 @@ object InheritanceVariant { trait Child extends Root { type B <: { type A <: Int } } - def foo(c: Child): Trait2 = c.thing + def foo(c: Child): Trait2 = c.thing // error class Asploder extends Child { type B = { type A = String & Int } @@ -98,7 +98,7 @@ object UpperBoundVariant { trait Child extends Root { type A <: Cov[Int] } - def foo(c: Child): Trait2 = c.thing + def foo(c: Child): Trait2 = c.thing // error class Asploder extends Child { type A = Cov[String & Int] diff --git a/tests/pos/11982-a/119_1.scala b/tests/pos/11982-a/119_1.scala new file mode 100644 index 000000000000..4a8c33e62857 --- /dev/null +++ b/tests/pos/11982-a/119_1.scala @@ -0,0 +1,9 @@ +object Unpair { + class Inv[T] + + type Head[X] = X match { + case Tuple2[a, b] => a + } + + def unpair[X <: Tuple2[Any, Any]]: Head[X] = ??? +} diff --git a/tests/pos/11982-a/119_2.scala b/tests/pos/11982-a/119_2.scala new file mode 100644 index 000000000000..d4308f57dee4 --- /dev/null +++ b/tests/pos/11982-a/119_2.scala @@ -0,0 +1,5 @@ +object UnpairApp { + import Unpair._ + + val x: String = unpair[("msg", 42)] +} diff --git a/tests/pos/13491.scala b/tests/pos/13491.scala index d16452cf922c..b2c7941e15a0 100644 --- a/tests/pos/13491.scala +++ b/tests/pos/13491.scala @@ -86,7 +86,8 @@ object Rule { type RuleN[+L <: HList] = Rule[HNil, L] def rule[I <: HList, O <: HList](r: Rule[I, O]): Rule[I, O] = ??? - implicit def valueMap[T](m: Map[String, T])(implicit h: HListable[T]): RuleN[h.Out] = ??? + + implicit def valueMap[T, Out0 <: HList](m: Map[String, T])(implicit h: HListable[T] { type Out = Out0 }): RuleN[Out0] = ??? } object Test { diff --git a/tests/run/typeclass-derivation1.scala b/tests/run-custom-args/typeclass-derivation1.scala similarity index 89% rename from tests/run/typeclass-derivation1.scala rename to tests/run-custom-args/typeclass-derivation1.scala index f6c83c770af2..f2fda59bd3de 100644 --- a/tests/run/typeclass-derivation1.scala +++ b/tests/run-custom-args/typeclass-derivation1.scala @@ -96,3 +96,15 @@ object Test extends App { assert(!eq2.equals(yss, xss)) assert(eq2.equals(yss, yss)) } + +// -Ycheck failure minimized to: +// import scala.compiletime.* +// object Eq { +// inline def deriveForProduct[Elems <: Tuple](xs: Elems): Boolean = inline erasedValue[Elems] match { +// case _: (elem *: elems1) => +// val xs1 = xs.asInstanceOf[elem *: elems1] +// deriveForProduct(xs1.tail) +// case _: EmptyTuple => +// true +// } +// }