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

Commit 9f8b7f6

Browse files
committed
Static pickler should not attempt to pickle non-existing fields
In static pickler, don't attempt to pickle a private field that is not one of the declared fields according to Java reflection. Also adds another test.
1 parent 4637393 commit 9f8b7f6

File tree

5 files changed

+72
-66
lines changed

5 files changed

+72
-66
lines changed

core/src/main/scala/pickling/Macros.scala

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -111,34 +111,40 @@ trait PicklerMacros extends Macro {
111111
// for each field, compute a tree for pickling it
112112
// (or empty list, if impossible)
113113

114-
def putField(getterLogic: Tree) = {
115-
def wrap(pickleLogic: Tree) = q"builder.putField(${fir.name}, b => $pickleLogic)"
116-
wrap {
117-
if (fir.tpe.typeSymbol.isEffectivelyFinal) q"""
118-
b.hintStaticallyElidedType()
119-
$getterLogic.pickleInto(b)
120-
""" else q"""
121-
val subPicklee: ${fir.tpe} = $getterLogic
122-
if (subPicklee == null || subPicklee.getClass == classOf[${fir.tpe}]) b.hintDynamicallyElidedType() else ()
123-
subPicklee.pickleInto(b)
124-
"""
125-
}
114+
def pickleLogic(fieldValue: Tree): Tree =
115+
if (fir.tpe.typeSymbol.isEffectivelyFinal) q"""
116+
b.hintStaticallyElidedType()
117+
$fieldValue.pickleInto(b)
118+
""" else q"""
119+
val subPicklee: ${fir.tpe} = $fieldValue
120+
if (subPicklee == null || subPicklee.getClass == classOf[${fir.tpe}]) b.hintDynamicallyElidedType()
121+
subPicklee.pickleInto(b)
122+
"""
123+
124+
def putField(getterLogic: Tree) =
125+
q"builder.putField(${fir.name}, b => ${pickleLogic(getterLogic)})"
126+
127+
// we assume getterLogic is a tree of type Try[${fir.tpe}]
128+
def tryPutField(getterLogic: Tree) = {
129+
val tryName = c.fresh(newTermName("tr"))
130+
val valName = c.fresh(newTermName("value"))
131+
q"""
132+
val $tryName = $getterLogic
133+
if ($tryName.isSuccess) {
134+
val $valName = $tryName.get
135+
builder.putField(${fir.name}, b => ${pickleLogic(Ident(valName))})
136+
}
137+
"""
126138
}
127139

128140
if (sym.isModuleClass) {
129141
Nil
130142
} else if (fir.hasGetter) {
131-
132143
if (fir.isPublic) List(putField(q"picklee.${newTermName(fir.name)}"))
133144
else reflectively("picklee", fir)(fm => putField(q"$fm.get.asInstanceOf[${fir.tpe}]"))
134145
} else {
135-
//val clazz = universe.mirror.runtimeClass(tpe.erasure)
136-
//if (Try(clazz.getDeclaredField(fir.name)).isSuccess) {
137-
// pickle field using reflection
138-
reflectivelyWithoutGetter("picklee", fir)(fvalue => putField(q"$fvalue.asInstanceOf[${fir.tpe}]"))
139-
/*} else {
140-
Nil
141-
}*/
146+
reflectivelyWithoutGetter("picklee", fir)(fvalue =>
147+
tryPutField(q"$fvalue.asInstanceOf[scala.util.Try[${fir.tpe}]]"))
142148
}
143149
})
144150
val endEntry = q"builder.endEntry()"
@@ -267,11 +273,12 @@ trait UnpicklerMacros extends Macro {
267273
// nevertheless don't despair and try to prove whether this is or is not the fact
268274
// i was super scared that string sharing is going to fail due to the same reason, but it did not :)
269275
// in the worst case we can do the same as the interpreted runtime does - just go for allocateInstance
270-
val pendingFields = cir.fields.filter(fir =>
271-
fir.isNonParam ||
272-
(!canCallCtor && fir.isReifiedParam) ||
273-
shouldBotherAboutLooping(fir.tpe)
276+
277+
// pending fields are fields that are restored after instantiation (e.g., through field assignments)
278+
val pendingFields = if (!canCallCtor) cir.fields else cir.fields.filter(fir =>
279+
fir.isNonParam || shouldBotherAboutLooping(fir.tpe)
274280
)
281+
275282
val instantiationLogic = {
276283
if (sym.isModuleClass) {
277284
q"${sym.module}"
@@ -304,7 +311,15 @@ trait UnpicklerMacros extends Macro {
304311
val initPendingFields = pendingFields.flatMap(fir => {
305312
val readFir = readField(fir.name, fir.tpe)
306313
if (fir.isPublic && fir.hasSetter) List(q"$instance.${newTermName(fir.name)} = $readFir".asInstanceOf[Tree])
307-
else if (fir.accessor.isEmpty) List()
314+
else if (fir.accessor.isEmpty) List(q"""
315+
try {
316+
val javaField = $instance.getClass.getDeclaredField(${fir.name})
317+
javaField.setAccessible(true)
318+
javaField.set($instance, $readFir)
319+
} catch {
320+
case e: java.lang.NoSuchFieldException => /* do nothing */
321+
}
322+
""")
308323
else reflectively(instance, fir)(fm => q"$fm.set($readFir)".asInstanceOf[Tree])
309324
})
310325

core/src/main/scala/pickling/Runtime.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,9 @@ class InterpretedUnpicklerRuntime(mirror: Mirror, tag: FastTypeTag[_])(implicit
227227
if (shouldBotherAboutSharing(tpe)) registerUnpicklee(inst, preregisterUnpicklee())
228228
val im = mirror.reflect(inst)
229229

230-
debug(s"pendingFields: ${pendingFields.size}")
231-
debug(s"fieldVals: ${fieldVals.size}")
232-
230+
//debug(s"pendingFields: ${pendingFields.size}")
231+
//debug(s"fieldVals: ${fieldVals.size}")
232+
233233
pendingFields.zip(fieldVals) foreach {
234234
case (fir, fval) =>
235235
if (fir.hasGetter) {

core/src/main/scala/pickling/RuntimePickler.scala

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,13 @@ class RuntimePickler(classLoader: ClassLoader, clazz: Class[_])(implicit pf: Pic
133133
val format: PickleFormat = pf
134134

135135
val fields: List[Logic] = cir.fields.flatMap { fir =>
136-
if (fir.hasGetter) {
137-
//debug(s"pickling field of type ${fir.tpe.key}")
136+
if (fir.hasGetter)
138137
List(
139138
if (fir.tpe.typeSymbol.isEffectivelyFinal) new EffectivelyFinalLogic(fir)
140139
else if (fir.tpe.typeSymbol.asType.isAbstractType) new AbstractLogic(fir)
141140
else new DefaultLogic(fir)
142141
)
143-
} else
142+
else
144143
try {
145144
val javaField = clazz.getDeclaredField(fir.name)
146145
List(
@@ -151,32 +150,14 @@ class RuntimePickler(classLoader: ClassLoader, clazz: Class[_])(implicit pf: Pic
151150
case e: java.lang.NoSuchFieldException => List()
152151
}
153152
}
154-
/*
155-
{
156-
debug(s"cir.fields: ${cir.fields.mkString(",")}")
157-
158-
val regularFields = cir.fields.filter(_.hasGetter)
159-
debug(s"regular fields with getters: ${regularFields.mkString(",")}")
160-
161-
val ctorParamsWithoutGetter =
162-
clazz.getDeclaredFields().filter(df => regularFields.forall(_.name != df.getName))
163-
debug(s"ctor params without getters: ${ctorParamsWithoutGetter.mkString(",")}")
164-
165-
ctorParamsWithoutGetter.map(cp => new CtorParamLogic(cp)).toList ++ regularFields.map { fir =>
166-
debug(s"pickling field of type ${fir.tpe.key}")
167-
if (fir.tpe.typeSymbol.isEffectivelyFinal) new EffectivelyFinalLogic(fir)
168-
else if (fir.tpe.typeSymbol.asType.isAbstractType) new AbstractLogic(fir)
169-
else new DefaultLogic(fir)
170-
}
171-
}
172-
*/
153+
173154
def putFields(picklee: Any, builder: PBuilder): Unit = {
174155
val im = mirror.reflect(picklee)
175156
fields.foreach(_.run(builder, picklee, im))
176157
}
177158

178159
def pickle(picklee: Any, builder: PBuilder): Unit = {
179-
debug(s"pickling object of type: ${tag.key}")
160+
//debug(s"pickling object of type: ${tag.key}")
180161
builder.beginEntry(picklee)
181162
putFields(picklee, builder)
182163
builder.endEntry()

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,9 +379,10 @@ abstract class Macro { self =>
379379
val pickleeName = newTermName(target)
380380
val getFieldValue = q"""
381381
val clazz = $pickleeName.getClass
382-
val javaField: java.lang.reflect.Field = clazz.getDeclaredField(${fir.name})
383-
javaField.setAccessible(true)
384-
javaField.get($pickleeName)
382+
scala.util.Try(clazz.getDeclaredField(${fir.name})).map { javaField =>
383+
javaField.setAccessible(true)
384+
javaField.get($pickleeName)
385+
}
385386
"""
386387
List(body(getFieldValue))
387388
}
Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package scala.pickling.ctorparams
1+
package scala.pickling.test.ctorparams
22

33
import org.scalatest.FunSuite
44
import scala.pickling._
@@ -9,21 +9,30 @@ class Partitioner(numParts: Int) {
99
override def toString = s"Partitioner($numParts)"
1010
}
1111

12-
class AllRuntimeCtorsParamTest extends FunSuite {
13-
test("main") {
12+
class Partitioner2(numParts: Int, val other: String) {
13+
def numPartitions = numParts
14+
override def toString = s"Partitioner2($numParts,$other)"
15+
}
16+
17+
class CtorParamsTest extends FunSuite {
18+
test("runtime pickler") {
1419
val par: Any = new Partitioner(8)
1520
val p: JSONPickle = par.pickle
1621
val up = p.unpickle[Any]
1722
assert(par.toString == up.toString)
1823
}
19-
}
2024

21-
class StaticTriggeredCtorsParamTest extends FunSuite {
22-
test("main") {
23-
// val par = new Partitioner(8)
24-
// val p: JSONPickle = par.pickle
25-
// val up = p.unpickle[Partitioner]
26-
// assert(par.toString == up.toString)
27-
assert(true)
25+
test("static pickler") {
26+
val par = new Partitioner(8)
27+
val p: JSONPickle = par.pickle
28+
val up = p.unpickle[Partitioner]
29+
assert(par.toString == up.toString)
2830
}
29-
}
31+
32+
test("ctor param and public getter") {
33+
val par = new Partitioner2(8, "test")
34+
val p: JSONPickle = par.pickle
35+
val up = p.unpickle[Partitioner2]
36+
assert(par.toString == up.toString)
37+
}
38+
}

0 commit comments

Comments
 (0)