Merge pull request #3889 from dotty-staging/fix-#2675

Fix #2675: Properly handle undetermined type variables in unapply

Author: smarter

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/Flags.scala

@@ -327,9 +327,6 @@ object Flags {
   /** A method that has default params */
   final val DefaultParameterized = termFlag(27, "<defaultparam>")
 
-  /** A type that is defined by a type bind */
-  final val BindDefinedType = typeFlag(27, "<bind-defined>")
-
   /** Symbol is inlined */
   final val Inline = commonFlag(29, "inline")
 

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/TypeComparer.scala

@@ -1093,23 +1093,13 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
       val tparam = tr.symbol
       typr.println(i"narrow gadt bound of $tparam: ${tparam.info} from ${if (isUpper) "above" else "below"} to $bound ${bound.isRef(tparam)}")
       if (bound.isRef(tparam)) false
-      else bound match {
-        case bound: TypeRef
-        if bound.symbol.is(BindDefinedType) &&
-           ctx.gadt.bounds.contains(bound.symbol) &&
-           !tr.symbol.is(BindDefinedType) =>
-          // Avoid having pattern-bound types in gadt bounds,
-          // as these might be eliminated once the pattern is typechecked.
-          // Pattern-bound type symbols should be narrowed first, only if that fails
-          // should symbols in the environment be constrained.
-          narrowGADTBounds(bound, tr, !isUpper)
-        case _ =>
-          val oldBounds = ctx.gadt.bounds(tparam)
-          val newBounds =
-            if (isUpper) TypeBounds(oldBounds.lo, oldBounds.hi & bound)
-            else TypeBounds(oldBounds.lo | bound, oldBounds.hi)
-          isSubType(newBounds.lo, newBounds.hi) &&
-            { ctx.gadt.setBounds(tparam, newBounds); true }
+      else {
+        val oldBounds = ctx.gadt.bounds(tparam)
+        val newBounds =
+          if (isUpper) TypeBounds(oldBounds.lo, oldBounds.hi & bound)
+          else TypeBounds(oldBounds.lo | bound, oldBounds.hi)
+        isSubType(newBounds.lo, newBounds.hi) &&
+          { ctx.gadt.setBounds(tparam, newBounds); true }
       }
     }
 

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/TreePickler.scala

@@ -161,10 +161,6 @@ class TreePickler(pickler: TastyPickler) {
       pickleConstant(value)
     case tpe: NamedType =>
       val sym = tpe.symbol
-      def pickleDirectRef() = {
-        writeByte(if (tpe.isType) TYPEREFdirect else TERMREFdirect)
-        pickleSymRef(sym)
-      }
       def pickleExternalRef(sym: Symbol) = {
         def pickleCore() = {
           pickleNameAndSig(sym.name, tpe.signature)
@@ -188,17 +184,10 @@ class TreePickler(pickler: TastyPickler) {
         writeByte(if (tpe.isType) TYPEREFpkg else TERMREFpkg)
         pickleName(sym.fullName)
       }
-      else if (tpe.prefix == NoPrefix)
-        if (sym is Flags.BindDefinedType) {
-          registerDef(sym)
-          writeByte(BIND)
-          withLength {
-            pickleName(sym.name)
-            pickleType(sym.info)
-            pickleDirectRef()
-          }
-        }
-        else pickleDirectRef()
+      else if (tpe.prefix == NoPrefix) {
+        writeByte(if (tpe.isType) TYPEREFdirect else TERMREFdirect)
+        pickleSymRef(sym)
+      }
       else if (isLocallyDefined(sym)) {
         writeByte(if (tpe.isType) TYPEREFsymbol else TERMREFsymbol)
         pickleSymRef(sym); pickleType(tpe.prefix)
@@ -445,7 +434,9 @@ class TreePickler(pickler: TastyPickler) {
         case Bind(name, body) =>
           registerDef(tree.symbol)
           writeByte(BIND)
-          withLength { pickleName(name); pickleType(tree.symbol.info); pickleTree(body) }
+          withLength {
+            pickleName(name); pickleType(tree.symbol.info); pickleTree(body)
+          }
         case Alternative(alts) =>
           writeByte(ALTERNATIVE)
           withLength { alts.foreach(pickleTree) }

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

@@ -109,7 +109,10 @@ class TreeUnpickler(reader: TastyReader,
       while (nextByte == PARAMS || nextByte == TYPEPARAM) skipTree()
 
     /** Record all directly nested definitions and templates in current tree
-     *  as `OwnerTree`s in `buf`
+     *  as `OwnerTree`s in `buf`.
+     *  A complication concerns member definitions. These are lexically nested in a
+     *  Template node, but need to be listed separately in the OwnerTree of the enclosing class
+     *  in order not to confuse owner chains.
      */
     def scanTree(buf: ListBuffer[OwnerTree], mode: MemberDefMode = AllDefs): Unit = {
       val start = currentAddr
@@ -118,8 +121,15 @@ class TreeUnpickler(reader: TastyReader,
         case VALDEF | DEFDEF | TYPEDEF | TYPEPARAM | PARAM | TEMPLATE =>
           val end = readEnd()
           for (i <- 0 until numRefs(tag)) readNat()
-          if (tag == TEMPLATE) scanTrees(buf, end, MemberDefsOnly)
-          if (mode != NoMemberDefs) buf += new OwnerTree(start, tag, fork, end)
+          if (tag == TEMPLATE) {
+            // Read all member definitions now, whereas non-members are children of
+            // template's owner tree.
+            val nonMemberReader = fork
+            scanTrees(buf, end, MemberDefsOnly)
+            buf += new OwnerTree(start, tag, nonMemberReader, end)
+          }
+          else if (mode != NoMemberDefs)
+            buf += new OwnerTree(start, tag, fork, end)
           goto(end)
         case tag =>
           if (mode == MemberDefsOnly) skipTree(tag)
@@ -132,7 +142,11 @@ class TreeUnpickler(reader: TastyReader,
             }
             else {
               for (i <- 0 until nrefs) readNat()
-              scanTrees(buf, end)
+              if (tag == BIND) {
+                buf += new OwnerTree(start, tag, fork, end)
+                goto(end)
+              }
+              else scanTrees(buf, end)
             }
           }
           else if (tag >= firstNatASTTreeTag) { readNat(); scanTree(buf) }
@@ -267,12 +281,6 @@ class TreeUnpickler(reader: TastyReader,
               OrType(readType(), readType())
             case SUPERtype =>
               SuperType(readType(), readType())
-            case BIND =>
-              val sym = ctx.newSymbol(ctx.owner, readName().toTypeName, BindDefinedType, readType(),
-                coord = coordAt(start))
-              registerSym(start, sym)
-              if (currentAddr != end) readType()
-              TypeRef(NoPrefix, sym)
             case POLYtype =>
               readMethodic(PolyType, _.toTypeName)
             case METHODtype =>
@@ -431,6 +439,8 @@ class TreeUnpickler(reader: TastyReader,
     def createSymbol()(implicit ctx: Context): Symbol = nextByte match {
       case VALDEF | DEFDEF | TYPEDEF | TYPEPARAM | PARAM =>
         createMemberSymbol()
+      case BIND =>
+        createBindSymbol()
       case TEMPLATE =>
         val localDummy = ctx.newLocalDummy(ctx.owner)
         registerSym(currentAddr, localDummy)
@@ -439,6 +449,25 @@ class TreeUnpickler(reader: TastyReader,
         throw new Error(s"illegal createSymbol at $currentAddr, tag = $tag")
     }
 
+    private def createBindSymbol()(implicit ctx: Context): Symbol = {
+      val start = currentAddr
+      val tag = readByte()
+      val end = readEnd()
+      var name: Name = readName()
+      nextUnsharedTag match {
+        case TYPEBOUNDS | TYPEALIAS => name = name.toTypeName
+        case _ =>
+      }
+      val typeReader = fork
+      val completer = new LazyType {
+        def complete(denot: SymDenotation)(implicit ctx: Context) =
+          denot.info = typeReader.readType()
+      }
+      val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, completer, coord = coordAt(start))
+      registerSym(start, sym)
+      sym
+    }
+
     /** Create symbol of member definition or parameter node and enter in symAtAddr map
      *  @return  the created symbol
      */
@@ -994,10 +1023,9 @@ class TreeUnpickler(reader: TastyReader,
               val elemtpt = readTpt()
               SeqLiteral(until(end)(readTerm()), elemtpt)
             case BIND =>
-              val name = readName()
-              val info = readType()
-              val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, info, coord = coordAt(start))
-              registerSym(start, sym)
+              val sym = symAtAddr.getOrElse(start, forkAt(start).createSymbol())
+              readName()
+              readType()
               Bind(sym, readTerm())
             case ALTERNATIVE =>
               Alternative(until(end)(readTerm()))
@@ -1154,11 +1182,16 @@ class TreeUnpickler(reader: TastyReader,
    */
   class OwnerTree(val addr: Addr, tag: Int, reader: TreeReader, val end: Addr) {
 
+    private var myChildren: List[OwnerTree] = null
+
     /** All definitions that have the definition at `addr` as closest enclosing definition */
-    lazy val children: List[OwnerTree] = {
-      val buf = new ListBuffer[OwnerTree]
-      reader.scanTrees(buf, end, if (tag == TEMPLATE) NoMemberDefs else AllDefs)
-      buf.toList
+    def children: List[OwnerTree] = {
+      if (myChildren == null) myChildren = {
+        val buf = new ListBuffer[OwnerTree]
+        reader.scanTrees(buf, end, if (tag == TEMPLATE) NoMemberDefs else AllDefs)
+        buf.toList
+      }
+      myChildren
     }
 
     /** Find the owner of definition at `addr` */
@@ -1177,13 +1210,19 @@ class TreeUnpickler(reader: TastyReader,
       }
       catch {
         case ex: TreeWithoutOwner =>
-          println(i"no owner for $addr among $cs")  // DEBUG
+          pickling.println(i"no owner for $addr among $cs%, %")
+          throw ex
+      }
+      try search(children, NoSymbol)
+      catch {
+        case ex: TreeWithoutOwner =>
+          pickling.println(s"ownerTree = $ownerTree")
           throw ex
       }
-      search(children, NoSymbol)
     }
 
-    override def toString = s"OwnerTree(${addr.index}, ${end.index}"
+    override def toString =
+      s"OwnerTree(${addr.index}, ${end.index}, ${if (myChildren == null) "?" else myChildren.mkString(" ")})"
   }
 }
 

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`?
@@ -923,27 +937,8 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
             fullyDefinedType(unapplyArgType, "pattern selector", tree.pos)
             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/TypeAssigner.scala

@@ -362,7 +362,7 @@ trait TypeAssigner {
   def assignType(tree: untpd.Apply, fn: Tree, args: List[Tree])(implicit ctx: Context) = {
     val ownType = fn.tpe.widen match {
       case fntpe: MethodType =>
-        if (sameLength(fntpe.paramInfos, args) || ctx.phase.prev.relaxedTyping || true)
+        if (sameLength(fntpe.paramInfos, args) || ctx.phase.prev.relaxedTyping)
           if (fntpe.isDependent) safeSubstParams(fntpe.resultType, fntpe.paramRefs, args.tpes)
           else fntpe.resultType
         else