Skip to content

Fix #3816: Protect against cycles when unpickling hk types #3941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package core
import Periods._, Contexts._, Symbols._, Denotations._, Names._, NameOps._, Annotations._
import Types._, Flags._, Decorators._, DenotTransformers._, StdNames._, Scopes._, Comments._
import NameOps._, NameKinds._, Phases._
import TypeApplications.TypeParamInfo
import Scopes.Scope
import collection.mutable
import collection.BitSet
Expand Down Expand Up @@ -1915,6 +1916,11 @@ object SymDenotations {
override def complete(denot: SymDenotation)(implicit ctx: Context) = self.complete(denot)
}

/** The type parameters computed by the completer before completion has finished */
def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeParamInfo] =
if (sym is Touched) Nil // return `Nil` instead of throwing a cyclic reference
else sym.info.typeParams

def decls: Scope = myDecls
def sourceModule(implicit ctx: Context): Symbol = mySourceModuleFn(ctx)
def moduleClass(implicit ctx: Context): Symbol = myModuleClassFn(ctx)
Expand All @@ -1924,12 +1930,12 @@ object SymDenotations {
def withModuleClass(moduleClassFn: Context => Symbol): this.type = { myModuleClassFn = moduleClassFn; this }
}

/** A subclass of LazyTypes where type parameters can be completed independently of
* the info.
/** A subtrait of LazyTypes where completerTypeParams yields a List[TypeSymbol], which
* should be completed independently of the info.
*/
trait TypeParamsCompleter extends LazyType {
/** The type parameters computed by the completer before completion has finished */
def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeSymbol]
override def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeSymbol] =
unsupported("completerTypeParams") // should be abstract, but Scala-2 will then compute the wrong type for it
}

val NoSymbolFn = (ctx: Context) => NoSymbol
Expand Down
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/dotc/core/TypeApplications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,10 @@ class TypeApplications(val self: Type) extends AnyVal {
val tsym = self.symbol
if (tsym.isClass) tsym.typeParams
else if (!tsym.exists) self.info.typeParams
else if (!tsym.isCompleting) tsym.info.typeParams
else Nil
else tsym.infoOrCompleter match {
case info: LazyType => info.completerTypeParams(tsym)
case info => info.typeParams
}
case self: AppliedType =>
if (self.tycon.typeSymbol.isClass) Nil
else self.superType.typeParams
Expand Down
8 changes: 7 additions & 1 deletion compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -745,12 +745,18 @@ class TreeUnpickler(reader: TastyReader,
}
TypeDef(readTemplate(localCtx))
} else {
sym.info = TypeBounds.empty // needed to avoid cyclic references when unpicklin rhs, see i3816.scala
sym.setFlag(Provisional)
val rhs = readTpt()(localCtx)
sym.info = NoCompleter
sym.info = new NoCompleter {
override def completerTypeParams(sym: Symbol)(implicit ctx: Context) =
rhs.tpe.typeParams
}
sym.info = rhs.tpe match {
case _: TypeBounds | _: ClassInfo => checkNonCyclic(sym, rhs.tpe, reportErrors = false)
case _ => TypeAlias(rhs.tpe)
}
sym.resetFlag(Provisional)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how come we don't need to do something similar to ensureUpToDate in Namer#typeDefSig ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly: I don't know. The other question is why we need to use abstracted in Namer but the same example passes OK in TreeUnpickler. I tried to remove abstracted in Namer, bu then i3816.scala would fail to compile. So the two cases are not the same. But I did not have the time to dig deeper.

TypeDef(rhs)
}
case PARAM =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas
/** 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] =
override def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeSymbol] =
loadTypeParams
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ object Checking {
}
if (isInteresting(pre)) {
val pre1 = this(pre, false, false)
if (locked.contains(tp) || tp.symbol.infoOrCompleter == NoCompleter)
if (locked.contains(tp) || tp.symbol.infoOrCompleter.isInstanceOf[NoCompleter])
throw CyclicReference(tp.symbol)
locked += tp
try checkInfo(tp.info)
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ class Namer { typer: Typer =>
private[this] var nestedCtx: Context = null
assert(!original.isClassDef)

def completerTypeParams(sym: Symbol)(implicit ctx: Context): List[TypeSymbol] = {
override 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
Expand Down Expand Up @@ -1229,6 +1229,7 @@ class Namer { typer: Typer =>
sym.info = NoCompleter
sym.info = checkNonCyclic(sym, unsafeInfo, reportErrors = true)
}
sym.resetFlag(Provisional)

// Here we pay the price for the cavalier setting info to TypeBounds.empty above.
// We need to compensate by reloading the denotation of references that might
Expand Down
4 changes: 4 additions & 0 deletions tests/pickling/i3816.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
trait Iterable { self =>
//type CC <: Iterable { type CC = self.CC }
type DD[X] <: Iterable { type DD[Y] = self.DD[Y] }
}