Skip to content

Memoize: duplicate scala2 behaviour: don't create fields for final vals. #773

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 11 commits into from
Sep 14, 2015
Merged
1 change: 0 additions & 1 deletion src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ class Compiler {
List(new VCInlineMethods,
new SeqLiterals,
new InterceptedMethods,
new Literalize,
new Getters,
new ClassTags,
new ElimByName,
Expand Down
14 changes: 11 additions & 3 deletions src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -326,18 +326,26 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
// see a detailed explanation of this trick in `GenSymbols.reifyFreeTerm`
free.symbol.hasStableFlag && isIdempotentExpr(free)
*/
case Apply(fn, Nil) =>
case Apply(fn, args) =>
def isKnownPureOp(sym: Symbol) =
sym.owner.isPrimitiveValueClass || sym.owner == defn.StringClass
// Note: After uncurry, field accesses are represented as Apply(getter, Nil),
// so an Apply can also be pure.
if (fn.symbol is Stable) exprPurity(fn) else Impure
if (args.isEmpty && fn.symbol.is(Stable)) exprPurity(fn)
else if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol))
// A constant expression with pure arguments is pure.
minOf(exprPurity(fn), args.map(exprPurity))
else Impure
case Typed(expr, _) =>
exprPurity(expr)
case Block(stats, expr) =>
(exprPurity(expr) /: stats.map(statPurity))(_ min _)
minOf(exprPurity(expr), stats.map(statPurity))
case _ =>
Impure
}

private def minOf(l0: PurityLevel, ls: List[PurityLevel]) = (l0 /: ls)(_ min _)

def isPureExpr(tree: tpd.Tree)(implicit ctx: Context) = exprPurity(tree) == Pure
def isIdempotentExpr(tree: tpd.Tree)(implicit ctx: Context) = exprPurity(tree) >= Idempotent

Expand Down
8 changes: 8 additions & 0 deletions src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,14 @@ object Types {
case _ => this
}

/** Widen from TermRef to its underlying non-termref
* base type, while also skipping Expr types.
*/
final def widenTermRefExpr(implicit ctx: Context): Type = stripTypeVar match {
case tp: TermRef if !tp.isOverloaded => tp.underlying.widenExpr.widenTermRefExpr
case _ => this
}

/** Widen from ExprType type to its result type.
* (Note: no stripTypeVar needed because TypeVar's can't refer to ExprTypes.)
*/
Expand Down
25 changes: 19 additions & 6 deletions src/dotty/tools/dotc/transform/Memoize.scala
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,26 @@ import Decorators._
lazy val field = sym.field.orElse(newField).asTerm
if (sym.is(Accessor, butNot = NoFieldNeeded))
if (sym.isGetter) {
var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform)
if (isWildcardArg(rhs)) rhs = EmptyTree
val fieldDef = transformFollowing(ValDef(field, rhs))
val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info))
Thicket(fieldDef, getterDef)
def skipBlocks(t: Tree): Tree = t match {
case Block(_, t1) => skipBlocks(t1)
case _ => t
}
else if (sym.isSetter) {
skipBlocks(tree.rhs) match {
case lit: Literal if sym.is(Final) && isIdempotentExpr(tree.rhs) =>
// duplicating scalac behavior: for final vals that have rhs as constant, we do not create a field
// and instead return the value. This seemingly minor optimization has huge effect on initialization
// order and the values that can be observed during superconstructor call

// see remark about idempotency in PostTyper#normalizeTree
cpy.DefDef(tree)(rhs = lit)
case _ =>
var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform)
if (isWildcardArg(rhs)) rhs = EmptyTree
val fieldDef = transformFollowing(ValDef(field, rhs))
val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info))
Thicket(fieldDef, getterDef)
}
} else if (sym.isSetter) {
if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // this is intended as an assertion
val initializer = Assign(ref(field), ref(tree.vparamss.head.head.symbol))
cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(ctx.withOwner(sym), info))
Expand Down
41 changes: 38 additions & 3 deletions src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,43 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
// TODO fill in
}

