Skip to content

Commit

Permalink
Don't follow BaseType of abstract binders in MT reduction
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
OlivierBlanvillain committed Oct 20, 2021
1 parent d3a26e2 commit 60fb562
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 9 deletions.
11 changes: 8 additions & 3 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -756,8 +756,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
Expand All @@ -776,7 +781,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 _ =>
def isNullable(tp: Type): Boolean = tp.widenDealias match {
case tp: TypeRef => tp.symbol.isNullableClass
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,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
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,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")),
Expand Down
40 changes: 40 additions & 0 deletions tests/neg/11982.scala
Original file line number Diff line number Diff line change
@@ -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"
}
24 changes: 23 additions & 1 deletion tests/neg/6570-1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
6 changes: 3 additions & 3 deletions tests/neg/6570.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 }
Expand Down Expand Up @@ -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]
Expand Down
9 changes: 9 additions & 0 deletions tests/pos/11982-a/119_1.scala
Original file line number Diff line number Diff line change
@@ -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] = ???
}
5 changes: 5 additions & 0 deletions tests/pos/11982-a/119_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object UnpairApp {
import Unpair._

val x: String = unpair[("msg", 42)]
}
3 changes: 2 additions & 1 deletion tests/pos/13491.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
// }
// }

0 comments on commit 60fb562

Please sign in to comment.