Fix #2675: Properly handle undetermined type variables in unapply

In non-variant contexts, we used to always maximize them, but this is unsound.
See neg/patternUnsoundness.scala. We now create fresh abstract types in
these situations.

Author: odersky

compiler/src/dotty/tools/dotc/ast/tpd.scala

@@ -158,7 +158,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
   def TypeBoundsTree(lo: Tree, hi: Tree)(implicit ctx: Context): TypeBoundsTree =
     ta.assignType(untpd.TypeBoundsTree(lo, hi), lo, hi)
 
-  def Bind(sym: TermSymbol, body: Tree)(implicit ctx: Context): Bind =
+  def Bind(sym: Symbol, body: Tree)(implicit ctx: Context): Bind =
     ta.assignType(untpd.Bind(sym.name, body), sym)
 
   /** A pattern corresponding to `sym: tpe` */

compiler/src/dotty/tools/dotc/core/Symbols.scala

@@ -219,6 +219,14 @@ trait Symbols { this: Context =>
       modFlags | PackageCreationFlags, clsFlags | PackageCreationFlags,
       Nil, decls)
 
+  /** Define a new symbol associated with a Bind or pattern wildcard and
+   *  make it gadt narrowable.
+   */
+  def newPatternBoundSymbol(name: Name, info: Type, pos: Position) = {
+    val sym = newSymbol(owner, name, Case, info, coord = pos)
+    if (name.isTypeName) gadt.setBounds(sym, info.bounds)
+    sym
+  }
 
   /** Create a stub symbol that will issue a missing reference error
    *  when attempted to be completed.

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -3308,7 +3308,7 @@ object Types {
     def isInstantiated(implicit ctx: Context) = instanceOpt.exists
 
     /** Instantiate variable with given type */