private def normalizeTypeTree(tree: Tree)(implicit ctx: Context) = {
def norm(tree: Tree) = if (tree.isType) TypeTree(tree.tpe).withPos(tree.pos) else tree
/** Check bounds of AppliedTypeTrees.
* Replace type trees with TypeTree nodes.
* Replace constant expressions with Literal nodes.
* Note: Demanding idempotency instead of purityin literalize is strictly speaking too loose.
* Example
*
* object O { final val x = 42; println("43") }
* O.x
*
* Strictly speaking we can't replace `O.x` with `42`. But this would make
* most expressions non-constant. Maybe we can change the spec to accept this
* kind of eliding behavior. Or else enforce true purity in the compiler.
* The choice will be affected by what we will do with `inline` and with
* Singleton type bounds (see SIP 23). Presumably
*
* object O1 { val x: Singleton = 42; println("43") }
* object O2 { inline val x = 42; println("43") }
*
* should behave differently.
*
* O1.x should have the same effect as { println("43"; 42 }
*
* whereas
*
* O2.x = 42
*
* Revisit this issue once we have implemented `inline`. Then we can demand
* purity of the prefix unless the selection goes to an inline val.
*/
private def normalizeTree(tree: Tree)(implicit ctx: Context): Tree = {
def literalize(tp: Type): Tree = tp.widenTermRefExpr match {
case ConstantType(value) if isIdempotentExpr(tree) => Literal(value)
case _ => tree
}
def norm(tree: Tree) =
if (tree.isType) TypeTree(tree.tpe).withPos(tree.pos)
else literalize(tree.tpe)
tree match {
case tree: TypeTree =>
tree
Expand Down Expand Up @@ -115,7 +150,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
}

override def transform(tree: Tree)(implicit ctx: Context): Tree =
try normalizeTypeTree(tree) match {
try normalizeTree(tree) match {
case tree: Ident =>
tree.tpe match {
case tpe: ThisType => This(tpe.cls).withPos(tree.pos)
Expand Down
8 changes: 4 additions & 4 deletions src/dotty/tools/dotc/typer/ConstFold.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ object ConstFold {
def apply(tree: Tree)(implicit ctx: Context): Tree = finish(tree) {
tree match {
case Apply(Select(xt, op), yt :: Nil) =>
xt.tpe match {
xt.tpe.widenTermRefExpr match {
case ConstantType(x) =>
yt.tpe match {
yt.tpe.widenTermRefExpr match {
case ConstantType(y) => foldBinop(op, x, y)
case _ => null
}
case _ => null
}
case Select(xt, op) =>
xt.tpe match {
xt.tpe.widenTermRefExpr match {
case ConstantType(x) => foldUnop(op, x)
case _ => null
}
Expand All @@ -42,7 +42,7 @@ object ConstFold {
*/
def apply(tree: Tree, pt: Type)(implicit ctx: Context): Tree =
finish(apply(tree)) {
tree.tpe match {
tree.tpe.widenTermRefExpr match {
case ConstantType(x) => x convertTo pt
case _ => null
}
Expand Down
13 changes: 7 additions & 6 deletions src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -707,18 +707,19 @@ class Namer { typer: Typer =>
// println(s"final inherited for $sym: ${inherited.toString}") !!!
// println(s"owner = ${sym.owner}, decls = ${sym.owner.info.decls.show}")
def isInline = sym.is(Final, butNot = Method)
def widenRhs(tp: Type): Type = tp match {
case tp: TermRef => widenRhs(tp.underlying)
case tp: ExprType => widenRhs(tp.resultType)
def widenRhs(tp: Type): Type = tp.widenTermRefExpr match {
case tp: ConstantType if isInline => tp
case _ => tp.widen.approximateUnion
}
val rhsCtx = ctx.addMode(Mode.InferringReturnType)
def rhsType = typedAheadExpr(mdef.rhs, rhsProto)(rhsCtx).tpe
def rhsType = typedAheadExpr(mdef.rhs, inherited orElse rhsProto)(rhsCtx).tpe
def cookedRhsType = ctx.deskolemize(widenRhs(rhsType))
def lhsType = fullyDefinedType(cookedRhsType, "right-hand side", mdef.pos)
lazy val lhsType = fullyDefinedType(cookedRhsType, "right-hand side", mdef.pos)
//if (sym.name.toString == "y") println(i"rhs = $rhsType, cooked = $cookedRhsType")
if (inherited.exists) inherited
if (inherited.exists)
if (sym.is(Final, butNot = Method) && lhsType.isInstanceOf[ConstantType])
lhsType // keep constant types that fill in for a non-constant (to be revised when inline has landed).
else inherited
else {
if (sym is Implicit) {
val resStr = if (mdef.isInstanceOf[DefDef]) "result " else ""
Expand Down
7 changes: 7 additions & 0 deletions tests/run/final-fields.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
T.f1
T.f2
T.f3
T.f4
3 2 0 0
3
g
43 changes: 43 additions & 0 deletions tests/run/final-fields.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
trait T {

val f1: Int = {println("T.f1"); -1}
val f2: Int = {println("T.f2"); -2}
val f3: Int = {println("T.f3"); -3}
val f4: Int = {println("T.f4"); -4}

println(s"$f1 $f2 $f3 $f4")
}

trait U {
val f2: Int
}

object Test0 extends U {
final val f1 = 1
final val f2 = 2
final val f3 = f1 + f2
val f4: 3 = f3
}

object Test1 extends U {
final val f1 = 1
final val f3 = f1 + f2
final val f2 = 2
val f4: 3 = f3


}

object Test extends T {
override final val f1 = /*super.f1*/ 1 + f2
override final val f2 = 2
override final val f3 = {println(3); 3}
override val f4 = f3 + 1

def g: 3 = { println("g"); 3 }
final val x = g + 1
def main(args: Array[String]): Unit = {
Test0
Test1
}
}