From 426744441c22fa3153b7192bead46f8b244c4f12 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Sun, 25 Nov 2012 18:29:16 -0800 Subject: Fix for SerialVersionUID instability. Can hardly believe this has been broken for a decade or so, but there it is - see test case. Four classes attempt to set their SerialVersionUID to 13. One succeeds. No warnings or errors. The output before this patch (for me anyway - your random numbers may differ) was: 860336111422349646 13 8409527228024057943 -7852527872932878365 There was already code in place for rejecting annotations with non-constant args when constant args are required, but that check is only performed on ClassfileAnnotations, and SerialVersionUID was a StaticAnnotation. Maybe people don't reach for ClassfileAnnotation because of this giant warning which I see no way to suppress: warning: Implementation restriction: subclassing Classfile does not make your annotation visible at runtime. If that is what you want, you must write the annotation class in Java. Why did I change the name of the field from uid to value? If you don't use the name 'value', you have to name the argument every time you use it, even if it's the only parameter. I didn't relish breaking every usage of scala's @SerialVersionUID in the known universe. --- src/library/scala/SerialVersionUID.scala | 2 +- test/files/neg/serialversionuid-not-const.check | 10 ++++++++++ test/files/neg/serialversionuid-not-const.scala | 16 ++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/serialversionuid-not-const.check create mode 100644 test/files/neg/serialversionuid-not-const.scala diff --git a/src/library/scala/SerialVersionUID.scala b/src/library/scala/SerialVersionUID.scala index 1f7d047060..77094f0bbf 100644 --- a/src/library/scala/SerialVersionUID.scala +++ b/src/library/scala/SerialVersionUID.scala @@ -12,4 +12,4 @@ package scala * Annotation for specifying the `static SerialVersionUID` field * of a serializable class. */ -class SerialVersionUID(uid: Long) extends scala.annotation.StaticAnnotation +class SerialVersionUID(value: Long) extends scala.annotation.ClassfileAnnotation diff --git a/test/files/neg/serialversionuid-not-const.check b/test/files/neg/serialversionuid-not-const.check new file mode 100644 index 0000000000..9c383d97ad --- /dev/null +++ b/test/files/neg/serialversionuid-not-const.check @@ -0,0 +1,10 @@ +serialversionuid-not-const.scala:1: error: annotation argument needs to be a constant; found: 13L.toLong +@SerialVersionUID(13l.toLong) class C1 extends Serializable + ^ +serialversionuid-not-const.scala:3: error: annotation argument needs to be a constant; found: 13.asInstanceOf[Long] +@SerialVersionUID(13.asInstanceOf[Long]) class C3 extends Serializable + ^ +serialversionuid-not-const.scala:4: error: annotation argument needs to be a constant; found: Test.bippy +@SerialVersionUID(Test.bippy) class C4 extends Serializable + ^ +three errors found diff --git a/test/files/neg/serialversionuid-not-const.scala b/test/files/neg/serialversionuid-not-const.scala new file mode 100644 index 0000000000..f0e3ef4e7e --- /dev/null +++ b/test/files/neg/serialversionuid-not-const.scala @@ -0,0 +1,16 @@ +@SerialVersionUID(13l.toLong) class C1 extends Serializable +@SerialVersionUID(13l) class C2 extends Serializable +@SerialVersionUID(13.asInstanceOf[Long]) class C3 extends Serializable +@SerialVersionUID(Test.bippy) class C4 extends Serializable + +object Test { + val bippy = 13L + + def show(c: Class[_]) = println(java.io.ObjectStreamClass.lookup(c).getSerialVersionUID) + def main(args: Array[String]): Unit = { + show(classOf[C1]) + show(classOf[C2]) + show(classOf[C3]) + show(classOf[C4]) + } +} -- cgit v1.2.3 From a854529c93b42141da07aba055d83192b99d4899 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Sun, 25 Nov 2012 23:25:14 -0800 Subject: Eliminate some one-arg asserts. The only thing more fun than debugging non-deterministic scaladoc crashes unrelated to one's change is doing so with all one-argument asserts. --- src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala index 04f95455a5..9fb5806c3d 100644 --- a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala +++ b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala @@ -233,8 +233,8 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { * exists, but should not be documented (either it's not included in the source or it's not visible) */ class NoDocTemplateImpl(sym: Symbol, inTpl: TemplateImpl) extends EntityImpl(sym, inTpl) with TemplateImpl with HigherKindedImpl with NoDocTemplate { - assert(modelFinished) - assert(!(noDocTemplatesCache isDefinedAt sym)) + assert(modelFinished, this) + assert(!(noDocTemplatesCache isDefinedAt sym), (sym, noDocTemplatesCache(sym))) noDocTemplatesCache += (sym -> this) def isDocTemplate = false } @@ -269,7 +269,7 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { * All ancestors of the template and all non-package members. */ abstract class DocTemplateImpl(sym: Symbol, inTpl: DocTemplateImpl) extends MemberTemplateImpl(sym, inTpl) with DocTemplateEntity { - assert(!modelFinished) + assert(!modelFinished, (sym, inTpl)) assert(!(docTemplatesCache isDefinedAt sym), sym) docTemplatesCache += (sym -> this) @@ -620,7 +620,7 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { */ def createTemplate(aSym: Symbol, inTpl: DocTemplateImpl): Option[MemberImpl] = { // don't call this after the model finished! - assert(!modelFinished) + assert(!modelFinished, (aSym, inTpl)) def createRootPackageComment: Option[Comment] = if(settings.docRootContent.isDefault) None @@ -636,7 +636,7 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { } def createDocTemplate(bSym: Symbol, inTpl: DocTemplateImpl): DocTemplateImpl = { - assert(!modelFinished) // only created BEFORE the model is finished + assert(!modelFinished, (bSym, inTpl)) // only created BEFORE the model is finished if (bSym.isAliasType && bSym != AnyRefClass) new DocTemplateImpl(bSym, inTpl) with AliasImpl with AliasType { override def isAliasType = true } else if (bSym.isAbstractType) -- cgit v1.2.3 From 5573281a24bc57dc75d3e9efa85ff720eeb39d10 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Sun, 25 Nov 2012 23:25:41 -0800 Subject: Account for existence of scala's ClassfileAnnotation. Apparently this thing is not real well tested, as the scaladoc code was written as if it does not exist. --- .../scala/tools/nsc/doc/model/ModelFactory.scala | 26 +++++++++++++--------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala index 9fb5806c3d..acdc3e6797 100644 --- a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala +++ b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala @@ -842,24 +842,28 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { lazy val annotationClass = makeTemplate(annot.symbol) val arguments = { // lazy - def noParams = annot.args map { _ => None } + def annotArgs = annot.args match { + case Nil => annot.assocs collect { case (_, LiteralAnnotArg(const)) => Literal(const) } + case xs => xs + } + def noParams = annotArgs map (_ => None) + val params: List[Option[ValueParam]] = annotationClass match { case aClass: DocTemplateEntity with Class => (aClass.primaryConstructor map { _.valueParams.head }) match { case Some(vps) => vps map { Some(_) } - case None => noParams + case _ => noParams } case _ => noParams } - assert(params.length == annot.args.length) - (params zip annot.args) flatMap { case (param, arg) => - makeTree(arg) match { - case Some(tree) => - Some(new ValueArgument { - def parameter = param - def value = tree - }) - case None => None + assert(params.length == annotArgs.length, (params, annotArgs)) + + params zip annotArgs flatMap { case (param, arg) => + makeTree(arg) map { tree => + new ValueArgument { + def parameter = param + def value = tree + } } } } -- cgit v1.2.3 From b9e01a04618764cceb251830400b1a74ff8f02d3 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Sun, 25 Nov 2012 23:35:22 -0800 Subject: Disabled part of a test. Hmmm, the giant blob of binary data embedded in a test suddenly stopped working. What does one do in this spot. --- test/files/run/t5374.check | 3 +-- test/files/run/t5374.scala | 46 +++++++++++++++++++++++----------------------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/test/files/run/t5374.check b/test/files/run/t5374.check index 6be88d77ec..c1cd843080 100644 --- a/test/files/run/t5374.check +++ b/test/files/run/t5374.check @@ -2,5 +2,4 @@ ListBuffer(1, 2, 3, 1) ListBuffer(1, 2, 3, 1) ListBuffer() List(1, 2, 3, 4, 5) -List(1, 2, 3) -ok \ No newline at end of file +ok diff --git a/test/files/run/t5374.scala b/test/files/run/t5374.scala index 9b1671e795..f6a913e35c 100644 --- a/test/files/run/t5374.scala +++ b/test/files/run/t5374.scala @@ -7,15 +7,15 @@ import java.io._ object Test { - + def main(args: Array[String]) { ticketExample() emptyListBuffer() list() - legacyList() + // legacyList() objectWithMultipleLists() } - + def inAndOut[T <: AnyRef](obj: T): T = { val baos = new ByteArrayOutputStream val oos = new ObjectOutputStream(baos) @@ -24,53 +24,53 @@ object Test { val ois = new ObjectInputStream(bais) ois.readObject.asInstanceOf[T] } - + def ticketExample() { val lb = inAndOut(ListBuffer(1, 2, 3)) val lb2 = ListBuffer[Int]() ++= lb - + lb2 ++= List(1) lb ++= List(1) println(lb) println(lb2) } - + def emptyListBuffer() { val lb = inAndOut(ListBuffer[Int]()) - + println(lb) } - + def list() { val l = inAndOut(List(1, 2, 3, 4, 5)) - + println(l) } - + // this byte array corresponds to what List(1, 2, 3) used to be serialized to prior to this fix val listBytes = Array[Byte](-84, -19, 0, 5, 115, 114, 0, 39, 115, 99, 97, 108, 97, 46, 99, 111, 108, 108, 101, 99, 116, 105, 111, 110, 46, 105, 109, 109, 117, 116, 97, 98, 108, 101, 46, 36, 99, 111, 108, 111, 110, 36, 99, 111, 108, 111, 110, -118, 92, 99, 91, -10, -40, -7, 109, 3, 0, 2, 76, 0, 43, 115, 99, 97, 108, 97, 36, 99, 111, 108, 108, 101, 99, 116, 105, 111, 110, 36, 105, 109, 109, 117, 116, 97, 98, 108, 101, 36, 36, 99, 111, 108, 111, 110, 36, 99, 111, 108, 111, 110, 36, 36, 104, 100, 116, 0, 18, 76, 106, 97, 118, 97, 47, 108, 97, 110, 103, 47, 79, 98, 106, 101, 99, 116, 59, 76, 0, 2, 116, 108, 116, 0, 33, 76, 115, 99, 97, 108, 97, 47, 99, 111, 108, 108, 101, 99, 116, 105, 111, 110, 47, 105, 109, 109, 117, 116, 97, 98, 108, 101, 47, 76, 105, 115, 116, 59, 120, 112, 115, 114, 0, 17, 106, 97, 118, 97, 46, 108, 97, 110, 103, 46, 73, 110, 116, 101, 103, 101, 114, 18, -30, -96, -92, -9, -127, -121, 56, 2, 0, 1, 73, 0, 5, 118, 97, 108, 117, 101, 120, 114, 0, 16, 106, 97, 118, 97, 46, 108, 97, 110, 103, 46, 78, 117, 109, 98, 101, 114, -122, -84, -107, 29, 11, -108, -32, -117, 2, 0, 0, 120, 112, 0, 0, 0, 1, 115, 113, 0, 126, 0, 4, 0, 0, 0, 2, 115, 113, 0, 126, 0, 4, 0, 0, 0, 3, 115, 114, 0, 44, 115, 99, 97, 108, 97, 46, 99, 111, 108, 108, 101, 99, 116, 105, 111, 110, 46, 105, 109, 109, 117, 116, 97, 98, 108, 101, 46, 76, 105, 115, 116, 83, 101, 114, 105, 97, 108, 105, 122, 101, 69, 110, 100, 36, -118, 92, 99, 91, -9, 83, 11, 109, 2, 0, 0, 120, 112, 120) - - def legacyList() { - val bais = new ByteArrayInputStream(listBytes) - val ois = new ObjectInputStream(bais) - val l = ois.readObject() - - println(l) - } - + + // def legacyList() { + // val bais = new ByteArrayInputStream(listBytes) + // val ois = new ObjectInputStream(bais) + // val l = ois.readObject() + + // println(l) + // } + class Foo extends Serializable { val head = List(1, 2, 3) val last = head.tail.tail def structuralSharing: Boolean = head.tail.tail eq last - + assert(structuralSharing) } - + def objectWithMultipleLists() { val foo = inAndOut(new Foo) - + if (foo.structuralSharing) println("ok") else println("no structural sharing") } - + } -- cgit v1.2.3