-    private def instantiateWith(tp: Type)(implicit ctx: Context): Type = {
+    def instantiateWith(tp: Type)(implicit ctx: Context): Type = {
       assert(tp ne this, s"self instantiation of ${tp.show}, constraint = ${ctx.typerState.constraint.show}")
       typr.println(s"instantiating ${this.show} with ${tp.show}")
       if ((ctx.typerState eq owningState.get) && !ctx.typeComparer.subtypeCheckInProgress)

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -994,8 +994,9 @@ class TreeUnpickler(reader: TastyReader,
               val elemtpt = readTpt()
               SeqLiteral(until(end)(readTerm()), elemtpt)
             case BIND =>
-              val name = readName()
+              var name: Name = readName()
               val info = readType()
+              if (info.isInstanceOf[TypeType]) name = name.toTypeName
               val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, info, coord = coordAt(start))
               registerSym(start, sym)
               Bind(sym, readTerm())

compiler/src/dotty/tools/dotc/transform/TailRec.scala

@@ -219,7 +219,6 @@ class TailRec extends MiniPhase with FullParameterization {
         val isRecursiveCall = (method eq sym)
         val recvWiden = prefix.tpe.widenDealias
 
-
         def continue = {
           val method = noTailTransform(call)
           val methodWithTargs = if (targs.nonEmpty) TypeApply(method, targs) else method
@@ -237,7 +236,9 @@ class TailRec extends MiniPhase with FullParameterization {
 
         if (isRecursiveCall) {
           if (ctx.tailPos) {
-            val receiverIsSame = enclosingClass.appliedRef.widenDealias =:= recvWiden
+            val receiverIsSame =
+              recvWiden <:< enclosingClass.appliedRef &&
+              (sym.isEffectivelyFinal || enclosingClass.appliedRef <:< recvWiden)
             val receiverIsThis = prefix.tpe =:= thisType || prefix.tpe.widen =:= thisType
 
             def rewriteTailCall(recv: Tree): Tree = {

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -890,12 +890,26 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
     /** Produce a typed qual.unapply or qual.unapplySeq tree, or
      *  else if this fails follow a type alias and try again.
      */
-    val unapplyFn = trySelectUnapply(qual) { sel =>
+    var unapplyFn = trySelectUnapply(qual) { sel =>
       val qual1 = followTypeAlias(qual)
       if (qual1.isEmpty) notAnExtractor(sel)
       else trySelectUnapply(qual1)(_ => notAnExtractor(sel))
     }
 
+    /** Add a `Bind` node for each `bound` symbol in a type application `unapp` */
+    def addBinders(unapp: Tree, bound: List[Symbol]) = unapp match {
+      case TypeApply(fn, args) =>
+        def addBinder(arg: Tree) = arg.tpe.stripTypeVar match {
+          case ref: TypeRef if bound.contains(ref.symbol) =>
+            tpd.Bind(ref.symbol, Ident(ref))
+          case _ =>
+            arg
+        }
+        tpd.cpy.TypeApply(unapp)(fn, args.mapConserve(addBinder))
+      case _ =>
+        unapp
+    }
+
     def fromScala2x = unapplyFn.symbol.exists && (unapplyFn.symbol.owner is Scala2x)
 
     /** Is `subtp` a subtype of `tp` or of some generalization of `tp`?
@@ -922,27 +936,8 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
             unapp.println(i"case 1 $unapplyArgType ${ctx.typerState.constraint}")
             selType
           } else if (isSubTypeOfParent(unapplyArgType, selType)(ctx.addMode(Mode.GADTflexible))) {
-            maximizeType(unapplyArgType) match {
-              case Some(tvar) =>
-                def msg =
-                  ex"""There is no best instantiation of pattern type $unapplyArgType
-                      |that makes it a subtype of selector type $selType.
-                      |Non-variant type variable ${tvar.origin} cannot be uniquely instantiated."""
-                if (fromScala2x) {
-                  // We can't issue an error here, because in Scala 2, ::[B] is invariant
-                  // whereas List[+T] is covariant. According to the strict rule, a pattern
-                  // match of a List[C] against a case x :: xs is illegal, because
-                  // B cannot be uniquely instantiated. Of course :: should have been
-                  // covariant in the first place, but in the Scala libraries it isn't.
-                  // So for now we allow these kinds of patterns, even though they
-                  // can open unsoundness holes. See SI-7952 for an example of the hole this opens.
-                  if (ctx.settings.verbose.value) ctx.warning(msg, tree.pos)
-                } else {
-                  unapp.println(s" ${unapplyFn.symbol.owner} ${unapplyFn.symbol.owner is Scala2x}")
-                  ctx.strictWarning(msg, tree.pos)
-                }
-              case _ =>
-            }
+            val patternBound = maximizeType(unapplyArgType, tree.pos, fromScala2x)
+            if (patternBound.nonEmpty) unapplyFn = addBinders(unapplyFn, patternBound)
             unapp.println(i"case 2 $unapplyArgType ${ctx.typerState.constraint}")
             unapplyArgType
           } else {

compiler/src/dotty/tools/dotc/typer/Inferencing.scala

@@ -9,6 +9,7 @@ import Trees._
 import Constants._
 import Scopes._
 import ProtoTypes._
+import NameKinds.WildcardParamName
 import annotation.unchecked
 import util.Positions._
 import util.{Stats, SimpleIdentityMap}
@@ -222,23 +223,28 @@ object Inferencing {
       case _ => NoType
     }
 
-  /** Instantiate undetermined type variables to that type `tp` is
-   *  maximized and return None. If this is not possible, because a non-variant
-   *  typevar is not uniquely determined, return that typevar in a Some.
+  /** Instantiate undetermined type variables so that type `tp` is maximized.
+   *  @return   The list of type symbols that were created
+   *            to instantiate undetermined type variables that occur non-variantly
    */
-  def maximizeType(tp: Type)(implicit ctx: Context): Option[TypeVar] = Stats.track("maximizeType") {
+  def maximizeType(tp: Type, pos: Position, fromScala2x: Boolean)(implicit ctx: Context): List[Symbol] = Stats.track("maximizeType") {
     val vs = variances(tp, alwaysTrue)
-    var result: Option[TypeVar] = None
+    val patternBound = new mutable.ListBuffer[Symbol]
     vs foreachBinding { (tvar, v) =>
       if (v == 1) tvar.instantiate(fromBelow = false)
       else if (v == -1) tvar.instantiate(fromBelow = true)
       else {
         val bounds = ctx.typerState.constraint.fullBounds(tvar.origin)
-        if (!(bounds.hi <:< bounds.lo)) result = Some(tvar)
-        tvar.instantiate(fromBelow = false)
+        if (bounds.hi <:< bounds.lo || bounds.hi.classSymbol.is(Final) || fromScala2x)
+          tvar.instantiate(fromBelow = false)
+        else {
+          val wildCard = ctx.newPatternBoundSymbol(WildcardParamName.fresh().toTypeName, bounds, pos)
+          tvar.instantiateWith(wildCard.typeRef)
+          patternBound += wildCard
+        }
       }
     }
-    result
+    patternBound.toList
   }
 
   type VarianceMap = SimpleIdentityMap[TypeVar, Integer]

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -1252,16 +1252,6 @@ class Typer extends Namer
     assignType(cpy.ByNameTypeTree(tree)(result1), result1)
   }
 
-  /** Define a new symbol associated with a Bind or pattern wildcard and
-   *  make it gadt narrowable.
-   */
-  private def newPatternBoundSym(name: Name, info: Type, pos: Position)(implicit ctx: Context) = {
-    val flags = if (name.isTypeName) BindDefinedType else EmptyFlags
-    val sym = ctx.newSymbol(ctx.owner, name, flags | Case, info, coord = pos)
-    if (name.isTypeName) ctx.gadt.setBounds(sym, info.bounds)
-    sym
-  }
-
   def typedTypeBoundsTree(tree: untpd.TypeBoundsTree)(implicit ctx: Context): TypeBoundsTree = track("typedTypeBoundsTree") {
     val TypeBoundsTree(lo, hi) = tree
     val lo1 = typed(lo)
@@ -1276,7 +1266,8 @@ class Typer extends Namer
       // The bounds of the type symbol can be constrained when comparing a pattern type
       // with an expected type in typedTyped. The type symbol is eliminated once
       // the enclosing pattern has been typechecked; see `indexPattern` in `typedCase`.
-      val wildcardSym = newPatternBoundSym(tpnme.WILDCARD, tree1.tpe, tree.pos)
+      val wildcardSym = ctx.newPatternBoundSymbol(tpnme.WILDCARD, tree1.tpe, tree.pos)
+      wildcardSym.setFlag(BindDefinedType) // can we get rid of this?
       tree1.withType(wildcardSym.typeRef)
     }
     else tree1
@@ -1296,7 +1287,8 @@ class Typer extends Namer
       case _ =>
         if (tree.name == nme.WILDCARD) body1
         else {
-          val sym = newPatternBoundSym(tree.name, body1.tpe.underlyingIfRepeated(isJava = false), tree.pos)
+          val sym = ctx.newPatternBoundSymbol(tree.name, body1.tpe.underlyingIfRepeated(isJava = false), tree.pos)
+          if (sym.isType) sym.setFlag(BindDefinedType) // can we get rid of this?
           if (ctx.mode.is(Mode.InPatternAlternative))
             ctx.error(i"Illegal variable ${sym.name} in pattern alternative", tree.pos)
           assignType(cpy.Bind(tree)(tree.name, body1), sym)

tests/neg-tailcall/t1672b.scala

@@ -46,7 +46,7 @@ object Test1772B {
     else 1 + (try {
       throw new RuntimeException
     } catch {
-      case _: Throwable => bar(i - 1) // old error
+      case _: Throwable => bar(i - 1) // error: cannot rewrite recursive call
     })
   }
 }

tests/pos/patternUnsoundness.scala renamed to tests/neg/patternUnsoundness.scala

@@ -10,8 +10,8 @@ object patternUnsoundness extends App {
   val y: C[Object] = x
 
   y match {
-    case d @ D(x) => d.s = new Integer(1)
+    case d @ D(x) => d.s = new Integer(1) // error
   }
 
-  val z: String = x.s // ClassCast exception
+  val z: String = x.s // used to throw ClassCast exception
 }