Skip to content

Commit

Permalink
Fix scala#1401: Make sure all refs are forwarded
Browse files Browse the repository at this point in the history
Faced with recursive dependencies through self types, we might have
to apply `normalizeToClassRefs` to a class P with a parent that is not
yet initialized (witnessed by P's parents being Nil). In that case
we should still execute forwardRefs on P, but we have to
wait in a suspension until P is initialized.

This avoids the problem raised in scala#1401. I am still not quite sure
why forwardRefs is needed, but it seems that asSeenFrom alone is not
enough to track the dependencies in this case.
  • Loading branch information
odersky committed Jul 21, 2016
1 parent eaffc78 commit c37185d
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 10 deletions.
22 changes: 16 additions & 6 deletions src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,23 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
* to the current scope, provided (1) variances of both aliases are the same, and
* (2) X is not yet defined in current scope. This "short-circuiting" prevents
* long chains of aliases which would have to be traversed in type comparers.
*
* Note: Test i1401.scala shows that `forwardRefs` is also necessary
* for typechecking in the case where self types refer to type parameters
* that are upper-bounded by subclass instances.
*/
def forwardRefs(from: Symbol, to: Type, prefs: List[TypeRef]) = to match {
case to @ TypeBounds(lo1, hi1) if lo1 eq hi1 =>
for (pref <- prefs)
for (argSym <- pref.decls)
if (argSym is BaseTypeArg)
forwardRef(argSym, from, to, cls, decls)
for (pref <- prefs) {
def forward(): Unit =
for (argSym <- pref.decls)
if (argSym is BaseTypeArg)
forwardRef(argSym, from, to, cls, decls)
pref.info match {
case info: TempClassInfo => info.suspensions = (() => forward()) :: info.suspensions // !!! dotty deviation `forward` alone does not eta expand
case _ => forward()
}
}
case _ =>
}

Expand Down Expand Up @@ -419,9 +429,9 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
s"redefinition of ${decls.lookup(name).debugString} in ${cls.showLocated}")
enterArgBinding(formals(name), refinedInfo, cls, decls)
}
// Forward definitions in super classes that have one of the refined paramters
// Forward definitions in super classes that have one of the refined parameters
// as aliases directly to the refined info.
// Note that this cannot be fused bwith the previous loop because we now
// Note that this cannot be fused with the previous loop because we now
// assume that all arguments have been entered in `decls`.
refinements foreachBinding { (name, refinedInfo) =>
forwardRefs(formals(name), refinedInfo, parentRefs)
Expand Down
13 changes: 12 additions & 1 deletion src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3043,9 +3043,20 @@ object Types {
override def toString = s"ClassInfo($prefix, $cls)"
}

final class CachedClassInfo(prefix: Type, cls: ClassSymbol, classParents: List[TypeRef], decls: Scope, selfInfo: DotClass)
class CachedClassInfo(prefix: Type, cls: ClassSymbol, classParents: List[TypeRef], decls: Scope, selfInfo: DotClass)
extends ClassInfo(prefix, cls, classParents, decls, selfInfo)

/** A class for temporary class infos where `parents` are not yet known. */
final class TempClassInfo(prefix: Type, cls: ClassSymbol, decls: Scope, selfInfo: DotClass)
extends CachedClassInfo(prefix, cls, Nil, decls, selfInfo) {

/** A list of actions that were because they rely on the class info of `cls` to
* be no longer temporary. These actions will be performed once `cls` gets a real
* ClassInfo.
*/
var suspensions: List[() => Unit] = Nil
}

object ClassInfo {
def apply(prefix: Type, cls: ClassSymbol, classParents: List[TypeRef], decls: Scope, selfInfo: DotClass = NoType)(implicit ctx: Context) =
unique(new CachedClassInfo(prefix, cls, classParents, decls, selfInfo))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ object Scala2Unpickler {
// `denot.sourceModule.exists` provision i859.scala crashes in the backend.
denot.owner.thisType select denot.sourceModule
else selfInfo
denot.info = ClassInfo(denot.owner.thisType, denot.classSymbol, Nil, decls, ost) // first rough info to avoid CyclicReferences
denot.info = new TempClassInfo(denot.owner.thisType, denot.classSymbol, decls, ost) // first rough info to avoid CyclicReferences
var parentRefs = ctx.normalizeToClassRefs(parents, cls, decls)
if (parentRefs.isEmpty) parentRefs = defn.ObjectType :: Nil
for (tparam <- tparams) {
Expand Down
5 changes: 4 additions & 1 deletion src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,8 @@ class Namer { typer: Typer =>
else createSymbol(self)

// pre-set info, so that parent types can refer to type params
denot.info = ClassInfo(cls.owner.thisType, cls, Nil, decls, selfInfo)
val tempClassInfo = new TempClassInfo(cls.owner.thisType, cls, decls, selfInfo)
denot.info = tempClassInfo

// Ensure constructor is completed so that any parameter accessors
// which have type trees deriving from its parameters can be
Expand All @@ -705,6 +706,8 @@ class Namer { typer: Typer =>

index(rest)(inClassContext(selfInfo))
denot.info = ClassInfo(cls.owner.thisType, cls, parentRefs, decls, selfInfo)
tempClassInfo.suspensions.foreach(_())

Checking.checkWellFormed(cls)
if (isDerivedValueClass(cls)) cls.setFlag(Final)
cls.setApplicableFlags(
Expand Down
2 changes: 1 addition & 1 deletion test/dotc/scala-collections.whitelist
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
./scala-scala/src/library/scala/collection/immutable/Seq.scala
./scala-scala/src/library/scala/collection/mutable/IndexedSeq.scala
./scala-scala/src/library/scala/collection/mutable/ListBuffer.scala
#./scala-scala/src/library/scala/collection/mutable/BufferLike.scala // works under junit, fails under partest, but can't see more info on the cause
./scala-scala/src/library/scala/collection/mutable/BufferLike.scala

./scala-scala/src/library/scala/collection/mutable/ArrayBuilder.scala

Expand Down
25 changes: 25 additions & 0 deletions tests/pos/i1401.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package i1401

trait Subtractable[A, +Repr <: Subtractable[A, Repr]] {
def -(elem: A): Repr
}

trait BufferLike[BA, +This <: BufferLike[BA, This] with Buffer[BA]]
extends Subtractable[BA, This]
{ self : This =>

/* Without fix-#1401:
*
error: overriding method - in trait Subtractable of type (elem: A)This & i1401.Buffer[A];
method - of type (elem: BA)This has incompatible type
def -(elem: BA): This
^
one error found
*/
def -(elem: BA): This
}

trait Buffer[A] extends BufferLike[A, Buffer[A]]



0 comments on commit c37185d

Please sign in to comment.