Skip to content

Drop empty companion objects #1075

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 5 commits into from
Feb 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 3 additions & 3 deletions src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ class Compiler {
new CapturedVars, // capturedVars has a transformUnit: no phases should introduce local mutable vars here
new Constructors, // constructors changes decls in transformTemplate, no InfoTransformers should be added after it
new FunctionalInterfaces,
new GetClass), // getClass transformation should be applied to specialized methods
new GetClass), // getClass transformation should be applied to specialized methods
List(new LambdaLift, // in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here
new ElimStaticThis,
new Flatten,
new DropEmptyCompanions,
new RestoreScopes),
List(/*new PrivateToStatic,*/
new ExpandPrivate,
List(new ExpandPrivate,
new CollectEntryPoints,
new LabelDefs),
List(new GenBCode)
Expand Down
1 change: 1 addition & 0 deletions src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ object Flags {
final val AbstractFinal = allOf(Abstract, Final)
final val AbstractSealed = allOf(Abstract, Sealed)
final val SyntheticArtifact = allOf(Synthetic, Artifact)
final val SyntheticModule = allOf(Synthetic, Module)
final val SyntheticTermParam = allOf(Synthetic, TermParam)
final val SyntheticTypeParam = allOf(Synthetic, TypeParam)
final val SyntheticCase = allOf(Synthetic, Case)
Expand Down
7 changes: 7 additions & 0 deletions src/dotty/tools/dotc/core/NameOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ object NameOps {
def isModuleVarName(name: Name): Boolean =
name.stripAnonNumberSuffix endsWith MODULE_VAR_SUFFIX
def isSelectorName = name.startsWith(" ") && name.tail.forall(_.isDigit)
def isLazyLocal = name.endsWith(nme.LAZY_LOCAL)

/** Is name a variable name? */
def isVariableName: Boolean = name.length > 0 && {
Expand Down Expand Up @@ -423,5 +424,11 @@ object NameOps {
case NO_NAME => primitivePostfixMethodName
case name => name
}

def lazyLocalName = name ++ nme.LAZY_LOCAL
def nonLazyName = {
assert(name.isLazyLocal)
name.dropRight(nme.LAZY_LOCAL.length)
}
}
}
98 changes: 98 additions & 0 deletions src/dotty/tools/dotc/transform/DropEmptyCompanions.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package dotty.tools.dotc
package transform

import core._
import DenotTransformers.SymTransformer
import Phases.Phase
import Contexts.Context
import Flags._
import Symbols._
import SymDenotations.SymDenotation
import ast.Trees._
import collection.mutable
import Decorators._
import NameOps._
import TreeTransforms.MiniPhaseTransform
import dotty.tools.dotc.transform.TreeTransforms.TransformerInfo

/** Remove companion objects that are empty
* Lots of constraints here:
* 1. It's impractical to place DropEmptyCompanions before lambda lift because dropped
* modules can be anywhere and have hard to trace references.
* 2. DropEmptyCompanions cannot be interleaved with LambdaLift or Flatten because
* they put things in liftedDefs sets which cause them to surface later. So
* removed modules resurface.
* 3. DropEmptyCompanions has to be before RestoreScopes.
* The solution to the constraints is to put DropEmptyCompanions between Flatten
* and RestoreScopes and to only start working once we are back on PackageDef
* level, so we know that all objects moved by LambdaLift and Flatten have arrived
* at their destination.
*/
class DropEmptyCompanions extends MiniPhaseTransform { thisTransform =>
import ast.tpd._
override def phaseName = "dropEmpty"
override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Flatten])

override def transformPackageDef(pdef: PackageDef)(implicit ctx: Context, info: TransformerInfo) = {

/** Is `tree` an empty companion object? */
def isEmptyCompanion(tree: Tree) = tree match {
case TypeDef(_, impl: Template) if tree.symbol.is(SyntheticModule) &&
tree.symbol.companionClass.exists &&
impl.body.forall(_.symbol.isPrimaryConstructor) =>
println(i"removing ${tree.symbol}")
Copy link
Contributor

Choose a reason for hiding this comment

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

println left in code.
I'll make a pr to remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already removed in another PR of mine. So it should go away by itself
soon.

On Thu, Feb 18, 2016 at 11:23 AM, Dmitry Petrashko <[email protected]

wrote:

In src/dotty/tools/dotc/transform/DropEmptyCompanions.scala
#1075 (comment):

  • * level, so we know that all objects moved by LambdaLift and Flatten have arrived
  • * at their destination.
  • */
    +class DropEmptyCompanions extends MiniPhaseTransform { thisTransform =>
  • import ast.tpd._
  • override def phaseName = "dropEmpty"
  • override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Flatten])
  • override def transformPackageDef(pdef: PackageDef)(implicit ctx: Context, info: TransformerInfo) = {
  • /** Is tree an empty companion object? */
  • def isEmptyCompanion(tree: Tree) = tree match {
  •  case TypeDef(_, impl: Template) if tree.symbol.is(SyntheticModule) &&
    
  •    tree.symbol.companionClass.exists &&
    
  •    impl.body.forall(_.symbol.isPrimaryConstructor) =>
    
  •    println(i"removing ${tree.symbol}")
    

println left in code.
I'll make a pr to remove it.


Reply to this email directly or view it on GitHub
https://github.com/lampepfl/dotty/pull/1075/files#r53295488.

Martin Odersky
EPFL

true
case _ =>
false
}

val dropped = pdef.stats.filter(isEmptyCompanion).map(_.symbol).toSet

/** Symbol is a $lzy field representing a module */
def isLazyModuleVar(sym: Symbol) =
sym.name.isLazyLocal &&
sym.owner.info.decl(sym.name.asTermName.nonLazyName).symbol.is(Module)

/** Symbol should be dropped together with a dropped companion object.
* Such symbols are:
* - lzy fields pointing to modules,
* - vals and getters representing modules.
*/
def symIsDropped(sym: Symbol): Boolean =
(sym.is(Module) || isLazyModuleVar(sym)) &&
dropped.contains(sym.info.resultType.typeSymbol)

/** Tree should be dropped because it (is associated with) an empty
* companion object. Such trees are
* - module classes of empty companion objects
* - definitions of lazy module variables or assignments to them.
* - vals and getters for empty companion objects
*/
def toDrop(stat: Tree): Boolean = stat match {
case stat: TypeDef => dropped.contains(stat.symbol)
case stat: ValOrDefDef => symIsDropped(stat.symbol)
case stat: Assign => symIsDropped(stat.lhs.symbol)
case _ => false
}

def prune(tree: Tree): Tree = tree match {
case tree @ TypeDef(name, impl @ Template(constr, _, _, _)) =>
cpy.TypeDef(tree)(
rhs = cpy.Template(impl)(
constr = cpy.DefDef(constr)(rhs = pruneLocals(constr.rhs)),
body = pruneStats(impl.body)))
case _ =>
tree
}

def pruneStats(stats: List[Tree]) =
stats.filterConserve(!toDrop(_)).mapConserve(prune)

def pruneLocals(expr: Tree) = expr match {
case Block(stats, expr) => cpy.Block(expr)(pruneStats(stats), expr)
case _ => expr
}

cpy.PackageDef(pdef)(pdef.pid, pruneStats(pdef.stats))
}
}
1 change: 1 addition & 0 deletions src/dotty/tools/dotc/transform/Flatten.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import collection.mutable
import TreeTransforms.MiniPhaseTransform
import dotty.tools.dotc.transform.TreeTransforms.TransformerInfo

/** Lift nested classes to toplevel */
class Flatten extends MiniPhaseTransform with SymTransformer { thisTransform =>
import ast.tpd._
override def phaseName = "flatten"
Expand Down
12 changes: 6 additions & 6 deletions src/dotty/tools/dotc/transform/LazyVals.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer {

if (isField) {
if (sym.isVolatile ||
(sym.is(Flags.Module) && !sym.is(Flags.Synthetic)))
// module class is user-defined.
(sym.is(Flags.Module) && !sym.is(Flags.Synthetic)))
// module class is user-defined.
// Should be threadsafe, to mimic safety guaranteed by global object
transformMemberDefVolatile(tree)
else if (sym.is(Flags.Module)) { // synthetic module
Expand Down Expand Up @@ -101,7 +101,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer {
*/
def transformSyntheticModule(tree: ValOrDefDef)(implicit ctx: Context) = {
val sym = tree.symbol
val holderSymbol = ctx.newSymbol(sym.owner, sym.asTerm.name ++ nme.LAZY_LOCAL,
val holderSymbol = ctx.newSymbol(sym.owner, sym.asTerm.name.lazyLocalName,
Flags.Synthetic, sym.info.widen.resultType).enteredAfter(this)
val field = ValDef(holderSymbol, tree.rhs.changeOwnerAfter(sym, holderSymbol, this))
val getter = DefDef(sym.asTerm, ref(holderSymbol))
Expand All @@ -114,7 +114,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer {
*/
def transformLocalDef(x: ValOrDefDef)(implicit ctx: Context) = {
val valueInitter = x.rhs
val holderName = ctx.freshName(x.name ++ StdNames.nme.LAZY_LOCAL).toTermName
val holderName = ctx.freshName(x.name.asTermName.lazyLocalName).toTermName
val initName = ctx.freshName(x.name ++ StdNames.nme.LAZY_LOCAL_INIT).toTermName
val tpe = x.tpe.widen.resultType.widen

Expand Down Expand Up @@ -206,7 +206,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer {
val claz = x.symbol.owner.asClass
val tpe = x.tpe.widen.resultType.widen
assert(!(x.mods is Flags.Mutable))
val containerName = ctx.freshName(x.name ++ StdNames.nme.LAZY_LOCAL).toTermName
val containerName = ctx.freshName(x.name.asTermName.lazyLocalName).toTermName
val containerSymbol = ctx.newSymbol(claz, containerName,
x.symbol.flags &~ containerFlagsMask | containerFlags | Flags.Private,
tpe, coord = x.symbol.coord
Expand Down Expand Up @@ -367,7 +367,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer {
appendOffsetDefs += (companion.moduleClass -> new OffsetInfo(List(offsetTree), ord))
}

val containerName = ctx.freshName(x.name ++ StdNames.nme.LAZY_LOCAL).toTermName
val containerName = ctx.freshName(x.name.asTermName.lazyLocalName).toTermName
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DarkDimius This is just DRY in order not to do the same low-level name plumbing over and over again.

val containerSymbol = ctx.newSymbol(claz, containerName, (x.mods &~ containerFlagsMask | containerFlags).flags, tpe, coord = x.symbol.coord).enteredAfter(this)
val containerTree = ValDef(containerSymbol, defaultValue(tpe))

Expand Down
67 changes: 39 additions & 28 deletions src/dotty/tools/dotc/transform/RestoreScopes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,35 +23,46 @@ class RestoreScopes extends MiniPhaseTransform with IdentityDenotTransformer { t
import ast.tpd._
override def phaseName = "restoreScopes"

override def transformTypeDef(tree: TypeDef)(implicit ctx: Context, info: TransformerInfo) = {
val TypeDef(_, impl: Template) = tree
//
val restoredDecls = newScope
for (stat <- impl.constr :: impl.body)
if (stat.isInstanceOf[MemberDef] && stat.symbol.exists)
restoredDecls.enter(stat.symbol)
/* Note: We need to wait until we see a package definition because
* DropEmptyConstructors changes template members when analyzing the
* enclosing package definitions. So by the time RestoreScopes gets to
* see a typedef or template, it still might be changed by DropEmptyConstructors.
*/
override def transformPackageDef(pdef: PackageDef)(implicit ctx: Context, info: TransformerInfo) = {
pdef.stats.foreach(restoreScope)
pdef
}

private def restoreScope(tree: Tree)(implicit ctx: Context, info: TransformerInfo) = tree match {
case TypeDef(_, impl: Template) =>
val restoredDecls = newScope
for (stat <- impl.constr :: impl.body)
if (stat.isInstanceOf[MemberDef] && stat.symbol.exists)
restoredDecls.enter(stat.symbol)
// Enter class in enclosing package scope, in case it was an inner class before flatten.
// For top-level classes this does nothing.
val cls = tree.symbol.asClass
val pkg = cls.owner.asClass

// Bring back companion links
val companionClass = cls.info.decls.lookup(nme.COMPANION_CLASS_METHOD)
val companionModule = cls.info.decls.lookup(nme.COMPANION_MODULE_METHOD)

if (companionClass.exists) {
restoredDecls.enter(companionClass)
}

if (companionModule.exists) {
restoredDecls.enter(companionModule)
}

pkg.enter(cls)
val cinfo = cls.classInfo
tree.symbol.copySymDenotation(
info = cinfo.derivedClassInfo( // Dotty deviation: Cannot expand cinfo inline without a type error
decls = restoredDecls: Scope)).installAfter(thisTransform)
tree
val cls = tree.symbol.asClass
val pkg = cls.owner.asClass

// Bring back companion links
val companionClass = cls.info.decls.lookup(nme.COMPANION_CLASS_METHOD)
val companionModule = cls.info.decls.lookup(nme.COMPANION_MODULE_METHOD)

if (companionClass.exists) {
restoredDecls.enter(companionClass)
}

if (companionModule.exists) {
restoredDecls.enter(companionModule)
}

pkg.enter(cls)
val cinfo = cls.classInfo
tree.symbol.copySymDenotation(
info = cinfo.derivedClassInfo( // Dotty deviation: Cannot expand cinfo inline without a type error
decls = restoredDecls: Scope)).installAfter(thisTransform)
tree
case tree => tree
}
}

5 changes: 5 additions & 0 deletions tests/run/t920.scala → tests/pending/run/t920.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,9 @@ object Test {
def main(args : Array[String]) : Unit = {
b.initialize;
}
class XYZ
}