Skip to content

Commit ddff9bd

Browse files
committed
Emit serialVersionUID fields as private.
The JVM serialization code doesn't care either way, and leaving them as public leads to annoying (but mostly harmless) MiMa errors if the annotation is added. Since the generated field is synthetic, it can only be referenced from Java, so privatizing it is unlikely to break anyone's use of it, or so I'd hope. See scala#6101 and scala#6138 for examples of places where this has caused troubles before.
1 parent a40e8fa commit ddff9bd

File tree

9 files changed

+75
-10
lines changed

9 files changed

+75
-10
lines changed

src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -864,14 +864,16 @@ abstract class BCodeHelpers extends BCodeIdiomatic {
864864
val MIN_SWITCH_DENSITY = 0.7
865865

866866
/*
867-
* Add public static final field serialVersionUID with value `id`
867+
* Add private static final field serialVersionUID with value `id`.
868868
*
869869
* can-multi-thread
870870
*/
871871
def addSerialVUID(id: Long, jclass: asm.ClassVisitor) {
872872
// add static serialVersionUID field if `clasz` annotated with `@SerialVersionUID(uid: Long)`
873+
// private for ease of binary compatibility (docs for java.io.Serializable
874+
// claim that the access modifier can be anything we want).
873875
jclass.visitField(
874-
GenBCode.PublicStaticFinal,
876+
GenBCode.PrivateStaticFinal,
875877
"serialVersionUID",
876878
"J",
877879
null, // no java-generic-signature

src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ object GenBCode {
8181

8282
final val PublicStatic = Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC
8383
final val PublicStaticFinal = Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL
84+
final val PrivateStaticFinal = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_FINAL
8485

8586
val CLASS_CONSTRUCTOR_NAME = "<clinit>"
8687
val INSTANCE_CONSTRUCTOR_NAME = "<init>"

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,6 +1430,14 @@ abstract class RefChecks extends Transform {
14301430
analyzer.ImplicitNotFoundMsg.check(sym) foreach messageWarning("implicitNotFound")
14311431
analyzer.ImplicitAmbiguousMsg.check(sym) foreach messageWarning("implicitAmbiguous")
14321432

1433+
if (sym.isClass && sym.hasAnnotation(SerialVersionUIDAttr)) {
1434+
def warn(what: String) =
1435+
reporter.warning(tree.pos, s"@serialVersionUID has no effect on $what")
1436+
1437+
if (sym.isTrait) warn("traits")
1438+
else if (!sym.isSerializable) warn("non-serializable classes")
1439+
}
1440+
14331441
case tpt@TypeTree() =>
14341442
if (tpt.original != null) {
14351443
tpt.original foreach {
Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* __ *\
22
** ________ ___ / / ___ Scala API **
3-
** / __/ __// _ | / / / _ | (c) 2002-2013, LAMP/EPFL **
3+
** / __/ __// _ | / / / _ | (c) 2002-2017, LAMP/EPFL **
44
** __\ \/ /__/ __ |/ /__/ __ | http://scala-lang.org/ **
55
** /____/\___/_/ |_/____/_/ | | **
66
** |/ **
@@ -9,7 +9,14 @@
99
package scala
1010

1111
/**
12-
* Annotation for specifying the `static SerialVersionUID` field
13-
* of a serializable class.
14-
*/
12+
* Annotation for specifying the `serialVersionUID` field of a (serializable) class.
13+
*
14+
* On the JVM, a class with this annotation will receive a `private`, `static`,
15+
* and `final` field called `serialVersionUID` with the provided [[value]],
16+
* which the JVM's serialization mechanism uses to determine serialization
17+
* compatibility between different versions of a class.
18+
*
19+
* @see [[http://docs.oracle.com/javase/8/docs/api/java/io/Serializable.html `java.io.Serializable`]]
20+
* @see [[Serializable]]
21+
*/
1522
class SerialVersionUID(value: Long) extends scala.annotation.ClassfileAnnotation
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
warn-useless-svuid.scala:2: warning: @serialVersionUID has no effect on non-serializable classes
2+
class X
3+
^
4+
warn-useless-svuid.scala:5: warning: @serialVersionUID has no effect on non-serializable classes
5+
class Y extends X
6+
^
7+
warn-useless-svuid.scala:17: warning: @serialVersionUID has no effect on traits
8+
trait T
9+
^
10+
warn-useless-svuid.scala:20: warning: @serialVersionUID has no effect on traits
11+
trait U extends scala.Serializable
12+
^
13+
warn-useless-svuid.scala:23: warning: @serialVersionUID has no effect on traits
14+
trait V extends java.io.Serializable
15+
^
16+
error: No warnings can be incurred under -Xfatal-warnings.
17+
5 warnings found
18+
one error found
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-Xfatal-warnings
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
@SerialVersionUID(1L)
2+
class X
3+
4+
@SerialVersionUID(1L)
5+
class Y extends X
6+
7+
@SerialVersionUID(1L)
8+
class Z extends scala.Serializable
9+
10+
@SerialVersionUID(1L)
11+
class W extends java.io.Serializable
12+
13+
@SerialVersionUID(1L)
14+
class Q extends Z
15+
16+
@SerialVersionUID(1L)
17+
trait T
18+
19+
@SerialVersionUID(1L)
20+
trait U extends scala.Serializable
21+
22+
@SerialVersionUID(1L)
23+
trait V extends java.io.Serializable

test/files/run/t8549b.scala

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
1+
import java.lang.reflect.Modifier._
12

23
@SerialVersionUID(42)
3-
class C
4+
class C extends Serializable
45

56
@SerialVersionUID(43 - 1)
6-
class D
7+
class D extends Serializable
78

89

910
object Test extends App {
1011
def checkId(cls: Class[_]) {
11-
val id = cls.getDeclaredField("serialVersionUID").get(null)
12-
assert(id == 42, (cls, id))
12+
val field = cls.getDeclaredField("serialVersionUID")
13+
assert(isPrivate(field.getModifiers))
14+
field.setAccessible(true)
15+
val id = field.get(null)
16+
assert(id == 42, (cls, id))
1317
}
1418
checkId(classOf[C])
1519
checkId(classOf[D])

test/files/run/t8960.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ object Test extends App {
88
o.getClass.getName)
99

1010
val Some(f) = o.getClass.getDeclaredFields.find(_.getName == "serialVersionUID")
11+
f.setAccessible(true)
1112
assert(f.getLong(null) == 0l)
1213
}
1314

0 commit comments

Comments
 (0)