Skip to content
This repository was archived by the owner on Feb 20, 2019. It is now read-only.

Commit 95501ff

Browse files
committed
Handle sealed classes in check for non-cyclic types
When deciding to delay the initialization of fields due to possible cycles, sealed classes were not handled. This could cause issues, such as #128, where the initialization of a constructor parameter was delayed and thus the parameter was null during the execution of the constructor.
1 parent c64d5d6 commit 95501ff

File tree

2 files changed

+56
-7
lines changed

2 files changed

+56
-7
lines changed

core/src/main/scala/pickling/Tools.scala

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -206,13 +206,40 @@ abstract class ShareAnalyzer[U <: Universe](val u: U) {
206206
if (visited(currTpe)) {
207207
if (tpe <:< currTpe) true // TODO: make sure this sanely works for polymorphic types
208208
else loop(rest, visited)
209-
} else if (currTpe.isNotNullable || currTpe.isEffectivelyPrimitive || currSym == StringClass || currSym.isModuleClass) loop(rest, visited)
210-
// TODO: extend the traversal logic to support sealed classes
211-
// when doing that don't forget:
212-
// 1) sealeds can themselves be extended, so we need to recur
213-
// 2) the entire sealed hierarchy should be added to todo
214-
else if (!currSym.isFinal) true // NOTE: returning true here is important for soundness!
215-
else {
209+
} else if (currTpe.isNotNullable || currTpe.isEffectivelyPrimitive || currSym == StringClass || currSym.isModuleClass) {
210+
loop(rest, visited)
211+
} else if (currSym.isClass && currSym.asClass.isSealed) {
212+
val currTargs: List[Type] =
213+
currTpe.asInstanceOf[scala.reflect.internal.SymbolTable#Type]
214+
.dealias
215+
.typeArgs
216+
.map(_.asInstanceOf[Type])
217+
218+
// field types have to be OK
219+
val more = flattenedClassIR(currTpe).fields.map(_.tpe)
220+
221+
// all known subclasses have to be OK, too
222+
val ksc = currSym.asClass.knownDirectSubclasses
223+
val subclasses = ksc.map(sym => sym.asClass.toTypeIn(currTpe))
224+
225+
// collect field types of all subclasses
226+
val fieldTypes = subclasses.flatMap { subclassTpe =>
227+
flattenedClassIR(subclassTpe).fields.map(_.tpe)
228+
}
229+
230+
// for field types that are type parameters,
231+
// use type arguments of currTpe (currTargs) instead
232+
val fieldTypesToCheck = fieldTypes.flatMap { tp =>
233+
if (tp.typeSymbol.isParameter) currTargs
234+
else List(tp)
235+
}
236+
237+
val allTodo = rest ++ more ++ fieldTypesToCheck
238+
loop(allTodo, visited + currTpe)
239+
}
240+
else if (!currSym.isFinal) {
241+
true // NOTE: returning true here is important for soundness!
242+
} else {
216243
val more = flattenedClassIR(currTpe).fields.map(_.tpe)
217244
loop(rest ++ more, visited + currTpe)
218245
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package scala.pickling.test.issue128
2+
3+
import scala.pickling._
4+
import scala.pickling.json._
5+
6+
import org.scalatest.FunSuite
7+
8+
case class A(intOpt: Option[Int]) {
9+
intOpt match {
10+
case Some(int) =>
11+
case None =>
12+
}
13+
}
14+
15+
class Issue128Test extends FunSuite {
16+
test("Issue #128") {
17+
val opt = Some(5)
18+
val a = A(opt)
19+
val pickle = a.pickle
20+
assert(pickle.unpickle[A] === a)
21+
}
22+
}

0 commit comments

Comments
 (0)