Skip to content

Commit e7ef08d

Browse files
committed
Merge pull request #773 from dotty-staging/final-vals2
Memoize: duplicate scala2 behaviour: don't create fields for final vals.
2 parents 2a2940e + e491cfb commit e7ef08d

File tree

9 files changed

+137
-23
lines changed

9 files changed

+137
-23
lines changed

src/dotty/tools/dotc/Compiler.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ class Compiler {
5656
List(new VCInlineMethods,
5757
new SeqLiterals,
5858
new InterceptedMethods,
59-
new Literalize,
6059
new Getters,
6160
new ClassTags,
6261
new ElimByName,

src/dotty/tools/dotc/ast/TreeInfo.scala

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,18 +326,26 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
326326
// see a detailed explanation of this trick in `GenSymbols.reifyFreeTerm`
327327
free.symbol.hasStableFlag && isIdempotentExpr(free)
328328
*/
329-
case Apply(fn, Nil) =>
329+
case Apply(fn, args) =>
330+
def isKnownPureOp(sym: Symbol) =
331+
sym.owner.isPrimitiveValueClass || sym.owner == defn.StringClass
330332
// Note: After uncurry, field accesses are represented as Apply(getter, Nil),
331333
// so an Apply can also be pure.
332-
if (fn.symbol is Stable) exprPurity(fn) else Impure
334+
if (args.isEmpty && fn.symbol.is(Stable)) exprPurity(fn)
335+
else if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol))
336+
// A constant expression with pure arguments is pure.
337+
minOf(exprPurity(fn), args.map(exprPurity))
338+
else Impure
333339
case Typed(expr, _) =>
334340
exprPurity(expr)
335341
case Block(stats, expr) =>
336-
(exprPurity(expr) /: stats.map(statPurity))(_ min _)
342+
minOf(exprPurity(expr), stats.map(statPurity))
337343
case _ =>
338344
Impure
339345
}
340346

347+
private def minOf(l0: PurityLevel, ls: List[PurityLevel]) = (l0 /: ls)(_ min _)
348+
341349
def isPureExpr(tree: tpd.Tree)(implicit ctx: Context) = exprPurity(tree) == Pure
342350
def isIdempotentExpr(tree: tpd.Tree)(implicit ctx: Context) = exprPurity(tree) >= Idempotent
343351

src/dotty/tools/dotc/core/Types.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,14 @@ object Types {
715715
case _ => this
716716
}
717717

718+
/** Widen from TermRef to its underlying non-termref
719+
* base type, while also skipping Expr types.
720+
*/
721+
final def widenTermRefExpr(implicit ctx: Context): Type = stripTypeVar match {
722+
case tp: TermRef if !tp.isOverloaded => tp.underlying.widenExpr.widenTermRefExpr
723+
case _ => this
724+
}
725+
718726
/** Widen from ExprType type to its result type.
719727
* (Note: no stripTypeVar needed because TypeVar's can't refer to ExprTypes.)
720728
*/

src/dotty/tools/dotc/transform/Memoize.scala

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,26 @@ import Decorators._
7070
lazy val field = sym.field.orElse(newField).asTerm
7171
if (sym.is(Accessor, butNot = NoFieldNeeded))
7272
if (sym.isGetter) {
73-
var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform)
74-
if (isWildcardArg(rhs)) rhs = EmptyTree
75-
val fieldDef = transformFollowing(ValDef(field, rhs))
76-
val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info))
77-
Thicket(fieldDef, getterDef)
73+
def skipBlocks(t: Tree): Tree = t match {
74+
case Block(_, t1) => skipBlocks(t1)
75+
case _ => t
7876
}
79-
else if (sym.isSetter) {
77+
skipBlocks(tree.rhs) match {
78+
case lit: Literal if sym.is(Final) && isIdempotentExpr(tree.rhs) =>
79+
// duplicating scalac behavior: for final vals that have rhs as constant, we do not create a field
80+
// and instead return the value. This seemingly minor optimization has huge effect on initialization
81+
// order and the values that can be observed during superconstructor call
82+
83+
// see remark about idempotency in PostTyper#normalizeTree
84+
cpy.DefDef(tree)(rhs = lit)
85+
case _ =>
86+
var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform)
87+
if (isWildcardArg(rhs)) rhs = EmptyTree
88+
val fieldDef = transformFollowing(ValDef(field, rhs))
89+
val getterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(ref(field))(ctx.withOwner(sym), info))
90+
Thicket(fieldDef, getterDef)
91+
}
92+
} else if (sym.isSetter) {
8093
if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // this is intended as an assertion
8194
val initializer = Assign(ref(field), ref(tree.vparamss.head.head.symbol))
8295
cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(ctx.withOwner(sym), info))

