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

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

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 {

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 =

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,

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,19 +195,18 @@ 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
       while (i < index.length) {
         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()

src/dotty/tools/dotc/typer/Checking.scala

@@ -194,7 +194,7 @@ object Checking {
             case SuperType(thistp, _) => isInteresting(thistp)
             case AndType(tp1, tp2) => isInteresting(tp1) || isInteresting(tp2)
             case OrType(tp1, tp2) => isInteresting(tp1) && isInteresting(tp2)
-            case _: RefinedOrRecType => true
+            case _: RefinedOrRecType | _: HKApply => true
             case _ => false
           }
           if (isInteresting(pre)) {
@@ -223,6 +223,9 @@ object Checking {
     val checker = new CheckNonCyclicMap(sym, reportErrors)(ctx.addMode(Mode.CheckCyclic))
     try checker.checkInfo(info)
     catch {
+          case ex: AssertionError =>
+            println(s"assertion error for $info")
+            throw ex
       case ex: CyclicReference =>
         if (reportErrors) {
           ctx.error(i"illegal cyclic reference: ${checker.where} ${checker.lastChecked} of $sym refers back to the type itself", sym.pos)

src/dotty/tools/dotc/typer/Namer.scala

@@ -307,9 +307,14 @@ class Namer { typer: Typer =>
         // have no implementation.
         val cctx = if (tree.name == nme.CONSTRUCTOR && !(tree.mods is JavaDefined)) ctx.outer else ctx
 
+        val completer = tree match {
+          case tree: TypeDef => new TypeDefCompleter(tree)(cctx)
+          case _ => new Completer(tree)(cctx)
+        }
+
         recordSym(ctx.newSymbol(
           ctx.owner, name, flags | deferred | method | higherKinded | inSuperCall1,
-          adjustIfModule(new Completer(tree)(cctx), tree),
+          adjustIfModule(completer, tree),
           privateWithinClass(tree.mods), tree.pos), tree)
       case tree: Import =>
         recordSym(ctx.newSymbol(
@@ -489,42 +494,18 @@ class Namer { typer: Typer =>
   }
 
   /** The completer of a symbol defined by a member def or import (except ClassSymbols) */
-  class Completer(val original: Tree)(implicit ctx: Context) extends TypeParamsCompleter {
+  class Completer(val original: Tree)(implicit ctx: Context) extends LazyType {
 
     protected def localContext(owner: Symbol) = ctx.fresh.setOwner(owner).setTree(original)
 
-    private var myTypeParams: List[TypeSymbol] = null
-    private var nestedCtx: Context = null
-
-    def completerTypeParams(sym: Symbol): List[TypeSymbol] = {
-      if (myTypeParams == null) {
-        //println(i"completing type params of $sym in ${sym.owner}")
-        myTypeParams = original match {
-          case tdef: TypeDef =>
-            nestedCtx = localContext(sym).setNewScope
-            locally {
-              implicit val ctx: Context = nestedCtx
-              completeParams(tdef.tparams)
-              tdef.tparams.map(symbolOfTree(_).asType)
-            }
-          case _ =>
-            Nil
-        }
-      }
-      myTypeParams
-    }
-
-    private def typeSig(sym: Symbol): Type = original match {
+    protected def typeSig(sym: Symbol): Type = original match {
       case original: ValDef =>
         if (sym is Module) moduleValSig(sym)
         else valOrDefDefSig(original, sym, Nil, Nil, identity)(localContext(sym).setNewScope)
       case original: DefDef =>
         val typer1 = ctx.typer.newLikeThis
         nestedTyper(sym) = typer1
         typer1.defDefSig(original, sym)(localContext(sym).setTyper(typer1))
-      case original: TypeDef =>
-        assert(!original.isClassDef)
-        typeDefSig(original, sym, completerTypeParams(sym))(nestedCtx)
       case imp: Import =>
         try {
           val expr1 = typedAheadExpr(imp.expr, AnySelectionProto)
@@ -569,6 +550,28 @@ class Namer { typer: Typer =>
     }
   }
 
+  class TypeDefCompleter(original: TypeDef)(ictx: Context) extends Completer(original)(ictx) with TypeParamsCompleter {
+    private var myTypeParams: List[TypeSymbol] = null
+    private var nestedCtx: Context = null
+    assert(!original.isClassDef)
+
+    def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeSymbol] = {
+      if (myTypeParams == null) {
+        //println(i"completing type params of $sym in ${sym.owner}")
+        nestedCtx = localContext(sym).setNewScope
+        myTypeParams = {
+          implicit val ctx: Context = nestedCtx
+          completeParams(original.tparams)
+          original.tparams.map(symbolOfTree(_).asType)
+        }
+      }
+      myTypeParams
+    }
+
+    override protected def typeSig(sym: Symbol): Type =
+      typeDefSig(original, sym, completerTypeParams(sym)(ictx))(nestedCtx)
+  }
+
   class ClassCompleter(cls: ClassSymbol, original: TypeDef)(ictx: Context) extends Completer(original)(ictx) {
     withDecls(newScope)