From 1e48758ad5c100a7dd4d1a5b846ef5ff37e37721 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 29 Jun 2016 20:12:25 +0200 Subject: 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 | 36 ++++++++++------ src/dotty/tools/dotc/core/TypeApplications.scala | 9 ++-- src/dotty/tools/dotc/core/Types.scala | 8 +++- .../dotc/core/classfile/ClassfileParser.scala | 4 +- .../dotc/core/unpickleScala2/Scala2Unpickler.scala | 48 +++++++++++++++------- 5 files changed, 69 insertions(+), 36 deletions(-) (limited to 'src/dotty/tools/dotc/core') 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() -- cgit v1.2.3