Skip to content

Run all miniphase transforms at group end #3350

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 20 commits into from
Oct 26, 2017
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
12 changes: 6 additions & 6 deletions compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ class Compiler {
new ElimJavaPackages), // Eliminate syntactic references to Java packages
List(new CheckStatic, // Check restrictions that apply to @static members
new ElimRepeated, // Rewrite vararg parameters and arguments
new RefChecks, // Various checks mostly related to abstract members and overriding
new NormalizeFlags, // Rewrite some definition flags
new ExtensionMethods, // Expand methods of value classes with extension methods
new ExpandSAMs, // Expand single abstract method closures to anonymous classes
new TailRec, // Rewrite tail recursion to loops
new ByNameClosures, // Expand arguments to by-name parameters to closures
new LiftTry, // Put try expressions that might execute on non-empty stacks into their own methods
new HoistSuperArgs, // Hoist complex arguments of supercalls to enclosing scope
new ClassOf), // Expand `Predef.classOf` calls.
new ClassOf, // Expand `Predef.classOf` calls.
new RefChecks), // Various checks mostly related to abstract members and overriding
List(new TryCatchPatterns, // Compile cases in try/catch
new PatternMatcher, // Compile pattern matches
new ExplicitOuter, // Add accessors to outer classes from nested ones.
Expand Down Expand Up @@ -98,10 +98,10 @@ class Compiler {
List(new LinkScala2Impls, // Redirect calls to trait methods defined by Scala 2.x, so that they now go to their implementations
new LambdaLift, // Lifts out nested functions to class scope, storing free variables in environments
// Note: in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here
new ElimStaticThis, // Replace `this` references to static objects by global identifiers
new Flatten, // Lift all inner classes to package scope
new RestoreScopes), // Repair scopes rendered invalid by moving definitions in prior phases of the group
List(new RenameLifted, // Renames lifted classes to local numbering scheme
new ElimStaticThis), // Replace `this` references to static objects by global identifiers
List(new Flatten, // Lift all inner classes to package scope
new RestoreScopes, // Repair scopes rendered invalid by moving definitions in prior phases of the group
new RenameLifted, // Renames lifted classes to local numbering scheme
new TransformWildcards, // Replace wildcards with default values
new MoveStatics, // Move static methods to companion classes
new ExpandPrivate, // Widen private definitions accessed from nested classes
Expand Down
37 changes: 19 additions & 18 deletions compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -656,26 +656,27 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
/** After phase `trans`, set the owner of every definition in this tree that was formerly
* owner by `from` to `to`.
*/
def changeOwnerAfter(from: Symbol, to: Symbol, trans: DenotTransformer)(implicit ctx: Context): ThisTree = {
assert(ctx.phase == trans.next)
val traverser = new TreeTraverser {
def traverse(tree: Tree)(implicit ctx: Context) = tree match {
case tree: DefTree =>
val sym = tree.symbol
val prevDenot = sym.denot(ctx.withPhase(trans))
if (prevDenot.effectiveOwner == from.skipWeakOwner) {
val d = sym.copySymDenotation(owner = to)
d.installAfter(trans)
d.transformAfter(trans, d => if (d.owner eq from) d.copySymDenotation(owner = to) else d)
}
if (sym.isWeakOwner) traverseChildren(tree)
case _ =>
traverseChildren(tree)
def changeOwnerAfter(from: Symbol, to: Symbol, trans: DenotTransformer)(implicit ctx: Context): ThisTree =
if (ctx.phase == trans.next) {
val traverser = new TreeTraverser {
def traverse(tree: Tree)(implicit ctx: Context) = tree match {
case tree: DefTree =>
val sym = tree.symbol
val prevDenot = sym.denot(ctx.withPhase(trans))
if (prevDenot.effectiveOwner == from.skipWeakOwner) {
val d = sym.copySymDenotation(owner = to)
d.installAfter(trans)
d.transformAfter(trans, d => if (d.owner eq from) d.copySymDenotation(owner = to) else d)
}
if (sym.isWeakOwner) traverseChildren(tree)
case _ =>
traverseChildren(tree)
}
}
traverser.traverse(tree)
tree
}
traverser.traverse(tree)
tree
}
else changeOwnerAfter(from, to, trans)(ctx.withPhase(trans.next))

/** A select node with the given selector name and a computed type */
def select(name: Name)(implicit ctx: Context): Select =
Expand Down
5 changes: 0 additions & 5 deletions compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,6 @@ object Phases {
*/
def checkPostCondition(tree: tpd.Tree)(implicit ctx: Context): Unit = ()

/** If set, allow missing or superfluous arguments in applications
* and type applications.
*/
def relaxedTyping: Boolean = false

/** Is this phase the standard typerphase? True for FrontEnd, but
* not for other first phases (such as FromTasty). The predicate
* is tested in some places that perform checks and corrections. It's
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/CapturedVars.scala
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer { thisTransfo
}

override def prepareForValDef(vdef: ValDef)(implicit ctx: Context) = {
val sym = vdef.symbol
val sym = vdef.symbol(ctx.withPhase(thisTransform))
if (captured contains sym) {
val newd = sym.denot(ctx.withPhase(thisTransform)).copySymDenotation(
info = refClass(sym.info.classSymbol, sym.hasAnnotation(defn.VolatileAnnot)).typeRef,
Expand Down Expand Up @@ -119,7 +119,7 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer { thisTransfo
override def transformIdent(id: Ident)(implicit ctx: Context, info: TransformerInfo): Tree = {
val vble = id.symbol
if (captured(vble))
(id select nme.elem).ensureConforms(vble.denot(ctx.withPhase(thisTransform)).info)
id.select(nme.elem).ensureConforms(vble.denot(ctx.withPhase(thisTransform)).info)
else id
}

Expand Down
11 changes: 7 additions & 4 deletions compiler/src/dotty/tools/dotc/transform/ElimByName.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ class ElimByName extends TransformByNameApply with InfoTransformer { thisTransfo

/** Map `tree` to `tree.apply()` is `ftree` was of ExprType and becomes now a function */
private def applyIfFunction(tree: Tree, ftree: Tree)(implicit ctx: Context) =
if (isByNameRef(ftree)) tree.select(defn.Function0_apply).appliedToNone
if (isByNameRef(ftree))
ctx.atPhase(next) { implicit ctx => tree.select(defn.Function0_apply).appliedToNone }
else tree

override def transformIdent(tree: Ident)(implicit ctx: Context, info: TransformerInfo): Tree =
Expand All @@ -65,9 +66,11 @@ class ElimByName extends TransformByNameApply with InfoTransformer { thisTransfo
}

override def transformValDef(tree: ValDef)(implicit ctx: Context, info: TransformerInfo): Tree =
if (exprBecomesFunction(tree.symbol))
cpy.ValDef(tree)(tpt = tree.tpt.withType(tree.symbol.info))
else tree
ctx.atPhase(next) { implicit ctx =>
if (exprBecomesFunction(tree.symbol))
cpy.ValDef(tree)(tpt = tree.tpt.withType(tree.symbol.info))
else tree
}

def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context): Type = tp match {
case ExprType(rt) if exprBecomesFunction(sym) => defn.FunctionOf(Nil, rt)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import dotty.tools.dotc.core.Phases.Phase
/** Make private term members that are accessed from another class
* non-private by resetting the Private flag and expanding their name.
*
* Make private accessor in value class not-private. Ihis is necessary to unbox
* Make private accessor in value class not-private. This is necessary to unbox
* the value class when accessing it from separate compilation units
*
* Also, make non-private any private parameter forwarders that forward to an inherited
Expand Down
12 changes: 4 additions & 8 deletions compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,11 @@ object ExplicitOuter {
import ast.tpd._

/** Ensure that class `cls` has outer accessors */
def ensureOuterAccessors(cls: ClassSymbol)(implicit ctx: Context): Unit = {
//todo: implementing #165 would simplify this logic
val prevPhase = ctx.phase.prev
assert(prevPhase.id <= ctx.explicitOuterPhase.id, "can add $outer symbols only before ExplicitOuter")
assert(prevPhase.isInstanceOf[DenotTransformer], "adding outerAccessors requires being DenotTransformer")
if (!hasOuter(cls)) {
newOuterAccessors(cls).foreach(_.enteredAfter(prevPhase.asInstanceOf[DenotTransformer]))
def ensureOuterAccessors(cls: ClassSymbol)(implicit ctx: Context): Unit =
ctx.atPhase(ctx.explicitOuterPhase.next) { implicit ctx =>
if (!hasOuter(cls))
newOuterAccessors(cls).foreach(_.enteredAfter(ctx.explicitOuterPhase.asInstanceOf[DenotTransformer]))
}
}

/** The outer accessor and potentially outer param accessor needed for class `cls` */
private def newOuterAccessors(cls: ClassSymbol)(implicit ctx: Context) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with Ful
private def createExtensionMethod(imeth: Symbol, staticClass: Symbol)(implicit ctx: Context): TermSymbol = {
val extensionName = extensionNames(imeth).head.toTermName
val extensionMeth = ctx.newSymbol(staticClass, extensionName,
imeth.flags | Final &~ (Override | Protected | AbsOverride),
(imeth.flags | Final) &~ (Override | Protected | AbsOverride),
fullyParameterizedType(imeth.info, imeth.owner.asClass),
privateWithin = imeth.privateWithin, coord = imeth.coord)
extensionMeth.addAnnotations(imeth.annotations)(ctx.withPhase(thisTransformer))
Expand Down Expand Up @@ -175,8 +175,8 @@ class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with Ful
extensionDefs(staticClass) = newC
newC
}
store += atGroupEnd(fullyParameterizedDef(extensionMeth, tree)(_))
cpy.DefDef(tree)(rhs = atGroupEnd(forwarder(extensionMeth, tree)(_)))
store += fullyParameterizedDef(extensionMeth, tree)
cpy.DefDef(tree)(rhs = forwarder(extensionMeth, tree))
} else tree
}
}
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/LambdaLift.scala
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisTransform
val phaseName: String = "lambdaLift"
val treeTransform = new LambdaLifter

override def relaxedTyping = true

override def runsAfterGroupsOf: Set[Class[_ <: Phase]] = Set(classOf[Constructors], classOf[HoistSuperArgs])
// Constructors has to happen before LambdaLift because the lambda lift logic
// becomes simpler if it can assume that parameter accessors have already been
Expand Down
10 changes: 4 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/LazyVals.scala
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,10 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer {
// backend requires field usage to be after field definition
// need to bring containers to start of method
val (holders, stats) =
atGroupEnd { implicit ctx: Context =>
trees.partition {
_.symbol.flags.&~(Flags.Touched) == containerFlags
// Filtering out Flags.Touched is not required currently, as there are no LazyTypes involved here
// but just to be more safe
}
trees.partition {
_.symbol.flags.&~(Flags.Touched) == containerFlags
// Filtering out Flags.Touched is not required currently, as there are no LazyTypes involved here
// but just to be more safe
}
holders:::stats
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import DenotTransformers.SymTransformer
import Phases.Phase
import Contexts.Context
import SymDenotations.SymDenotation
import TreeTransforms.MiniPhaseTransform
import TreeTransforms.{MiniPhaseTransform, TransformerInfo}
import Flags._, Symbols._

/** 1. Widens all private[this] and protected[this] qualifiers to just private/protected
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ object PatternMatcher {
def outerTest: Tree = trans.transformFollowingDeep {
val expectedOuter = singleton(expectedTp.normalizedPrefix)
val expectedClass = expectedTp.dealias.classSymbol.asClass
ExplicitOuter.ensureOuterAccessors(expectedClass)(ctx.withPhase(ctx.explicitOuterPhase.next))
ExplicitOuter.ensureOuterAccessors(expectedClass)
scrutinee.ensureConforms(expectedTp)
.outerSelect(1, expectedOuter.tpe.widen)
.select(defn.Object_eq)
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/RenameLifted.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ import dotty.tools.dotc.core.Names._
import dotty.tools.dotc.core.Phases
import dotty.tools.dotc.core.SymDenotations.SymDenotation
import dotty.tools.dotc.core.Symbols._
import dotty.tools.dotc.transform.TreeTransforms.MiniPhaseTransform
import dotty.tools.dotc.transform.TreeTransforms.{MiniPhaseTransform, TransformerInfo}

/** Renames lifted classes to local numbering scheme */
class RenameLifted extends MiniPhaseTransform with SymTransformer { thisTransformer =>

override def phaseName = "renameLifted"

override def runsAfterGroupsOf: Set[Class[_ <: Phases.Phase]] = Set(classOf[RestoreScopes])
// Not clear why this should run after restoreScopes
// override def runsAfterGroupsOf: Set[Class[_ <: Phases.Phase]] = Set(classOf[RestoreScopes])
Copy link
Member

Choose a reason for hiding this comment

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

@nicolasstucki Can you comment on that ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Originally it was on Flatten because this transformation acts on the flattened names. But @DarkDimius noted that the flattening transformation finishes in RestoreScopes.


def transformSym(ref: SymDenotation)(implicit ctx: Context): SymDenotation =
if (needsRefresh(ref.symbol)) ref.copySymDenotation(name = refreshedName(ref.symbol))
Expand Down
Loading