src/dotty/tools/dotc/transform/PostTyper.scala

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,43 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
6868
// TODO fill in
6969
}
7070

71-
private def normalizeTypeTree(tree: Tree)(implicit ctx: Context) = {
72-
def norm(tree: Tree) = if (tree.isType) TypeTree(tree.tpe).withPos(tree.pos) else tree
71+
/** Check bounds of AppliedTypeTrees.
72+
* Replace type trees with TypeTree nodes.
73+
* Replace constant expressions with Literal nodes.
74+
* Note: Demanding idempotency instead of purityin literalize is strictly speaking too loose.
75+
* Example
76+
*
77+
* object O { final val x = 42; println("43") }
78+
* O.x
79+
*
80+
* Strictly speaking we can't replace `O.x` with `42`. But this would make
81+
* most expressions non-constant. Maybe we can change the spec to accept this
82+
* kind of eliding behavior. Or else enforce true purity in the compiler.
83+
* The choice will be affected by what we will do with `inline` and with
84+
* Singleton type bounds (see SIP 23). Presumably
85+
*
86+
* object O1 { val x: Singleton = 42; println("43") }
87+
* object O2 { inline val x = 42; println("43") }
88+
*
89+
* should behave differently.
90+
*
91+
* O1.x should have the same effect as { println("43"; 42 }
92+
*
93+
* whereas
94+
*
95+
* O2.x = 42
96+
*
97+
* Revisit this issue once we have implemented `inline`. Then we can demand
98+
* purity of the prefix unless the selection goes to an inline val.
99+
*/
100+
private def normalizeTree(tree: Tree)(implicit ctx: Context): Tree = {
101+
def literalize(tp: Type): Tree = tp.widenTermRefExpr match {
102+
case ConstantType(value) if isIdempotentExpr(tree) => Literal(value)
103+
case _ => tree
104+
}
105+
def norm(tree: Tree) =
106+
if (tree.isType) TypeTree(tree.tpe).withPos(tree.pos)
107+
else literalize(tree.tpe)
73108
tree match {
74109
case tree: TypeTree =>
75110
tree
@@ -115,7 +150,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
115150
}
116151

117152
override def transform(tree: Tree)(implicit ctx: Context): Tree =
118-
try normalizeTypeTree(tree) match {
153+
try normalizeTree(tree) match {
119154
case tree: Ident =>
120155
tree.tpe match {
121156
case tpe: ThisType => This(tpe.cls).withPos(tree.pos)

src/dotty/tools/dotc/typer/ConstFold.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,16 @@ object ConstFold {
2020
def apply(tree: Tree)(implicit ctx: Context): Tree = finish(tree) {
2121
tree match {
2222
case Apply(Select(xt, op), yt :: Nil) =>
23-
xt.tpe match {
23+
xt.tpe.widenTermRefExpr match {
2424
case ConstantType(x) =>
25-
yt.tpe match {
25+
yt.tpe.widenTermRefExpr match {
2626
case ConstantType(y) => foldBinop(op, x, y)
2727
case _ => null
2828
}
2929
case _ => null
3030
}
3131
case Select(xt, op) =>
32-
xt.tpe match {
32+
xt.tpe.widenTermRefExpr match {
3333
case ConstantType(x) => foldUnop(op, x)
3434
case _ => null
3535
}
@@ -42,7 +42,7 @@ object ConstFold {
4242
*/
4343
def apply(tree: Tree, pt: Type)(implicit ctx: Context): Tree =
4444
finish(apply(tree)) {
45-
tree.tpe match {
45+
tree.tpe.widenTermRefExpr match {
4646
case ConstantType(x) => x convertTo pt
4747
case _ => null
4848
}

src/dotty/tools/dotc/typer/Namer.scala

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -707,18 +707,19 @@ class Namer { typer: Typer =>
707707
// println(s"final inherited for $sym: ${inherited.toString}") !!!
708708
// println(s"owner = ${sym.owner}, decls = ${sym.owner.info.decls.show}")
709709
def isInline = sym.is(Final, butNot = Method)
710-
def widenRhs(tp: Type): Type = tp match {
711-
case tp: TermRef => widenRhs(tp.underlying)
712-
case tp: ExprType => widenRhs(tp.resultType)
710+
def widenRhs(tp: Type): Type = tp.widenTermRefExpr match {
713711
case tp: ConstantType if isInline => tp
714712
case _ => tp.widen.approximateUnion
715713
}
716714
val rhsCtx = ctx.addMode(Mode.InferringReturnType)
717-
def rhsType = typedAheadExpr(mdef.rhs, rhsProto)(rhsCtx).tpe
715+
def rhsType = typedAheadExpr(mdef.rhs, inherited orElse rhsProto)(rhsCtx).tpe
718716
def cookedRhsType = ctx.deskolemize(widenRhs(rhsType))
719-
def lhsType = fullyDefinedType(cookedRhsType, "right-hand side", mdef.pos)
717+
lazy val lhsType = fullyDefinedType(cookedRhsType, "right-hand side", mdef.pos)
720718
//if (sym.name.toString == "y") println(i"rhs = $rhsType, cooked = $cookedRhsType")
721-
if (inherited.exists) inherited
719+
if (inherited.exists)
720+
if (sym.is(Final, butNot = Method) && lhsType.isInstanceOf[ConstantType])
721+
lhsType // keep constant types that fill in for a non-constant (to be revised when inline has landed).
722+
else inherited
722723
else {
723724
if (sym is Implicit) {
724725
val resStr = if (mdef.isInstanceOf[DefDef]) "result " else ""

tests/run/final-fields.check

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
T.f1
2+
T.f2
3+
T.f3
4+
T.f4
5+
3 2 0 0
6+
3
7+
g

tests/run/final-fields.scala

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
trait T {
2+
3+
val f1: Int = {println("T.f1"); -1}
4+
val f2: Int = {println("T.f2"); -2}
5+
val f3: Int = {println("T.f3"); -3}
6+
val f4: Int = {println("T.f4"); -4}
7+
8+
println(s"$f1 $f2 $f3 $f4")
9+
}
10+
11+
trait U {
12+
val f2: Int
13+
}
14+
15+
object Test0 extends U {
16+
final val f1 = 1
17+
final val f2 = 2
18+
final val f3 = f1 + f2
19+
val f4: 3 = f3
20+
}
21+
22+
object Test1 extends U {
23+
final val f1 = 1
24+
final val f3 = f1 + f2
25+
final val f2 = 2
26+
val f4: 3 = f3
27+
28+
29+
}
30+
31+
object Test extends T {
32+
override final val f1 = /*super.f1*/ 1 + f2
33+
override final val f2 = 2
34+
override final val f3 = {println(3); 3}
35+
override val f4 = f3 + 1
36+
37+
def g: 3 = { println("g"); 3 }
38+
final val x = g + 1
39+
def main(args: Array[String]): Unit = {
40+
Test0
41+
Test1
42+
}
43+
}

0 commit comments

Comments
 (0)