Skip to content

Commit

Permalink
Cleanups after integrating lazyvals into fields.
Browse files Browse the repository at this point in the history
Mostly refactorings and catching up with doc updates.

Some changes to flag handling, removing some redundancy,
and making lazy fields and modules a bit more consistent
in their flags. They now uniformly carry LAZY or MODULEVAR.
Before, LAZY was dropped because mixin had some lazy val
logic. No longer.
  • Loading branch information
adriaanm committed Aug 31, 2016
1 parent 92d1af1 commit c0763b0
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 69 deletions.
131 changes: 71 additions & 60 deletions src/compiler/scala/tools/nsc/transform/Fields.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,38 @@ import scala.annotation.tailrec
import symtab.Flags._


/** Synthesize accessors and field for each (strict) val owned by a trait.
/** Synthesize accessors, fields (and bitmaps) for (lazy) vals and modules.
*
* For traits:
* During Namers, a `ValDef` that is `lazy`, deferred and/or defined in a trait carries its getter's symbol.
* The underlying field symbol does not exist until this phase.
*
* - Namers translates a definition `val x = rhs` into a getter `def x = rhs` -- no underlying field is created.
* - This phase synthesizes accessors and fields for any vals mixed into a non-trait class.
* - Constructors will move the rhs to an assignment in the template body.
* Those statements then move to the template into the constructor,
* which means it will initialize the fields defined in this template (and execute the corresponding side effects).
* We need to maintain the connection between getter and rhs until after specialization so that it can duplicate vals.
* - A ModuleDef is desugared to a ClassDef, an accessor (which reuses the module's term symbol)
* and a module var (unless the module is static and does not implement a member of a supertype, or we're in a trait).
* For subclasses of traits that define modules, a module var is mixed in, as well as the required module accessors.
* For `val`s defined in classes, we still emit a field immediately.
* TODO: uniformly assign getter symbol to all `ValDef`s, stop using `accessed`.
*
* Runs after uncurry to deal with classes that implement SAM traits with ValDefs.
* Runs before erasure (to get bridges), and thus before lambdalift/flatten, so that nested functions/definitions must be considered.
* This phase synthesizes accessors, fields and bitmaps (for lazy or init-checked vals under -Xcheckinit)
* in the first (closest in the subclassing lattice) subclass (not a trait) of a trait.
*
* We run after uncurry because it can introduce subclasses of traits with fields (SAMs with vals).
* Lambdalift also introduces new fields (paramaccessors for captured vals), but runs too late in the pipeline
* (mixins still synthesizes implementations for accessors that need to be mixed into subclasses of local traits that capture).
* For lazy vals and modules, we emit accessors that using double-checked locking (DCL) to balance thread safety
* and performance. A lazy val gets a compute method for the DCL's slow path, for a module it's all done in the accessor.
*
* Local lazy vals do not receive bitmaps, but use a Lazy*Holder that has the volatile init bit and the computed value.
* See `mkLazyLocalDef`.
*
* Constructors will move the rhs to an assignment in the template body.
* Those statements then move to the template into the constructor,
* which means it will initialize the fields defined in this template (and execute the corresponding side effects).
* We need to maintain the connection between getter and rhs until after specialization so that it can duplicate vals.
*
* A ModuleDef is desugared to a ClassDef, an accessor (which reuses the module's term symbol)
* and a module var (unless the module is static and does not implement a member of a supertype, or we're in a trait).
*
* For subclasses of traits that define modules, a module var is mixed in, as well as the required module accessors.
*
* Phase ordering:
* - Runs after uncurry to deal with classes that implement SAM traits with ValDefs.
* - Runs before erasure (to get bridges), and thus before lambdalift/flatten, so that nested functions/definitions must be considered.
* - Lambdalift introduces new paramaccessors for captured vals, but runs too late in the pipeline, so
* mixins still synthesizes implementations for these accessors when a local trait that captures is subclassed.
*
*
* In the future, would like to get closer to dotty, which lifts a val's RHS (a similar thing is done for template-level statements)
Expand All @@ -54,7 +66,10 @@ import symtab.Flags._
* The only change due to overriding is that its value is never written to the field
* (the overridden val's value is, of course, stored in the field in addition to its side-effect being performed).
*
* TODO: check init support (or drop the -Xcheck-init flag??)
* TODO: Java 9 support for vals defined in traits. They are currently emitted as final,
* but the write (putfield) to the val does not occur syntactically within the <init> method
* (it's done by the trait setter, which is called from the trait's mixin constructor,
* which is called from the subclass's constructor...)
*/
abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransformers with AccessorSynthesis {
import global._
Expand All @@ -68,8 +83,8 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
if (sym.isJavaDefined || sym.isPackageClass || !sym.isClass) tp
else synthFieldsAndAccessors(tp)

// we leave lazy vars/accessors and early-init vals alone for now
private def excludedAccessorOrFieldByFlags(statSym: Symbol): Boolean = statSym hasFlag LAZY | PRESUPER
// TODO: drop PRESUPER support when we implement trait parameters in 2.13
private def excludedAccessorOrFieldByFlags(statSym: Symbol): Boolean = statSym hasFlag PRESUPER

// used for internal communication between info and tree transform of this phase -- not pickled, not in initialflags
// TODO: reuse MIXEDIN for NEEDS_TREES?
Expand Down Expand Up @@ -172,19 +187,29 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor


// can't use the referenced field since it already tracks the module's moduleClass
private[this] val moduleVarOf = perRunCaches.newMap[Symbol, Symbol]
private[this] val moduleOrLazyVarOf = perRunCaches.newMap[Symbol, Symbol]

// TODO: can we drop FINAL? In any case, since these variables are MUTABLE, they cannot and will
// not be emitted as ACC_FINAL. They are FINAL in the Scala sense, though: cannot be overridden.
private final val ModuleOrLazyFieldFlags = FINAL | PrivateLocal | SYNTHETIC | NEEDS_TREES

private def newModuleVarSymbol(owner: Symbol, module: Symbol, tp: Type, extraFlags: Long): TermSymbol = {
private def newModuleVarSymbol(owner: Symbol, module: Symbol, tp: Type): TermSymbol = {
// println(s"new module var in $site for $module of type $tp")
val moduleVar = owner.newVariable(nme.moduleVarName(module.name.toTermName), module.pos.focus, MODULEVAR | extraFlags) setInfo tp addAnnotation VolatileAttr
moduleVarOf(module) = moduleVar
val flags = MODULEVAR | (if (owner.isClass) ModuleOrLazyFieldFlags else 0)

val moduleVar =
(owner.newVariable(nme.moduleVarName(module.name.toTermName), module.pos.focus, flags)
setInfo tp
addAnnotation VolatileAttr)

moduleOrLazyVarOf(module) = moduleVar

moduleVar
}

private def moduleInit(module: Symbol) = {
// println(s"moduleInit for $module in ${module.ownerChain} --> ${moduleVarOf.get(module)}")
val moduleVar = moduleVarOf(module)
val moduleVar = moduleOrLazyVarOf(module)
def moduleVarRef = gen.mkAttributedRef(moduleVar)

// for local modules, we synchronize on the owner of the method that owns the module
Expand Down Expand Up @@ -215,28 +240,19 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
}

// NoSymbol for lazy accessor sym with unit result type
def lazyVarOf(sym: Symbol) = moduleVarOf.getOrElse(sym, NoSymbol)
def lazyVarOf(sym: Symbol) = moduleOrLazyVarOf.getOrElse(sym, NoSymbol)

private def newLazyVarSymbol(owner: Symbol, member: Symbol, tp: Type, extraFlags: Long = 0): TermSymbol = {
val flags = member.flags | extraFlags
val pos = member.pos
private def newLazyVarMember(clazz: Symbol, member: Symbol, tp: Type): TermSymbol = {
val flags = LAZY | (member.flags & FieldFlags) | ModuleOrLazyFieldFlags
val name = member.name.toTermName.append(reflect.NameTransformer.LOCAL_SUFFIX_STRING)

// the underlying field for a lazy val should not be final because we write to it outside of a constructor,
// so, set the MUTABLE flag
val fieldFlags = flags & FieldFlags | PrivateLocal | MUTABLE
// Set the MUTABLE flag because the field cannot be ACC_FINAL since we write to it outside of a constructor.
val sym = clazz.newVariable(name, member.pos.focus, flags) setInfo tp

val sym = owner.newValue(name, pos.focus, fieldFlags | extraFlags) setInfo tp
moduleVarOf(member) = sym
moduleOrLazyVarOf(member) = sym
sym
}

private def lazyValInit(member: Symbol, rhs: Tree) = {
val lazyVar = moduleVarOf(member)
assert(lazyVar.isMutable, lazyVar)
gen.mkAssignAndReturn(lazyVar, rhs)
}

private object synthFieldsAndAccessors extends TypeMap {
private def newTraitSetter(getter: Symbol, clazz: Symbol) = {
// Add setter for an immutable, memoizing getter
Expand All @@ -256,7 +272,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
private def newModuleAccessor(module: Symbol, site: Symbol, moduleVar: Symbol) = {
val accessor = site.newMethod(module.name.toTermName, site.pos, STABLE | MODULE | NEEDS_TREES)

moduleVarOf(accessor) = moduleVar
moduleOrLazyVarOf(accessor) = moduleVar

// we're in the same prefix as module, so no need for site.thisType.memberType(module)
accessor setInfo MethodType(Nil, moduleVar.info)
Expand Down Expand Up @@ -287,9 +303,6 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
lazyCallingSuper setInfo tp
}

// make sure they end up final in bytecode
final private val fieldFlags = PrivateLocal | FINAL | SYNTHETIC | NEEDS_TREES

def apply(tp0: Type): Type = tp0 match {
// TODO: make less destructive (name changes, decl additions, flag setting --
// none of this is actually undone when travelling back in time using atPhase)
Expand Down Expand Up @@ -359,10 +372,10 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
}

def newModuleVarMember(member: Symbol): TermSymbol =
newModuleVarSymbol(clazz, member, site.memberType(member).resultType, fieldFlags)
newModuleVarSymbol(clazz, member, site.memberType(member).resultType)

def newLazyVarMember(member: Symbol): TermSymbol =
newLazyVarSymbol(clazz, member, site.memberType(member).resultType, fieldFlags)
Fields.this.newLazyVarMember(clazz, member, site.memberType(member).resultType)

// a module does not need treatment here if it's static, unless it has a matching member in a superclass
// a non-static method needs a module var
Expand Down Expand Up @@ -491,9 +504,8 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor

// done by uncurry's info transformer
// instead of forcing every member's info to run said transformer, duplicate the flag update logic...
def nonStaticModuleToMethod(module: Symbol): Unit = {
def nonStaticModuleToMethod(module: Symbol): Unit =
if (!module.isStatic) module setFlag METHOD | STABLE
}

class FieldsTransformer(unit: CompilationUnit) extends TypingTransformer(unit) with CheckedAccessorTreeSynthesis {
protected def typedPos(pos: Position)(tree: Tree): Tree = localTyper.typedPos(pos)(tree)
Expand Down Expand Up @@ -579,11 +591,10 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
val accessor = mkAccessor(lazyVal)(mkChecked(getValue, Apply(Ident(computerSym), Nil)))

// do last!
// remove LAZY: prevent lazy expansion in mixin
// remove STABLE: prevent replacing accessor call of type Unit by BoxedUnit.UNIT in erasure
// remove ACCESSOR: prevent constructors from eliminating the method body if the lazy val is
// lifted into a trait (TODO: not sure about the details here)
lazyVal.resetFlag(LAZY | STABLE | ACCESSOR)
lazyVal.resetFlag(STABLE | ACCESSOR)

Thicket(mkTypedValDef(holderSym, New(refTpe)) :: computer :: accessor :: Nil)
}
Expand Down Expand Up @@ -651,7 +662,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
atOwner(newOwner)(super.transform(stat.rhs.changeOwner(stat.symbol -> newOwner)))

override def transform(stat: Tree): Tree = {
val clazz = currentOwner
val currOwner = currentOwner // often a class, but not necessarily
val statSym = stat.symbol

/*
Expand All @@ -674,36 +685,36 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor
// also remove ACCESSOR flag since there won't be an underlying field to access?
case DefDef(_, _, _, _, _, rhs) if (statSym hasFlag ACCESSOR)
&& (rhs ne EmptyTree) && !excludedAccessorOrFieldByFlags(statSym)
&& !clazz.isTrait // we've already done this for traits.. the asymmetry will be solved by the above todo
&& fieldMemoizationIn(statSym, clazz).constantTyped =>
&& !currOwner.isTrait // we've already done this for traits.. the asymmetry will be solved by the above todo
&& fieldMemoizationIn(statSym, currOwner).constantTyped =>
deriveDefDef(stat)(_ => gen.mkAttributedQualifier(rhs.tpe))

// deferred val, trait val, lazy val (local or in class)
case vd@ValDef(mods, name, tpt, rhs) if vd.symbol.hasFlag(ACCESSOR) && treeInfo.noFieldFor(vd, clazz) =>
case vd@ValDef(mods, name, tpt, rhs) if vd.symbol.hasFlag(ACCESSOR) && treeInfo.noFieldFor(vd, currOwner) =>
val transformedRhs = atOwner(statSym)(transform(rhs))

if (rhs == EmptyTree) mkAccessor(statSym)(EmptyTree)
else if (clazz.isTrait) mkAccessor(statSym)(transformedRhs)
else if (!clazz.isClass) mkLazyLocalDef(vd.symbol, transformedRhs)
else if (currOwner.isTrait) mkAccessor(statSym)(transformedRhs)
else if (!currOwner.isClass) mkLazyLocalDef(vd.symbol, transformedRhs)
else {
// TODO: make `synthAccessorInClass` a field and update it in atOwner?
// note that `LazyAccessorTreeSynth` is pretty lightweight
// (it's just a bunch of methods that all take a `clazz` parameter, which is thus stored as a field)
val synthAccessorInClass = new SynthLazyAccessorsIn(clazz)
synthAccessorInClass.expandLazyClassMember(lazyVarOf(statSym), statSym, transformedRhs, nullables.getOrElse(clazz, Map.empty))
val synthAccessorInClass = new SynthLazyAccessorsIn(currOwner)
synthAccessorInClass.expandLazyClassMember(lazyVarOf(statSym), statSym, transformedRhs, nullables.getOrElse(currOwner, Map.empty))
}

// drop the val for (a) constant (pure & not-stored) and (b) not-stored (but still effectful) fields
case ValDef(mods, _, _, rhs) if (rhs ne EmptyTree) && !excludedAccessorOrFieldByFlags(statSym)
&& fieldMemoizationIn(statSym, clazz).constantTyped =>
&& fieldMemoizationIn(statSym, currOwner).constantTyped =>
EmptyThicket

case ModuleDef(_, _, impl) =>
// ??? The typer doesn't take kindly to seeing this ClassDef; we have to set NoType so it will be ignored.
val cd = super.transform(ClassDef(statSym.moduleClass, impl) setType NoType)
if (clazz.isClass) cd
if (currOwner.isClass) cd
else { // local module -- symbols cannot be generated by info transformer, so do it all here
val moduleVar = newModuleVarSymbol(currentOwner, statSym, statSym.info.resultType, 0)
val moduleVar = newModuleVarSymbol(currOwner, statSym, statSym.info.resultType)
Thicket(cd :: mkTypedValDef(moduleVar) :: mkAccessor(statSym)(moduleInit(statSym)) :: Nil)
}

Expand Down
12 changes: 5 additions & 7 deletions src/compiler/scala/tools/nsc/transform/Mixin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes
* (private modules, on the other hand, are implemented statically, but their
* module variable is not. all such private modules are lifted, because
* non-lifted private modules have been eliminated in ExplicitOuter)
* - field accessors and superaccessors, except for lazy value accessors which become initializer
* methods in the impl class (because they can have arbitrary initializers)
* - field accessors and superaccessors
*/
private def isImplementedStatically(sym: Symbol) = (
(sym.isMethod || ((sym hasFlag MODULE) && !sym.isStatic))
// TODO: ^^^ non-static modules should have been turned into methods by fields by now, no? maybe the info transformer hasn't run???
&& notDeferred(sym)
&& sym.owner.isTrait
&& (!sym.isModule || sym.hasFlag(PRIVATE | LIFTED))
&& (!(sym hasFlag (ACCESSOR | SUPERACCESSOR)) || sym.isLazy)
&& (!(sym hasFlag (ACCESSOR | SUPERACCESSOR)) || (sym hasFlag LAZY))
&& !sym.isPrivate
&& !sym.hasAllFlags(LIFTED | MODULE | METHOD)
&& !sym.isConstructor
Expand Down Expand Up @@ -181,9 +181,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes
else {
assert(member.isTerm && !member.isDeferred, member)
// disable assert to support compiling against code compiled by an older compiler (until we re-starr)
// assert(member hasFlag LAZY | PRESUPER, s"unexpected $member in $clazz ${member.debugFlagString}")
// lazy vals still leave field symbols lying around in traits -- TODO: never emit them to begin with
// ditto for early init vals
// assert(member hasFlag PRESUPER, s"unexpected $member in $clazz ${member.debugFlagString}")
clazz.info.decls.unlink(member)
}

Expand Down Expand Up @@ -407,7 +405,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes
if (clazz.isTrait || sym.isSuperAccessor) addDefDef(sym)
// implement methods mixed in from a supertrait (the symbols were created by mixinTraitMembers)
else if (sym.hasFlag(ACCESSOR) && !sym.hasFlag(DEFERRED)) {
assert(sym hasFlag (PARAMACCESSOR), s"mixed in $sym from $clazz is not lazy/param?!?")
assert(sym hasFlag (PARAMACCESSOR), s"mixed in $sym from $clazz is not param?!?")

// add accessor definitions
addDefDef(sym, accessorBody(sym))
Expand Down
4 changes: 2 additions & 2 deletions test/files/neg/t6666.check
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
t6666.scala:23: error: Implementation restriction: access of method x$2 in object O1 from <$anon: Function0>, would require illegal premature access to object O1
F.byname(x)
^
t6666.scala:30: error: Implementation restriction: access of method x$3 in object O2 from <$anon: Function0>, would require illegal premature access to object O2
t6666.scala:30: error: Implementation restriction: access of lazy value x$3 in object O2 from <$anon: Function0>, would require illegal premature access to object O2
F.byname(x)
^
t6666.scala:37: error: Implementation restriction: access of method x$4 in object O3 from <$anon: Function0>, would require illegal premature access to object O3
Expand All @@ -10,7 +10,7 @@ t6666.scala:37: error: Implementation restriction: access of method x$4 in objec
t6666.scala:50: error: Implementation restriction: access of method x$6 in class C1 from <$anon: Function0>, would require illegal premature access to the unconstructed `this` of class C1
F.byname(x)
^
t6666.scala:54: error: Implementation restriction: access of method x$7 in class C2 from <$anon: Function0>, would require illegal premature access to the unconstructed `this` of class C2
t6666.scala:54: error: Implementation restriction: access of lazy value x$7 in class C2 from <$anon: Function0>, would require illegal premature access to the unconstructed `this` of class C2
F.byname(x)
^
t6666.scala:58: error: Implementation restriction: access of method x$8 in class C3 from <$anon: Function0>, would require illegal premature access to the unconstructed `this` of class C3
Expand Down

0 comments on commit c0763b0

Please sign in to comment.