Refactor handling of unpickled type params

Under the new hk scheme we discovered that type parameters
are sometimes unpickled in the wrong order. The fault was always
present but the previous hk schemes were somehow lenient
enough in their subtyping rules to not discover the problem.
E.g., when reading Coder.scala, dotc believed that parameter
`A` of `TraversableOnce#BufferedCanBuildFrom` is higher-kinded
and parameter `CC` is first-order where the opposite is true.

This commit hardens the way we read type parameters in order
to make this swap impossible by design.

- Revert auto-healing in derivedAppliedType

The healing hid a real error about order of type parameters
in Scala2 unpickling which was fixed in the previous commits.
The healing caused Map.scala to fail because it is possible
that type parameters are mis-prediced to be Nil in an F-bounded
context.

- Smallish fixes to type applications

5 files changed, 69 insertions, 36 deletions

diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala
index 46d93b753..54884a24c 100644
--- a/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -1213,15 +1213,25 @@ object SymDenotations {
 
     private[this] var myNamedTypeParams: Set[TypeSymbol] = _
 
+    /** The type parameters in this class, in the order they appear in the current
+     *  scope `decls`. This is might be temporarily the incorrect order when
+     *  reading Scala2 pickled info. The problem is fixed by `updateTypeParams`
+     *  which is called once an unpickled symbol has been completed.
+     */
+    private def typeParamsFromDecls(implicit ctx: Context) =
+      unforcedDecls.filter(sym =>
+        (sym is TypeParam) && sym.owner == symbol).asInstanceOf[List[TypeSymbol]]
+
     /** The type parameters of this class */
     override final def typeParams(implicit ctx: Context): List[TypeSymbol] = {
-      def computeTypeParams = {
-        if (ctx.erasedTypes || is(Module)) Nil // fast return for modules to avoid scanning package decls
-        else if (this ne initial) initial.asSymDenotation.typeParams
-        else unforcedDecls.filter(sym =>
-          (sym is TypeParam) && sym.owner == symbol).asInstanceOf[List[TypeSymbol]]
-      }
-      if (myTypeParams == null) myTypeParams = computeTypeParams
+      if (myTypeParams == null)
+        myTypeParams =
+          if (ctx.erasedTypes || is(Module)) Nil // fast return for modules to avoid scanning package decls
+          else if (this ne initial) initial.asSymDenotation.typeParams
+          else infoOrCompleter match {
+            case info: TypeParamsCompleter => info.completerTypeParams(symbol)
+            case _ => typeParamsFromDecls
+          }
       myTypeParams
     }
 
@@ -1537,16 +1547,16 @@ object SymDenotations {
       if (myMemberCache != null) myMemberCache invalidate sym.name
     }
 
-    /** Make sure the type parameters of this class are `tparams`, reorder definitions
-     *  in scope if necessary.
+    /** Make sure the type parameters of this class appear in the order given
+     *  by `tparams` in the scope of the class. Reorder definitions in scope if necessary.
      *  @pre  All type parameters in `tparams` are entered in class scope `info.decls`.
      */
     def updateTypeParams(tparams: List[Symbol])(implicit ctx: Context): Unit =
-      if (!typeParams.corresponds(tparams)(_.name == _.name)) {
+      if (!ctx.erasedTypes && !typeParamsFromDecls.corresponds(typeParams)(_.name == _.name)) {
         val decls = info.decls
         val decls1 = newScope
         for (tparam <- tparams) decls1.enter(decls.lookup(tparam.name))
-        for (sym <- decls) if (!typeParams.contains(sym)) decls1.enter(sym)
+        for (sym <- decls) if (!tparams.contains(sym)) decls1.enter(sym)
         info = classInfo.derivedClassInfo(decls = decls1)
         myTypeParams = null
       }
@@ -1868,9 +1878,9 @@ object SymDenotations {
   /** A subclass of LazyTypes where type parameters can be completed independently of
    *  the info.
    */
-  abstract class TypeParamsCompleter extends LazyType {
+  trait TypeParamsCompleter extends LazyType {
     /** The type parameters computed by the completer before completion has finished */
-    def completerTypeParams(sym: Symbol): List[TypeSymbol]
+    def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeSymbol]
   }
 
   val NoSymbolFn = (ctx: Context) => NoSymbol
diff --git a/src/dotty/tools/dotc/core/TypeApplications.scala b/src/dotty/tools/dotc/core/TypeApplications.scala
index 94e09eaf0..274fc8ff8 100644
--- a/src/dotty/tools/dotc/core/TypeApplications.scala
+++ b/src/dotty/tools/dotc/core/TypeApplications.scala
@@ -4,7 +4,7 @@ package core
 import Types._
 import Contexts._
 import Symbols._
-import SymDenotations.TypeParamsCompleter
+import SymDenotations.{LazyType, TypeParamsCompleter}
 import Decorators._
 import util.Stats._
 import util.common._
@@ -191,7 +191,8 @@ object TypeApplications {
     else {
       def bounds(tparam: MemberBinding) = tparam match {
         case tparam: Symbol => tparam.infoOrCompleter
-        case tparam: RefinedType => tparam.memberBounds
+        case tparam: RefinedType if !Config.newHK => tparam.memberBounds
+        case tparam: LambdaParam => tparam.memberBounds
       }
       args.zipWithConserve(tparams)((arg, tparam) => arg.etaExpandIfHK(bounds(tparam)))
     }
@@ -380,7 +381,7 @@ class TypeApplications(val self: Type) extends AnyVal {
     case self: WildcardType => self.optBounds.knownHK
     case self: PolyParam => self.underlying.knownHK
     case self: TypeProxy => self.underlying.knownHK
-    case NoType => 0
+    case NoType | _: LazyType => 0
     case _ => -1
   }
 
@@ -742,7 +743,7 @@ class TypeApplications(val self: Type) extends AnyVal {
         self.derivedTypeBounds(self.lo, self.hi.appliedTo(args))
       case self: LazyRef =>
         LazyRef(() => self.ref.appliedTo(args, typParams))
-      case _ if typParams.nonEmpty && typParams.head.isInstanceOf[LambdaParam] =>
+      case _ if typParams.isEmpty || typParams.head.isInstanceOf[LambdaParam] =>
         HKApply(self, args)
       case _ =>
         matchParams(self, typParams, args) match {
diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala
index c23050c19..593dcb967 100644
--- a/src/dotty/tools/dotc/core/Types.scala
+++ b/src/dotty/tools/dotc/core/Types.scala
@@ -1436,6 +1436,9 @@ object Types {
       else computeDenot
     }
 
+    /** Hook for adding debug check code when denotations are assigned */
+    final def checkDenot()(implicit ctx: Context) = {}
+
     /** A second fallback to recompute the denotation if necessary */
     private def computeDenot(implicit ctx: Context): Denotation = {
       val savedEphemeral = ctx.typerState.ephemeral
@@ -1471,6 +1474,7 @@ object Types {
 
           // Don't use setDenot here; double binding checks can give spurious failures after erasure
           lastDenotation = d
+          checkDenot()
           lastSymbol = d.symbol
           checkedPeriod = ctx.period
         }
@@ -1542,6 +1546,7 @@ object Types {
       // additional checks that intercept `denot` can be added here
 
       lastDenotation = denot
+      checkDenot()
       lastSymbol = denot.symbol
       checkedPeriod = Nowhere
     }
@@ -3462,8 +3467,7 @@ object Types {
     protected def derivedSuperType(tp: SuperType, thistp: Type, supertp: Type): Type =
       tp.derivedSuperType(thistp, supertp)
     protected def derivedAppliedType(tp: HKApply, tycon: Type, args: List[Type]): Type =
-      if (tycon.typeParams.isEmpty) tycon
-      else tp.derivedAppliedType(tycon, args)
+      tp.derivedAppliedType(tycon, args)
     protected def derivedAndOrType(tp: AndOrType, tp1: Type, tp2: Type): Type =
       tp.derivedAndOrType(tp1, tp2)
     protected def derivedAnnotatedType(tp: AnnotatedType, underlying: Type, annot: Annotation): Type =
diff --git a/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
index 2d7b037b1..813376655 100644
--- a/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
+++ b/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
@@ -389,7 +389,7 @@ class ClassfileParser(
       }
       index += 1
     }
-    val ownTypeParams = newTParams.toList
+    val ownTypeParams = newTParams.toList.asInstanceOf[List[TypeSymbol]]
     val tpe =
       if ((owner == null) || !owner.isClass)
         sig2type(tparams, skiptvs = false)
@@ -584,7 +584,7 @@ class ClassfileParser(
    *  a vararg argument. We solve this by creating two constructors, one with
    *  an array, the other with a repeated parameter.
    */
-  def addAnnotationConstructor(classInfo: Type, tparams: List[Symbol] = Nil)(implicit ctx: Context): Unit = {
+  def addAnnotationConstructor(classInfo: Type, tparams: List[TypeSymbol] = Nil)(implicit ctx: Context): Unit = {
     def addDefaultGetter(attr: Symbol, n: Int) =
       ctx.newSymbol(
         owner = moduleRoot.symbol,
diff --git a/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala
index 2ea911380..1da92d723 100644
--- a/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala
+++ b/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala
@@ -31,7 +31,7 @@ object Scala2Unpickler {
   /** Exception thrown if classfile is corrupted */
   class BadSignature(msg: String) extends RuntimeException(msg)
 
-  case class TempPolyType(tparams: List[Symbol], tpe: Type) extends UncachedGroundType {
+  case class TempPolyType(tparams: List[TypeSymbol], tpe: Type) extends UncachedGroundType {
     override def fallbackToText(printer: Printer): Text =
       "[" ~ printer.dclsText(tparams, ", ") ~ "]" ~ printer.toText(tpe)
   }
@@ -195,8 +195,6 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
     case _ => errorBadSignature(s"a runtime exception occurred: $ex", Some(ex))
   }
 
-  private var postReadOp: Context => Unit = null
-
   def run()(implicit ctx: Context) =
     try {
       var i = 0
@@ -204,10 +202,11 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
         if (entries(i) == null && isSymbolEntry(i)) {
           val savedIndex = readIndex
           readIndex = index(i)
-          entries(i) = readSymbol()
-          if (postReadOp != null) {
-            postReadOp(ctx)
-            postReadOp = null
+          val sym = readSymbol()
+          entries(i) = sym
+          sym.infoOrCompleter match {
+            case info: ClassUnpickler => info.init()
+            case _ =>
           }
           readIndex = savedIndex
         }
@@ -493,20 +492,20 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
         }
         ctx.newSymbol(owner, name1, flags1, localMemberUnpickler, coord = start)
       case CLASSsym =>
-        val infoRef = readNat()
-        postReadOp = implicit ctx => atReadPos(index(infoRef), readTypeParams) // force reading type params early, so they get entered in the right order.
+        var infoRef = readNat()
+        if (isSymbolRef(infoRef)) infoRef = readNat()
         if (isClassRoot)
           completeRoot(
-            classRoot, rootClassUnpickler(start, classRoot.symbol, NoSymbol))
+            classRoot, rootClassUnpickler(start, classRoot.symbol, NoSymbol, infoRef))
         else if (isModuleClassRoot)
           completeRoot(
-            moduleClassRoot, rootClassUnpickler(start, moduleClassRoot.symbol, moduleClassRoot.sourceModule))
+            moduleClassRoot, rootClassUnpickler(start, moduleClassRoot.symbol, moduleClassRoot.sourceModule, infoRef))
         else if (name == tpnme.REFINE_CLASS)
           // create a type alias instead
           ctx.newSymbol(owner, name, flags, localMemberUnpickler, coord = start)
         else {
           def completer(cls: Symbol) = {
-            val unpickler = new LocalUnpickler() withDecls symScope(cls)
+            val unpickler = new ClassUnpickler(infoRef) withDecls symScope(cls)
             if (flags is ModuleClass)
               unpickler withSourceModule (implicit ctx =>
                 cls.owner.info.decls.lookup(cls.name.sourceModuleName)
@@ -589,8 +588,27 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
 
   object localMemberUnpickler extends LocalUnpickler
 
-  def rootClassUnpickler(start: Coord, cls: Symbol, module: Symbol) =
-    (new LocalUnpickler with SymbolLoaders.SecondCompleter {
+  class ClassUnpickler(infoRef: Int) extends LocalUnpickler with TypeParamsCompleter {
+    private def readTypeParams()(implicit ctx: Context): List[TypeSymbol] = {
+      val tag = readByte()
+      val end = readNat() + readIndex
+      if (tag == POLYtpe) {
+        val unusedRestperef = readNat()
+        until(end, readSymbolRef).asInstanceOf[List[TypeSymbol]]
+      } else Nil
+    }
+    private def loadTypeParams(implicit ctx: Context) =
+      atReadPos(index(infoRef), readTypeParams)
+
+    /** Force reading type params early, we need them in setClassInfo of subclasses. */
+    def init()(implicit ctx: Context) = loadTypeParams
+
+    def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeSymbol] =
+      loadTypeParams
+  }
+
+  def rootClassUnpickler(start: Coord, cls: Symbol, module: Symbol, infoRef: Int) =
+    (new ClassUnpickler(infoRef) with SymbolLoaders.SecondCompleter {
       override def startCoord(denot: SymDenotation): Coord = start
     }) withDecls symScope(cls) withSourceModule (_ => module)
 
@@ -756,7 +774,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
       case POLYtpe =>
         val restpe = readTypeRef()
         val typeParams = until(end, readSymbolRef)
-        if (typeParams.nonEmpty) TempPolyType(typeParams, restpe.widenExpr)
+        if (typeParams.nonEmpty) TempPolyType(typeParams.asInstanceOf[List[TypeSymbol]], restpe.widenExpr)
         else ExprType(restpe)
       case EXISTENTIALtpe =>
         val restpe = readTypeRef()