From 956121dc4b8d1c734edd88b8771ba2e036f71ac3 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 4 May 2018 14:53:04 +1000 Subject: [PATCH] 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 "" 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."":()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 "" 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."":()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 } ``` --- .../scala/tools/nsc/transform/Constructors.scala | 7 ++++++- .../scala/tools/nsc/transform/Fields.scala | 16 +++++++++++----- .../scala/reflect/internal/StdAttachments.scala | 2 ++ .../scala/reflect/internal/StdNames.scala | 1 + 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala index e3d33fb2b548..99b3315b69c0 100644 --- a/src/compiler/scala/tools/nsc/transform/Constructors.scala +++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala @@ -756,11 +756,16 @@ abstract class Constructors extends Statics with Transform with TypingTransforme if (isDelayedInitSubclass && remainingConstrStats.nonEmpty) delayedInitDefsAndConstrStats(defs, remainingConstrStats) else (Nil, remainingConstrStats) + val fence = if (clazz.primaryConstructor.hasAttachment[ConstructorNeedsFence.type]) { + val tree = localTyper.typedPos(clazz.primaryConstructor.pos)(gen.mkRuntimeCall(nme.releaseFence, Nil)) + tree :: Nil + } else Nil + // Assemble final constructor val primaryConstructor = deriveDefDef(primaryConstr)(_ => { treeCopy.Block( primaryConstrBody, - paramInits ::: constructorPrefix ::: uptoSuperStats ::: guardSpecializedInitializer(remainingConstrStatsDelayedInit), + paramInits ::: constructorPrefix ::: uptoSuperStats ::: guardSpecializedInitializer(remainingConstrStatsDelayedInit) ::: fence, primaryConstrBody.expr) }) diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index 39323abacf8c..2d50c76d3224 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -423,12 +423,12 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor // println(s"expanded modules for $clazz: $expandedModules") // afterOwnPhase, so traits receive trait setters for vals (needs to be at finest grain to avoid looping) - val synthInSubclass = + val synthInSubclass: List[Symbol] = clazz.mixinClasses.flatMap(mixin => afterOwnPhase{mixin.info}.decls.toList.filter(accessorImplementedInSubclass)) // mixin field accessors -- // invariant: (accessorsMaybeNeedingImpl, mixedInAccessorAndFields).zipped.forall(case (acc, clone :: _) => `clone` is clone of `acc` case _ => true) - val mixedInAccessorAndFields = synthInSubclass.map{ member => + val mixedInAccessorAndFields: List[List[Symbol]] = synthInSubclass.map{ member => def cloneAccessor() = { val clonedAccessor = (member cloneSymbol clazz) setPos clazz.pos setMixedinAccessorFlags(member, clonedAccessor) @@ -653,9 +653,15 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor // trait val/var setter mixed into class else fieldAccess(setter) match { case NoSymbol => EmptyTree - case fieldSel => afterOwnPhase { // the assign only type checks after our phase (assignment to val) - mkAccessor(setter)(Assign(Select(This(clazz), fieldSel), castHack(Ident(setter.firstParam), fieldSel.info))) - } + case fieldSel => + if (!fieldSel.hasFlag(MUTABLE)) { + fieldSel.setFlag(MUTABLE) + fieldSel.owner.primaryConstructor.updateAttachment(ConstructorNeedsFence) + } + + afterOwnPhase { // the assign only type checks after our phase (assignment to val) + mkAccessor(setter)(Assign(Select(This(clazz), fieldSel), castHack(Ident(setter.firstParam), fieldSel.info))) + } } def moduleAccessorBody(module: Symbol): Tree = diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index e704632b4991..bd560c7ef1b0 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -107,4 +107,6 @@ trait StdAttachments { case object KnownDirectSubclassesCalled extends PlainAttachment class QualTypeSymAttachment(val sym: Symbol) + + case object ConstructorNeedsFence extends PlainAttachment } diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala index 882d7160d364..c611d7707900 100644 --- a/src/reflect/scala/reflect/internal/StdNames.scala +++ b/src/reflect/scala/reflect/internal/StdNames.scala @@ -756,6 +756,7 @@ trait StdNames { val productPrefix: NameType = "productPrefix" val raw_ : NameType = "raw" val readResolve: NameType = "readResolve" + val releaseFence: NameType = "releaseFence" val reify : NameType = "reify" val reificationSupport : NameType = "reificationSupport" val rootMirror : NameType = "rootMirror"