Skip to content

Implement @static annotation on singleton object fields. #894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 20, 2012
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 63 additions & 20 deletions src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
Original file line number Diff line number Diff line change
Expand Up @@ -113,26 +113,42 @@ abstract class GenICode extends SubComponent {
m.native = m.symbol.hasAnnotation(definitions.NativeAttr)

if (!m.isAbstractMethod && !m.native) {
ctx1 = genLoad(rhs, ctx1, m.returnType);

// reverse the order of the local variables, to match the source-order
m.locals = m.locals.reverse

rhs match {
case Block(_, Return(_)) => ()
case Return(_) => ()
case EmptyTree =>
globalError("Concrete method has no definition: " + tree + (
if (settings.debug.value) "(found: " + m.symbol.owner.info.decls.toList.mkString(", ") + ")"
else "")
)
case _ => if (ctx1.bb.isEmpty)
ctx1.bb.closeWith(RETURN(m.returnType), rhs.pos)
else
if (m.symbol.isAccessor && m.symbol.accessed.hasStaticAnnotation) {
// in companion object accessors to @static fields, we access the static field directly
val hostClass = m.symbol.owner.companionClass
val staticfield = hostClass.info.decls.find(_.name.toString.trim == m.symbol.accessed.name.toString.trim)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you store the static symbol somewhere? I'm worried linear search might be too slow, plus it's comparing strings, which isn't fast either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could store it somewhere during cleanup, but I'm not sure where so that it's visible later during icode. Will look into it.

Also, maybe findMember could help.


if (m.symbol.isGetter) {
ctx1.bb.emit(LOAD_FIELD(staticfield.get, true) setHostClass hostClass, tree.pos)
ctx1.bb.closeWith(RETURN(m.returnType))
} else if (m.symbol.isSetter) {
ctx1.bb.emit(LOAD_LOCAL(m.locals.head), tree.pos)
ctx1.bb.emit(STORE_FIELD(staticfield.get, true), tree.pos)
ctx1.bb.closeWith(RETURN(m.returnType))
} else assert(false, "unreachable")
} else {
ctx1 = genLoad(rhs, ctx1, m.returnType);

// reverse the order of the local variables, to match the source-order
m.locals = m.locals.reverse

rhs match {
case Block(_, Return(_)) => ()
case Return(_) => ()
case EmptyTree =>
globalError("Concrete method has no definition: " + tree + (
if (settings.debug.value) "(found: " + m.symbol.owner.info.decls.toList.mkString(", ") + ")"
else "")
)
case _ =>
if (ctx1.bb.isEmpty)
ctx1.bb.closeWith(RETURN(m.returnType), rhs.pos)
else
ctx1.bb.closeWith(RETURN(m.returnType))
}
if (!ctx1.bb.closed) ctx1.bb.close
prune(ctx1.method)
}
if (!ctx1.bb.closed) ctx1.bb.close
prune(ctx1.method)
} else
ctx1.method.setCode(NoCode)
ctx1
Expand Down Expand Up @@ -854,9 +870,32 @@ abstract class GenICode extends SubComponent {
generatedType = toTypeKind(fun.symbol.tpe.resultType)
ctx1

case app @ Apply(fun @ Select(qual, _), args)
if !ctx.method.symbol.isStaticConstructor
&& fun.symbol.isAccessor && fun.symbol.accessed.hasStaticAnnotation =>
// bypass the accessor to the companion object and load the static field directly
// the only place were this bypass is not done, is the static intializer for the static field itself
val sym = fun.symbol
generatedType = toTypeKind(sym.accessed.info)
val hostClass = qual.tpe.typeSymbol.orElse(sym.owner).companionClass
val staticfield = hostClass.info.decls.find(_.name.toString.trim == sym.accessed.name.toString.trim)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lookup could be abstracted in a helper method, it's duplicated right now. And the efficiency concern applies here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will address this.


if (sym.isGetter) {
ctx.bb.emit(LOAD_FIELD(staticfield.get, true) setHostClass hostClass, tree.pos)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any visibility restrictions on the @static annotation? Can you always by-pass the getter and go straight to the field (it's never private?).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. In the current implementation I did not check this. I would add a check to allow @static only on public fields for now.

ctx
} else if (sym.isSetter) {
val ctx1 = genLoadArguments(args, sym.info.paramTypes, ctx)
ctx1.bb.emit(STORE_FIELD(staticfield.get, true), tree.pos)
ctx1.bb.emit(CONSTANT(Constant(false)), tree.pos)
ctx1
} else {
assert(false, "supposedly unreachable")
ctx
}

case app @ Apply(fun, args) =>
val sym = fun.symbol

if (sym.isLabel) { // jump to a label
val label = ctx.labels.getOrElse(sym, {
// it is a forward jump, scan for labels
Expand Down Expand Up @@ -1623,8 +1662,12 @@ abstract class GenICode extends SubComponent {
* backend emits them as static).
* No code is needed for this module symbol.
*/
for (f <- cls.info.decls ; if !f.isMethod && f.isTerm && !f.isModule)
for (
f <- cls.info.decls;
if !f.isMethod && f.isTerm && !f.isModule && !(f.owner.isModuleClass && f.hasStaticAnnotation)
) {
ctx.clazz addField new IField(f)
}
}

/**
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,9 @@ abstract class GenASM extends SubComponent with BytecodeWriters {
log("No forwarder for " + m + " due to conflict with " + linkedClass.info.member(m.name))
else {
log("Adding static forwarder for '%s' from %s to '%s'".format(m, jclassName, moduleClass))
addForwarder(isRemoteClass, jclass, moduleClass, m)
if (m.isAccessor && m.accessed.hasStaticAnnotation) {
log("@static: accessor " + m + ", accessed: " + m.accessed)
} else addForwarder(isRemoteClass, jclass, moduleClass, m)
}
}
}
Expand Down Expand Up @@ -1675,6 +1677,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters {
jmethod = clinitMethod
jMethodName = CLASS_CONSTRUCTOR_NAME
jmethod.visitCode()
computeLocalVarsIndex(m)
genCode(m, false, true)
jmethod.visitMaxs(0, 0) // just to follow protocol, dummy arguments
jmethod.visitEnd()
Expand Down
12 changes: 8 additions & 4 deletions src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,8 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with

method = m
jmethod = clinitMethod

computeLocalVarsIndex(m)
genCode(m)
case None =>
legacyStaticInitializer(cls, clinit)
Expand Down Expand Up @@ -1114,15 +1116,17 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with
linkedClass.info.members collect { case sym if sym.name.isTermName => sym.name } toSet
}
debuglog("Potentially conflicting names for forwarders: " + conflictingNames)

for (m <- moduleClass.info.membersBasedOnFlags(ExcludedForwarderFlags, Flags.METHOD)) {
if (m.isType || m.isDeferred || (m.owner eq ObjectClass) || m.isConstructor)
debuglog("No forwarder for '%s' from %s to '%s'".format(m, className, moduleClass))
else if (conflictingNames(m.name))
log("No forwarder for " + m + " due to conflict with " + linkedClass.info.member(m.name))
else {
log("Adding static forwarder for '%s' from %s to '%s'".format(m, className, moduleClass))
addForwarder(jclass, moduleClass, m)
if (m.isAccessor && m.accessed.hasStaticAnnotation) {
log("@static: accessor " + m + ", accessed: " + m.accessed)
} else addForwarder(jclass, moduleClass, m)
}
}
}
Expand Down Expand Up @@ -1304,7 +1308,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with
jclass.getType())
}
}

style match {
case Static(true) => dbg("invokespecial"); jcode.emitINVOKESPECIAL(jowner, jname, jtype)
case Static(false) => dbg("invokestatic"); jcode.emitINVOKESTATIC(jowner, jname, jtype)
Expand Down Expand Up @@ -1885,7 +1889,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with
*/
def computeLocalVarsIndex(m: IMethod) {
var idx = if (m.symbol.isStaticMember) 0 else 1;

for (l <- m.params) {
debuglog("Index value for " + l + "{" + l.## + "}: " + idx)
l.index = idx
Expand Down
138 changes: 128 additions & 10 deletions src/compiler/scala/tools/nsc/transform/CleanUp.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
new CleanUpTransformer(unit)

class CleanUpTransformer(unit: CompilationUnit) extends Transformer {
private val newStaticMembers = mutable.Buffer.empty[Tree]
private val newStaticInits = mutable.Buffer.empty[Tree]
private val symbolsStoredAsStatic = mutable.Map.empty[String, Symbol]
private val newStaticMembers = mutable.Buffer.empty[Tree]
private val newStaticInits = mutable.Buffer.empty[Tree]
private val symbolsStoredAsStatic = mutable.Map.empty[String, Symbol]
private val staticBodies = mutable.Map.empty[(Symbol, Symbol), Tree]
private val syntheticClasses = mutable.Map.empty[Symbol, mutable.Set[Tree]] // package and trees
private val classNames = mutable.Map.empty[Symbol, Set[Name]]
private def clearStatics() {
newStaticMembers.clear()
newStaticInits.clear()
Expand All @@ -45,15 +48,16 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
result
}
private def transformTemplate(tree: Tree) = {
val Template(parents, self, body) = tree
val t @ Template(parents, self, body) = tree
clearStatics()

val newBody = transformTrees(body)
val templ = deriveTemplate(tree)(_ => transformTrees(newStaticMembers.toList) ::: newBody)
try addStaticInits(templ) // postprocess to include static ctors
finally clearStatics()
}
private def mkTerm(prefix: String): TermName = unit.freshTermName(prefix)

/** Kludge to provide a safe fix for #4560:
* If we generate a reference in an implementation class, we
* watch out for embedded This(..) nodes that point to the interface.
Expand Down Expand Up @@ -555,7 +559,62 @@ abstract class CleanUp extends Transform with ast.TreeDSL {

else tree
}


case ValDef(mods, name, tpt, rhs) if tree.symbol.hasStaticAnnotation =>
log("moving @static valdef field: " + name + ", in: " + tree.symbol.owner)
val sym = tree.symbol
val owner = sym.owner

val staticBeforeLifting = atPhase(currentRun.erasurePhase) { owner.isStatic }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

atPhase --> enteringPhase or whatever it will be renamed to :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! But this renaming still didn't take place as far as I can see (at least I cannot find it in the 2.10.x branch).

if (!owner.isModuleClass || !staticBeforeLifting) {
if (!sym.isSynthetic) {
reporter.error(tree.pos, "Only members of top-level objects and their nested objects can be annotated with @static.")
tree.symbol.removeAnnotation(StaticClass)
}
super.transform(tree)
} else if (owner.isModuleClass) {
val linkedClass = owner.companionClass match {
case NoSymbol =>
// create the companion class if it does not exist
val enclosing = owner.owner
val compclass = enclosing.newClass(newTypeName(owner.name.toString))
compclass setInfo ClassInfoType(List(ObjectClass.tpe), newScope, compclass)
enclosing.info.decls enter compclass

val compclstree = ClassDef(compclass, NoMods, List(List()), List(List()), List(), tree.pos)

syntheticClasses.getOrElseUpdate(enclosing, mutable.Set()) += compclstree

compclass
case comp => comp
}

// create a static field in the companion class for this @static field
val stfieldSym = linkedClass.newVariable(newTermName(name), tree.pos, STATIC | SYNTHETIC | FINAL) setInfo sym.tpe
stfieldSym.addAnnotation(StaticClass)

val names = classNames.getOrElseUpdate(linkedClass, linkedClass.info.decls.collect {
case sym if sym.name.isTermName => sym.name
} toSet)
if (names(stfieldSym.name)) {
reporter.error(
tree.pos,
"@static annotated field " + tree.symbol.name + " has the same name as a member of class " + linkedClass.name
)
} else {
linkedClass.info.decls enter stfieldSym

val initializerBody = rhs

// static field was previously initialized in the companion object itself, like this:
// staticBodies((linkedClass, stfieldSym)) = Select(This(owner), sym.getter(owner))
// instead, we move the initializer to the static ctor of the companion class
// we save the entire ValDef/DefDef to extract the rhs later
staticBodies((linkedClass, stfieldSym)) = tree
}
}
super.transform(tree)

/* MSIL requires that the stack is empty at the end of a try-block.
* Hence, we here rewrite all try blocks with a result != {Unit, All} such that they
* store their result in a local variable. The catch blocks are adjusted as well.
Expand Down Expand Up @@ -665,6 +724,11 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
if (newStaticInits.isEmpty)
template
else {
val ctorBody = newStaticInits.toList flatMap {
case Block(stats, expr) => stats :+ expr
case t => List(t)
}

val newCtor = findStaticCtor(template) match {
// in case there already were static ctors - augment existing ones
// currently, however, static ctors aren't being generated anywhere else
Expand All @@ -673,22 +737,76 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
deriveDefDef(ctor) {
case block @ Block(stats, expr) =>
// need to add inits to existing block
treeCopy.Block(block, newStaticInits.toList ::: stats, expr)
treeCopy.Block(block, ctorBody ::: stats, expr)
case term: TermTree =>
// need to create a new block with inits and the old term
treeCopy.Block(term, newStaticInits.toList, term)
treeCopy.Block(term, ctorBody, term)
}
case _ =>
// create new static ctor
val staticCtorSym = currentClass.newStaticConstructor(template.pos)
val rhs = Block(newStaticInits.toList, Literal(Constant(())))
val rhs = Block(ctorBody, Literal(Constant(())))

localTyper.typedPos(template.pos)(DefDef(staticCtorSym, rhs))
}
deriveTemplate(template)(newCtor :: _)
}
}


private def addStaticDeclarations(tree: Template, clazz: Symbol) {
// add static field initializer statements for each static field in clazz
if (!clazz.isModuleClass) for {
staticSym <- clazz.info.decls
if staticSym.hasStaticAnnotation
} staticSym match {
case stfieldSym if stfieldSym.isVariable =>
val valdef = staticBodies((clazz, stfieldSym))
val ValDef(_, _, _, rhs) = valdef
val fixedrhs = rhs.changeOwner((valdef.symbol, clazz.info.decl(nme.CONSTRUCTOR)))

val stfieldDef = localTyper.typedPos(tree.pos)(VAL(stfieldSym) === EmptyTree)
val flattenedInit = fixedrhs match {
case Block(stats, expr) => Block(stats, safeREF(stfieldSym) === expr)
case rhs => safeREF(stfieldSym) === rhs
}
val stfieldInit = localTyper.typedPos(tree.pos)(flattenedInit)

// add field definition to new defs
newStaticMembers append stfieldDef
newStaticInits append stfieldInit
}
}



override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = {
super.transformStats(stats, exprOwner) ++ {
// flush pending synthetic classes created in this owner
val synthclassdefs = syntheticClasses.get(exprOwner).toList.flatten
syntheticClasses -= exprOwner
synthclassdefs map {
cdef => localTyper.typedPos(cdef.pos)(cdef)
}
} map {
case clsdef @ ClassDef(mods, name, tparams, t @ Template(parent, self, body)) =>
// process all classes in the package again to add static initializers
clearStatics()

addStaticDeclarations(t, clsdef.symbol)

val templ = deriveTemplate(t)(_ => transformTrees(newStaticMembers.toList) ::: body)
val ntempl =
try addStaticInits(templ)
finally clearStatics()

val derived = deriveClassDef(clsdef)(_ => ntempl)
classNames.remove(clsdef.symbol)
derived

case stat => stat
}
}

} // CleanUpTransformer

}
7 changes: 5 additions & 2 deletions src/compiler/scala/tools/nsc/transform/Constructors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,15 @@ abstract class Constructors extends Transform with ast.TreeDSL {
// before the superclass constructor call, otherwise it goes after.
// Lazy vals don't get the assignment in the constructor.
if (!stat.symbol.tpe.isInstanceOf[ConstantType]) {
if (rhs != EmptyTree && !stat.symbol.isLazy) {
if (stat.symbol.hasStaticAnnotation) {
debuglog("@static annotated field initialization skipped.")
defBuf += deriveValDef(stat)(tree => tree)
} else if (rhs != EmptyTree && !stat.symbol.isLazy) {
val rhs1 = intoConstructor(stat.symbol, rhs);
(if (canBeMoved(stat)) constrPrefixBuf else constrStatBuf) += mkAssign(
stat.symbol, rhs1)
defBuf += deriveValDef(stat)(_ => EmptyTree)
}
defBuf += deriveValDef(stat)(_ => EmptyTree)
}
case ClassDef(_, _, _, _) =>
// classes are treated recursively, and left in the template
Expand Down
Loading