Skip to content

Commit

Permalink
Mixin fields with trait setters shouldn't be JVM final
Browse files Browse the repository at this point in the history
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
}
```
  • Loading branch information
retronym committed May 4, 2018
1 parent 7505d9f commit 956121d
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 6 deletions.
7 changes: 6 additions & 1 deletion src/compiler/scala/tools/nsc/transform/Constructors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down
16 changes: 11 additions & 5 deletions src/compiler/scala/tools/nsc/transform/Fields.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 =
Expand Down
2 changes: 2 additions & 0 deletions src/reflect/scala/reflect/internal/StdAttachments.scala
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,6 @@ trait StdAttachments {
case object KnownDirectSubclassesCalled extends PlainAttachment

class QualTypeSymAttachment(val sym: Symbol)

case object ConstructorNeedsFence extends PlainAttachment
}
1 change: 1 addition & 0 deletions src/reflect/scala/reflect/internal/StdNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 956121d

Please sign in to comment.