Skip to content

Commit 7ea53e7

Browse files
committed
Mixin fields with trait setters shouldn't be JVM final
Before: ``` scala> trait T { val foo = 24 }; class C extends T defined trait T defined class C scala> :javap -private -c C Compiled from "<console>" public class $line3.$read$$iw$$iw$C implements $line3.$read$$iw$$iw$T { private final int foo; public int foo(); Code: 0: aload_0 1: getfield #21 // Field foo:I 4: ireturn public void $line3$$read$$iw$$iw$T$_setter_$foo_$eq(int); Code: 0: aload_0 1: iload_1 2: putfield #21 // Field foo:I 5: return public $line3.$read$$iw$$iw$C(); Code: 0: aload_0 1: invokespecial #30 // Method java/lang/Object."<init>":()V 4: aload_0 5: invokestatic #34 // InterfaceMethod $line3/$read$$iw$$iw$T.$init$:(L$line3/$read$$iw$$iw$T;)V 8: return } ``` The assignment to the final field `foo` has always contravened the JVM spec, and this rule is enforced for any classfiles of format 53 and higher. After this patch: ``` scala> trait T { val foo = 24 }; class C extends T defined trait T defined class C scala> :javap -private -c C Compiled from "<console>" public class $line3.$read$$iw$$iw$C implements $line3.$read$$iw$$iw$T { private int foo; public int foo(); Code: 0: aload_0 1: getfield #21 // Field foo:I 4: ireturn public void $line3$$read$$iw$$iw$T$_setter_$foo_$eq(int); Code: 0: aload_0 1: iload_1 2: putfield #21 // Field foo:I 5: return public $line3.$read$$iw$$iw$C(); Code: 0: aload_0 1: invokespecial #30 // Method java/lang/Object."<init>":()V 4: aload_0 5: invokestatic #34 // InterfaceMethod $line3/$read$$iw$$iw$T.$init$:(L$line3/$read$$iw$$iw$T;)V 8: getstatic #40 // Field scala/runtime/ScalaRunTime$.MODULE$:Lscala/runtime/ScalaRunTime$; 11: invokevirtual #43 // Method scala/runtime/ScalaRunTime$.releaseFence:()V 14: return } ```
1 parent 577f460 commit 7ea53e7

File tree

7 files changed

+41
-13
lines changed

7 files changed

+41
-13
lines changed

src/compiler/scala/tools/nsc/transform/Constructors.scala

+6-1
Original file line numberDiff line numberDiff line change
@@ -756,11 +756,16 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
756756
if (isDelayedInitSubclass && remainingConstrStats.nonEmpty) delayedInitDefsAndConstrStats(defs, remainingConstrStats)
757757
else (Nil, remainingConstrStats)
758758

759+
val fence = if (clazz.primaryConstructor.hasAttachment[ConstructorNeedsFence.type]) {
760+
val tree = localTyper.typedPos(clazz.primaryConstructor.pos)(gen.mkRuntimeCall(nme.releaseFence, Nil))
761+
tree :: Nil
762+
} else Nil
763+
759764
// Assemble final constructor
760765
val primaryConstructor = deriveDefDef(primaryConstr)(_ => {
761766
treeCopy.Block(
762767
primaryConstrBody,
763-
paramInits ::: constructorPrefix ::: uptoSuperStats ::: guardSpecializedInitializer(remainingConstrStatsDelayedInit),
768+
paramInits ::: constructorPrefix ::: uptoSuperStats ::: guardSpecializedInitializer(remainingConstrStatsDelayedInit) ::: fence,
764769
primaryConstrBody.expr)
765770
})
766771

src/compiler/scala/tools/nsc/transform/Fields.scala

+11-5
Original file line numberDiff line numberDiff line change
@@ -423,12 +423,12 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
423423
// println(s"expanded modules for $clazz: $expandedModules")
424424

425425
// afterOwnPhase, so traits receive trait setters for vals (needs to be at finest grain to avoid looping)
426-
val synthInSubclass =
426+
val synthInSubclass: List[Symbol] =
427427
clazz.mixinClasses.flatMap(mixin => afterOwnPhase{mixin.info}.decls.toList.filter(accessorImplementedInSubclass))
428428

