From da12146b8f3a5215e9685cd978d898131f67f1eb Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Sat, 25 Feb 2012 20:43:37 -0800 Subject: Toward a higher level abstraction than atPhase. I guess these are self-explanatory. @inline final def afterErasure[T](op: => T): T = afterPhase(currentRun.erasurePhase)(op) @inline final def afterExplicitOuter[T](op: => T): T = afterPhase(currentRun.explicitouterPhase)(op) ... @inline final def beforeTyper[T](op: => T): T = beforePhase(currentRun.typerPhase)(op) @inline final def beforeUncurry[T](op: => T): T = beforePhase(currentRun.uncurryPhase)(op) This commit is basically pure substitution. To get anywhere interesting with all the phase-related bugs we have to determine why we use atPhase and capture that reasoning directly. With the exception of erasure, most phases don't have much meaning outside of the compiler. How can anyone know why a block of code which says atPhase(explicitouter.prev) { ... } needs to run there? Too much cargo cult, and it should stop. Every usage of atPhase should be commented as to intention. It's easy to find bugs like atPhase(uncurryPhase.prev) which was probably intended to run before uncurry, but actually runs before whatever happens to be before uncurry - which, luckily enough, is non-deterministic because the continuations plugin inserts phases between refchecks and uncurry. % scalac -Xplugin-disable:continuations -Xshow-phases refchecks 7 reference/override checking, translate nested objects uncurry 8 uncurry, translate function values to anonymous classes % scalac -Xshow-phases selectivecps 9 uncurry 10 uncurry, translate function values to anonymous classes Expressions like atPhase(somePhase.prev) are never right or are at best highly suboptimal, because most of the time you have no guarantees about what phase precedes you. Anyway, I think most or all atPhases expressed that way only wanted to run before somePhase, and because one can never be too sure without searching for documentation whether "atPhase" means before or after, people err on the side of caution and overshoot by a phase. Unfortunately, this usually works. (I prefer bugs which never work.) --- .../tools/selectivecps/SelectiveANFTransform.scala | 20 ++++++------ .../tools/selectivecps/SelectiveCPSTransform.scala | 38 +++++++++++----------- 2 files changed, 29 insertions(+), 29 deletions(-) (limited to 'src/continuations/plugin') diff --git a/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala b/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala index cbb33e68c3..d98169f21a 100644 --- a/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala +++ b/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala @@ -47,20 +47,20 @@ abstract class SelectiveANFTransform extends PluginComponent with Transform with // ValDef case here. case dd @ DefDef(mods, name, tparams, vparamss, tpt, rhs) => - log("transforming " + dd.symbol) + debuglog("transforming " + dd.symbol) atOwner(dd.symbol) { val rhs1 = transExpr(rhs, None, getExternalAnswerTypeAnn(tpt.tpe)) - log("result "+rhs1) - log("result is of type "+rhs1.tpe) + debuglog("result "+rhs1) + debuglog("result is of type "+rhs1.tpe) treeCopy.DefDef(dd, mods, name, transformTypeDefs(tparams), transformValDefss(vparamss), transform(tpt), rhs1) } case ff @ Function(vparams, body) => - log("transforming anon function " + ff.symbol) + debuglog("transforming anon function " + ff.symbol) atOwner(ff.symbol) { @@ -88,14 +88,14 @@ abstract class SelectiveANFTransform extends PluginComponent with Transform with transExpr(body, None, ext) } - log("result "+body1) - log("result is of type "+body1.tpe) + debuglog("result "+body1) + debuglog("result is of type "+body1.tpe) treeCopy.Function(ff, transformValDefs(vparams), body1) } case vd @ ValDef(mods, name, tpt, rhs) => // object-level valdefs - log("transforming valdef " + vd.symbol) + debuglog("transforming valdef " + vd.symbol) atOwner(vd.symbol) { @@ -298,8 +298,8 @@ abstract class SelectiveANFTransform extends PluginComponent with Transform with if (!expr.isEmpty && (expr.tpe.typeSymbol ne NothingClass)) { // must convert! - log("cps type conversion (has: " + cpsA + "/" + spc + "/" + expr.tpe + ")") - log("cps type conversion (expected: " + cpsR.get + "): " + expr) + debuglog("cps type conversion (has: " + cpsA + "/" + spc + "/" + expr.tpe + ")") + debuglog("cps type conversion (expected: " + cpsR.get + "): " + expr) if (!hasPlusMarker(expr.tpe)) unit.warning(tree.pos, "expression " + tree + " is cps-transformed unexpectedly") @@ -322,7 +322,7 @@ abstract class SelectiveANFTransform extends PluginComponent with Transform with } else if (!cpsR.isDefined && bot.isDefined) { // error! - log("cps type error: " + expr) + debuglog("cps type error: " + expr) //println("cps type error: " + expr + "/" + expr.tpe + "/" + getAnswerTypeAnn(expr.tpe)) //println(cpsR + "/" + spc + "/" + bot) diff --git a/src/continuations/plugin/scala/tools/selectivecps/SelectiveCPSTransform.scala b/src/continuations/plugin/scala/tools/selectivecps/SelectiveCPSTransform.scala index a90dc36639..6453671eac 100644 --- a/src/continuations/plugin/scala/tools/selectivecps/SelectiveCPSTransform.scala +++ b/src/continuations/plugin/scala/tools/selectivecps/SelectiveCPSTransform.scala @@ -39,10 +39,10 @@ abstract class SelectiveCPSTransform extends PluginComponent with val newtp = transformCPSType(tp) if (newtp != tp) - log("transformInfo changed type for " + sym + " to " + newtp); + debuglog("transformInfo changed type for " + sym + " to " + newtp); if (sym == MethReifyR) - log("transformInfo (not)changed type for " + sym + " to " + newtp); + debuglog("transformInfo (not)changed type for " + sym + " to " + newtp); newtp } @@ -83,13 +83,13 @@ abstract class SelectiveCPSTransform extends PluginComponent with case Apply(TypeApply(fun, targs), args) if (fun.symbol == MethShift) => - log("found shift: " + tree) + debuglog("found shift: " + tree) atPos(tree.pos) { val funR = gen.mkAttributedRef(MethShiftR) // TODO: correct? //gen.mkAttributedSelect(gen.mkAttributedSelect(gen.mkAttributedSelect(gen.mkAttributedIdent(ScalaPackage), //ScalaPackage.tpe.member("util")), ScalaPackage.tpe.member("util").tpe.member("continuations")), MethShiftR) //gen.mkAttributedRef(ModCPS.tpe, MethShiftR) // TODO: correct? - log(funR.tpe) + debuglog("funR.tpe = " + funR.tpe) Apply( TypeApply(funR, targs).setType(appliedType(funR.tpe, targs.map((t:Tree) => t.tpe))), args.map(transform(_)) @@ -98,10 +98,10 @@ abstract class SelectiveCPSTransform extends PluginComponent with case Apply(TypeApply(fun, targs), args) if (fun.symbol == MethShiftUnit) => - log("found shiftUnit: " + tree) + debuglog("found shiftUnit: " + tree) atPos(tree.pos) { val funR = gen.mkAttributedRef(MethShiftUnitR) // TODO: correct? - log(funR.tpe) + debuglog("funR.tpe = " + funR.tpe) Apply( TypeApply(funR, List(targs(0), targs(1))).setType(appliedType(funR.tpe, List(targs(0).tpe, targs(1).tpe))), @@ -114,7 +114,7 @@ abstract class SelectiveCPSTransform extends PluginComponent with log("found reify: " + tree) atPos(tree.pos) { val funR = gen.mkAttributedRef(MethReifyR) // TODO: correct? - log(funR.tpe) + debuglog("funR.tpe = " + funR.tpe) Apply( TypeApply(funR, targs).setType(appliedType(funR.tpe, targs.map((t:Tree) => t.tpe))), args.map(transform(_)) @@ -258,17 +258,17 @@ abstract class SelectiveCPSTransform extends PluginComponent with case vd @ ValDef(mods, name, tpt, rhs) if (vd.symbol.hasAnnotation(MarkerCPSSym)) => - log("found marked ValDef "+name+" of type " + vd.symbol.tpe) + debuglog("found marked ValDef "+name+" of type " + vd.symbol.tpe) val tpe = vd.symbol.tpe val rhs1 = atOwner(vd.symbol) { transform(rhs) } rhs1.changeOwner(vd.symbol -> currentOwner) // TODO: don't traverse twice - log("valdef symbol " + vd.symbol + " has type " + tpe) - log("right hand side " + rhs1 + " has type " + rhs1.tpe) + debuglog("valdef symbol " + vd.symbol + " has type " + tpe) + debuglog("right hand side " + rhs1 + " has type " + rhs1.tpe) - log("currentOwner: " + currentOwner) - log("currentMethod: " + currentMethod) + debuglog("currentOwner: " + currentOwner) + debuglog("currentMethod: " + currentMethod) val (bodyStms, bodyExpr) = transBlock(rest, expr) // FIXME: result will later be traversed again by TreeSymSubstituter and @@ -308,12 +308,12 @@ abstract class SelectiveCPSTransform extends PluginComponent with // see note about multiple traversals above - log("fun.symbol: "+fun.symbol) - log("fun.symbol.owner: "+fun.symbol.owner) - log("arg.owner: "+arg.owner) + debuglog("fun.symbol: "+fun.symbol) + debuglog("fun.symbol.owner: "+fun.symbol.owner) + debuglog("arg.owner: "+arg.owner) - log("fun.tpe:"+fun.tpe) - log("return type of fun:"+body1.tpe) + debuglog("fun.tpe:"+fun.tpe) + debuglog("return type of fun:"+body1.tpe) var methodName = nme.map @@ -324,7 +324,7 @@ abstract class SelectiveCPSTransform extends PluginComponent with else unit.error(rhs.pos, "cannot compute type for CPS-transformed function result") - log("will use method:"+methodName) + debuglog("will use method:"+methodName) localTyper.typed(atPos(vd.symbol.pos) { Apply(Select(ctxR, ctxR.tpe.member(methodName)), List(fun)) @@ -335,7 +335,7 @@ abstract class SelectiveCPSTransform extends PluginComponent with try { if (specialCaseTrivial) { - log("will optimize possible tail call: " + bodyExpr) + debuglog("will optimize possible tail call: " + bodyExpr) // FIXME: flatMap impl has become more complicated due to // exceptions. do we need to put a try/catch in the then part?? -- cgit v1.2.3