Skip to content

Commit

Permalink
SI-8708 Fix pickling of LOCAL_CHILD child of sealed classes
Browse files Browse the repository at this point in the history
When a sealed class or trait has local children, they are not pickled
in as part of the children of the symbol (introduced in 12a2b3b to fix
Aladdin bug 1055). Instead the compiler adds a single child class
named LOCAL_CHILD. The parents of its ClassInfoType were wrong: the
first parent should be a class. For sealed traits, we were using the
trait itself.

Also, the LOCAL_CHILD dummy class was entered as a member of its
enclosing class, which is wrong: it represents a local (non-member)
class, and it's a synthetic dummy anyway.
  • Loading branch information
lrytz committed Jul 7, 2014
1 parent 3f79f8e commit 14fa7be
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 6 deletions.
11 changes: 10 additions & 1 deletion src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,16 @@ abstract class Pickler extends SubComponent {
val (locals, globals) = sym.children partition (_.isLocalClass)
val children =
if (locals.isEmpty) globals
else globals + sym.newClassWithInfo(tpnme.LOCAL_CHILD, List(sym.tpe), EmptyScope, pos = sym.pos)
else {
// The LOCAL_CHILD was introduced in 12a2b3b to fix Aladdin bug 1055. When a sealed
// class/trait has local subclasses, a single <local child> class symbol is added
// as pickled child (instead of a reference to the anonymous class; that was done
// initially, but seems not to work, as the bug shows).
// Adding the LOCAL_CHILD is necessary to retain exhaustivity warnings under separate
// compilation. See test neg/aladdin1055.
val parents = (if (sym.isTrait) List(definitions.ObjectTpe) else Nil) ::: List(sym.tpe)
globals + sym.newClassWithInfo(tpnme.LOCAL_CHILD, parents, EmptyScope, pos = sym.pos)
}

putChildren(sym, children.toList sortBy (_.sealedSortName))
}
Expand Down
42 changes: 37 additions & 5 deletions src/reflect/scala/reflect/internal/pickling/UnPickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,25 @@ abstract class UnPickler {
def pflags = flags & PickledFlags

def finishSym(sym: Symbol): Symbol = {
/**
* member symbols (symbols owned by a class) are added to the class's scope, with a number
* of exceptions:
*
* (.) ...
* (1) `local child` represents local child classes, see comment in Pickler.putSymbol.
* Since it is not a member, it should not be entered in the owner's scope.
*/
def shouldEnterInOwnerScope = {
sym.owner.isClass &&
sym != classRoot &&
sym != moduleRoot &&
!sym.isModuleClass &&
!sym.isRefinementClass &&
!sym.isTypeParameter &&
!sym.isExistentiallyBound &&
sym.rawname != tpnme.LOCAL_CHILD // (1)
}

markFlagsCompleted(sym)(mask = AllFlags)
sym.privateWithin = privateWithin
sym.info = (
Expand All @@ -302,8 +321,7 @@ abstract class UnPickler {
newLazyTypeRefAndAlias(inforef, readNat())
}
)
if (sym.owner.isClass && sym != classRoot && sym != moduleRoot &&
!sym.isModuleClass && !sym.isRefinementClass && !sym.isTypeParameter && !sym.isExistentiallyBound)
if (shouldEnterInOwnerScope)
symScope(sym.owner) enter sym

sym
Expand Down Expand Up @@ -681,10 +699,24 @@ abstract class UnPickler {
private val p = phase
protected def completeInternal(sym: Symbol) : Unit = try {
val tp = at(i, () => readType(sym.isTerm)) // after NMT_TRANSITION, revert `() => readType(sym.isTerm)` to `readType`
if (p ne null)
slowButSafeEnteringPhase(p) (sym setInfo tp)

// This is a temporary fix allowing to read classes generated by an older, buggy pickler.
// See the generation of the LOCAL_CHILD class in Pickler.scala. In an earlier version, the
// pickler did not add the ObjectTpe superclass, it used a trait as the first parent. This
// tripped an assertion in AddInterfaces which checks that the first parent is not a trait.
// This workaround can probably be removed in 2.12, because the 2.12 compiler is supposed
// to only read classfiles generated by 2.12.
val fixLocalChildTp = if (sym.rawname == tpnme.LOCAL_CHILD) tp match {
case ClassInfoType(superClass :: traits, decls, typeSymbol) if superClass.typeSymbol.isTrait =>
ClassInfoType(definitions.ObjectTpe :: superClass :: traits, decls, typeSymbol)
case _ => tp
} else tp

if (p ne null) {
slowButSafeEnteringPhase(p)(sym setInfo fixLocalChildTp)
}
if (currentRunId != definedAtRunId)
sym.setInfo(adaptToNewRunMap(tp))
sym.setInfo(adaptToNewRunMap(fixLocalChildTp))
}
catch {
case e: MissingRequirementError => throw toTypeError(e)
Expand Down
7 changes: 7 additions & 0 deletions test/files/neg/aladdin1055.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Test_1.scala:2: warning: match may not be exhaustive.
It would fail on the following input: (_ : this.<local child>)
def foo(t: A.T) = t match {
^
error: No warnings can be incurred under -Xfatal-warnings.
one warning found
one error found
1 change: 1 addition & 0 deletions test/files/neg/aladdin1055.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-Xfatal-warnings
6 changes: 6 additions & 0 deletions test/files/neg/aladdin1055/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
object A {
sealed trait T { def f: Int }
class TT extends T { def f = 0 }

def foo = new T { def f = 1 } // local subclass of sealed trait T
}
5 changes: 5 additions & 0 deletions test/files/neg/aladdin1055/Test_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object Test {
def foo(t: A.T) = t match {
case a: A.TT => 0
}
}
6 changes: 6 additions & 0 deletions test/files/pos/t8708/Either_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
sealed trait \/[+A, +B]

sealed trait EitherT[F[+_], +A, +B]
object EitherT {
def apply[F[+_], A, B](a: F[A \/ B]): EitherT[F, A, B] = new EitherT[F, A, B] { val run = a }
}
13 changes: 13 additions & 0 deletions test/files/pos/t8708/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import scala.language.higherKinds

trait ClientTypes[M[+_]] {
final type Context[+A] = EitherT[M, String, A]
object Context {
def apply[A](ca: M[String \/ A]): Context[A] = EitherT[M, String, A](ca)
}

final type StatefulContext[+A] = EitherT[Context, String, A]
object StatefulContext {
def apply[A](state: Context[String \/ A]): StatefulContext[A] = ???
}
}
8 changes: 8 additions & 0 deletions test/files/run/t8708_b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Scope{
def <init>: <?>;
sealed abstract trait T extends ;
def foo: <?>
}
Scope{
def f: <?>
}
8 changes: 8 additions & 0 deletions test/files/run/t8708_b/A_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package p

class C {

sealed trait T { def f: Int }

def foo: T = new T { def f = 1 }
}
21 changes: 21 additions & 0 deletions test/files/run/t8708_b/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import scala.tools.partest._
import java.io.{Console => _, _}

object Test extends DirectTest {

override def extraSettings: String = "-usejavacp -cp " + testOutput.path

override def code = ""

override def show(): Unit = {
val g = newCompiler()
withRun(g)(r => {
val c = g.rootMirror.getRequiredClass("p.C")
println(c.info.decls)
val t = c.info.member(g.newTypeName("T"))
// this test ensrues that the <local child> dummy class symbol is not entered in the
// scope of trait T during unpickling.
println(t.info.decls)
})
}
}

0 comments on commit 14fa7be

Please sign in to comment.