429429
// mixin field accessors --
430430
// invariant: (accessorsMaybeNeedingImpl, mixedInAccessorAndFields).zipped.forall(case (acc, clone :: _) => `clone` is clone of `acc` case _ => true)
431-
val mixedInAccessorAndFields = synthInSubclass.map{ member =>
431+
val mixedInAccessorAndFields: List[List[Symbol]] = synthInSubclass.map{ member =>
432432
def cloneAccessor() = {
433433
val clonedAccessor = (member cloneSymbol clazz) setPos clazz.pos
434434
setMixedinAccessorFlags(member, clonedAccessor)
@@ -653,9 +653,15 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
653653
// trait val/var setter mixed into class
654654
else fieldAccess(setter) match {
655655
case NoSymbol => EmptyTree
656-
case fieldSel => afterOwnPhase { // the assign only type checks after our phase (assignment to val)
657-
mkAccessor(setter)(Assign(Select(This(clazz), fieldSel), castHack(Ident(setter.firstParam), fieldSel.info)))
658-
}
656+
case fieldSel =>
657+
if (!fieldSel.hasFlag(MUTABLE)) {
658+
fieldSel.setFlag(MUTABLE)
659+
fieldSel.owner.primaryConstructor.updateAttachment(ConstructorNeedsFence)
660+
}
661+
662+
afterOwnPhase { // the assign only type checks after our phase (assignment to val)
663+
mkAccessor(setter)(Assign(Select(This(clazz), fieldSel), castHack(Ident(setter.firstParam), fieldSel.info)))
664+
}
659665
}
660666

661667
def moduleAccessorBody(module: Symbol): Tree =

src/reflect/scala/reflect/internal/StdAttachments.scala

+2
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,6 @@ trait StdAttachments {
107107
case object KnownDirectSubclassesCalled extends PlainAttachment
108108

109109
class QualTypeSymAttachment(val sym: Symbol)
110+
111+
case object ConstructorNeedsFence extends PlainAttachment
110112
}

src/reflect/scala/reflect/internal/StdNames.scala

+1
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,7 @@ trait StdNames {
756756
val productPrefix: NameType = "productPrefix"
757757
val raw_ : NameType = "raw"
758758
val readResolve: NameType = "readResolve"
759+
val releaseFence: NameType = "releaseFence"
759760
val reify : NameType = "reify"
760761
val reificationSupport : NameType = "reificationSupport"
761762
val rootMirror : NameType = "rootMirror"

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse =>
5151
this.UseInvokeSpecial
5252
this.TypeParamVarargsAttachment
5353
this.KnownDirectSubclassesCalled
54+
this.ConstructorNeedsFence
5455
this.noPrint
5556
this.typeDebug
5657
this.Range

test/files/run/lazy-locals-2.scala

+18-5
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,8 @@ object Test {
281281
lzyComputeMethods == expComputeMethods,
282282
s"wrong lzycompute methods. expected:\n$expComputeMethods\nfound:\n$lzyComputeMethods")
283283

284-
val fields = c.getClass.getDeclaredFields.toList.sortBy(_.getName).map(_.toString)
285-
val expFields = List(
284+
val fields: List[String] = c.getClass.getDeclaredFields.toList.sortBy(_.getName).map(_.toString)
285+
val expFields = List[String](
286286
"private volatile byte C.bitmap$0",
287287
"private int C.lvl1",
288288
"private java.lang.String C.lvl2",
@@ -305,9 +305,22 @@ object Test {
305305
d.run()
306306

307307
val dFields = d.getClass.getDeclaredFields.toList.sortBy(_.getName).map(_.toString)
308-
assert(
309-
dFields == expFields.map(_.replaceAll(" C.", " D.")),
310-
s"wrong fields. expected:\n$expFields\nfound:\n$fields")
308+
val expDFields = List[String](
309+
"private volatile byte D.bitmap$0",
310+
"private int D.lvl1",
311+
"private java.lang.String D.lvl2",
312+
"private scala.runtime.BoxedUnit D.lvl3",
313+
"private int D.t1",
314+
"private java.lang.String D.t2",
315+
"private scala.runtime.BoxedUnit D.t3",
316+
"private int D.vl1",
317+
"private java.lang.String D.vl2",
318+
"private scala.runtime.BoxedUnit D.vl3",
319+
"private int D.vr1",
320+
"private java.lang.String D.vr2",
321+
"private scala.runtime.BoxedUnit D.vr3")
322+
assert(dFields == expDFields,
323+
s"wrong fields. expected:\n$expDFields\nfound:\n$dFields")
311324

312325

313326
val d1 = new D1

test/files/run/t10075b.check

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@
4545
@RetainedAnnotation() public int TMix.lzyValGetterAnnotation()
4646
private int TMix.lzyValGetterAnnotation$lzycompute()
4747
@RetainedAnnotation() public int TMix.method()
48-
@RetainedAnnotation() private final int TMix.valFieldAnnotation
48+
@RetainedAnnotation() private int TMix.valFieldAnnotation
4949
public int TMix.valFieldAnnotation()
50-
private final int TMix.valGetterAnnotation
50+
private int TMix.valGetterAnnotation
5151
@RetainedAnnotation() public int TMix.valGetterAnnotation()
5252
@RetainedAnnotation() private int TMix.varFieldAnnotation
5353
public int TMix.varFieldAnnotation()

0 commit comments

Comments
 (0)