Skip to content

Commit bb0f005

Browse files
OlivierBlanvillainOlivierBlanvillain
committed
Formatting, adressing smarter comments
1 parent 03c0ca6 commit bb0f005

8 files changed

+126
-85
lines changed

‎compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala

+14-12
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,11 @@ class DropNoEffects(val simplifyPhase: Simplify)(implicit val ctx: Context) exte
4242
else newExpr
4343

4444
// Keep only side effect free statements unit returning functions
45-
case a: DefDef if (a.symbol.info.finalResultType.derivesFrom(defn.UnitClass) &&
46-
!a.rhs.tpe.derivesFrom(defn.UnitClass) &&
47-
!a.rhs.tpe.derivesFrom(defn.NothingClass)) =>
45+
case a: DefDef
46+
if a.symbol.info.finalResultType.derivesFrom(defn.UnitClass) &&
47+
!a.rhs.tpe.derivesFrom(defn.UnitClass) &&
48+
!a.rhs.tpe.derivesFrom(defn.NothingClass) =>
49+
4850
def insertUnit(t: Tree) = {
4951
if (!t.tpe.derivesFrom(defn.UnitClass)) Block(t :: Nil, unitLiteral)
5052
else t
@@ -74,15 +76,15 @@ class DropNoEffects(val simplifyPhase: Simplify)(implicit val ctx: Context) exte
7476
elsep = nelsep.orElse(if (elsep.isInstanceOf[Literal]) elsep else unitLiteral))
7577

7678
// Accessing a field of a product
77-
case t @ Select(rec, _) if
78-
(t.symbol.isGetter && !t.symbol.is(Mutable | Lazy)) ||
79-
(t.symbol.owner.derivesFrom(defn.ProductClass) && t.symbol.owner.is(CaseClass) && t.symbol.name.isSelectorName) ||
80-
(t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable)) =>
81-
keepOnlySideEffects(rec)
82-
83-
case s @ Select(qual, name) if
84-
// !name.eq(nme.TYPE_) && // Keep the .TYPE added by ClassOf, would be needed for AfterErasure
85-
!s.symbol.is(Mutable | Lazy) && !s.symbol.is(Method) =>
79+
case t @ Select(rec, _)
80+
if (t.symbol.isGetter && !t.symbol.is(Mutable | Lazy)) ||
81+
(t.symbol.owner.derivesFrom(defn.ProductClass) && t.symbol.owner.is(CaseClass) && t.symbol.name.isSelectorName) ||
82+
(t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable)) =>
83+
84+
keepOnlySideEffects(rec)
85+
86+
// !name.eq(nme.TYPE_) && // Keep the .TYPE added by ClassOf, would be needed for AfterErasure
87+
case s @ Select(qual, name) if !s.symbol.is(Mutable | Lazy | Method) =>
8688
keepOnlySideEffects(qual)
8789

8890
case Block(List(t: DefDef), s: Closure) =>

‎compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala

+26-22
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ import ast.Trees._
1111
import transform.SymUtils._
1212
import Simplify.desugarIdent
1313

14-
/** Inline case class specific methods using desugarings assumptions. */
14+
/** Inline case class specific methods using desugarings assumptions.
15+
*
16+
* Note: to run this optimisation after erasure one would need to specialize
17+
* it for constructor with outer pointer and values classes. There is
18+
* probably no need to run this more than once.
19+
*/
1520
class InlineCaseIntrinsics(implicit val ctx: Context) extends Optimisation {
1621
import ast.tpd._
1722

@@ -20,13 +25,14 @@ class InlineCaseIntrinsics(implicit val ctx: Context) extends Optimisation {
2025
def transformer(localCtx: Context): Tree => Tree = {
2126
// For synthetic applies on case classes (both dotty/scalac)
2227
// - CC.apply(args) → new CC(args)
23-
case a: Apply if !a.tpe.isInstanceOf[MethodicType] &&
24-
a.symbol.is(Synthetic) &&
25-
a.symbol.owner.is(Module) &&
26-
(a.symbol.name == nme.apply) &&
27-
a.symbol.owner.companionClass.is(CaseClass) &&
28-
!a.tpe.derivesFrom(defn.EnumClass) &&
29-
(isPureExpr(a.fun) || a.fun.symbol.is(Synthetic)) =>
28+
case a: Apply
29+
if !a.tpe.isInstanceOf[MethodicType] &&
30+
a.symbol.is(Synthetic) &&
31+
a.symbol.owner.is(Module) &&
32+
a.symbol.name == nme.apply &&
33+
a.symbol.owner.companionClass.is(CaseClass) &&
34+
!a.tpe.derivesFrom(defn.EnumClass) &&
35+
(isPureExpr(a.fun) || a.fun.symbol.is(Synthetic)) =>
3036

3137
def unrollArgs(t: Tree, l: List[List[Tree]]): List[List[Tree]] = t match {
3238
case Apply(t, args) => unrollArgs(t, args :: l)
@@ -44,12 +50,13 @@ class InlineCaseIntrinsics(implicit val ctx: Context) extends Optimisation {
4450
// - CC.unapply(arg): CC → arg
4551
// - CC.unapply(arg): Boolean → true, dotty only
4652
// - CC.unapply(arg): Option[CC] → new Some(new scala.TupleN(arg._1, ..., arg._N))
47-
case a: Apply if a.symbol.is(Synthetic) &&
48-
a.symbol.owner.is(Module) &&
49-
(a.symbol.name == nme.unapply) &&
50-
a.symbol.owner.companionClass.is(CaseClass) &&
51-
!a.tpe.derivesFrom(defn.EnumClass) &&
52-
(isPureExpr(a.fun) || a.fun.symbol.is(Synthetic)) =>
53+
case a: Apply
54+
if a.symbol.is(Synthetic) &&
55+
a.symbol.owner.is(Module) &&
56+
a.symbol.name == nme.unapply &&
57+
a.symbol.owner.companionClass.is(CaseClass) &&
58+
!a.tpe.derivesFrom(defn.EnumClass) &&
59+
(isPureExpr(a.fun) || a.fun.symbol.is(Synthetic)) =>
5360

5461
val args = a.args.head
5562
val isDottyUnapply = !a.symbol.owner.is(Scala2x)
@@ -88,10 +95,11 @@ class InlineCaseIntrinsics(implicit val ctx: Context) extends Optimisation {
8895

8996
// Seq.unapplySeq(arg) → new Some(arg)
9097
// Where Seq is any companion of type <: SeqFactoryClass
91-
case a: Apply if (a.symbol.name == nme.unapplySeq) &&
92-
a.symbol.owner.derivesFrom(defn.SeqFactoryClass) &&
93-
a.symbol.extendedOverriddenSymbols.isEmpty &&
94-
(isPureExpr(a.fun) || a.fun.symbol.is(Synthetic)) =>
98+
case a: Apply
99+
if a.symbol.name == nme.unapplySeq &&
100+
a.symbol.owner.derivesFrom(defn.SeqFactoryClass) &&
101+
a.symbol.extendedOverriddenSymbols.isEmpty &&
102+
(isPureExpr(a.fun) || a.fun.symbol.is(Synthetic)) =>
95103

96104
def receiver(t: Tree): Type = t match {
97105
case t: Apply => receiver(t.fun)
@@ -127,8 +135,4 @@ class InlineCaseIntrinsics(implicit val ctx: Context) extends Optimisation {
127135
else
128136
Block(recv :: Nil, res)
129137
}
130-
131-
// To run this optimisation after erasure one would need to specialize it
132-
// for constructor with outer pointer and values classes. There is probably
133-
// no need to run this more than once.
134138
}

‎compiler/src/dotty/tools/dotc/transform/localopt/InlineLabelsCalledOnce.scala

+30-13
Original file line numberDiff line numberDiff line change
@@ -13,38 +13,55 @@ class InlineLabelsCalledOnce(implicit val ctx: Context) extends Optimisation {
1313
import ast.tpd._
1414

1515
val timesUsed = mutable.HashMap[Symbol, Int]()
16-
val defined = mutable.HashMap[Symbol, DefDef]()
16+
val defined = mutable.HashMap[Symbol, DefDef]()
1717

1818
val visitor: Tree => Unit = {
19-
case defdef: DefDef if defdef.symbol.is(Label) =>
19+
case d: DefDef if d.symbol.is(Label) =>
2020
var isRecursive = false
21-
defdef.rhs.foreachSubTree(x => if (x.symbol == defdef.symbol) isRecursive = true)
22-
if (!isRecursive) defined.put(defdef.symbol, defdef)
21+
d.rhs.foreachSubTree { x =>
22+
if (x.symbol == d.symbol)
23+
isRecursive = true
24+
}
25+
if (!isRecursive)
26+
defined.put(d.symbol, d)
27+
2328
case t: Apply if t.symbol.is(Label) =>
2429
val b4 = timesUsed.getOrElseUpdate(t.symbol, 0)
2530
timesUsed.put(t.symbol, b4 + 1)
31+
2632
case _ =>
2733
}
2834

2935
def transformer(localCtx: Context): Tree => Tree = {
3036
case a: Apply =>
3137
defined.get(a.symbol) match {
32-
case None => a
33-
case Some(defDef) if a.symbol.is(Label) && timesUsed.getOrElse(a.symbol, 0) == 1 && a.symbol.info.paramInfoss == List(Nil) =>
38+
case Some(defDef) if usedOnce(a) && a.symbol.info.paramInfoss == List(Nil) =>
3439
simplify.println(s"Inlining labeldef ${defDef.name}")
3540
defDef.rhs.changeOwner(defDef.symbol, localCtx.owner)
41+
3642
case Some(defDef) if defDef.rhs.isInstanceOf[Literal] =>
3743
defDef.rhs
38-
case Some(_) =>
39-
a
44+
45+
case _ => a
4046
}
41-
case a: DefDef if (a.symbol.is(Label) && timesUsed.getOrElse(a.symbol, 0) == 1 && defined.contains(a.symbol)) =>
42-
simplify.println(s"Dropping labeldef (used once) ${a.name} ${timesUsed.get(a.symbol)}")
43-
defined.put(a.symbol, a)
47+
48+
case d: DefDef if usedOnce(d) =>
49+
simplify.println(s"Dropping labeldef (used once) ${d.name} ${timesUsed.get(d.symbol)}")
50+
defined.put(d.symbol, d)
4451
EmptyTree
45-
case a: DefDef if (a.symbol.is(Label) && timesUsed.getOrElse(a.symbol, 0) == 0 && defined.contains(a.symbol)) =>
46-
simplify.println(s"Dropping labeldef (never used) ${a.name} ${timesUsed.get(a.symbol)}")
52+
53+
case d: DefDef if neverUsed(d) =>
54+
simplify.println(s"Dropping labeldef (never used) ${d.name} ${timesUsed.get(d.symbol)}")
4755
EmptyTree
56+
4857
case t => t
4958
}
59+
60+
def usedN(t: Tree, n: Int): Boolean =
61+
t.symbol.is(Label) &&
62+
timesUsed.getOrElse(t.symbol, 0) == n &&
63+
defined.contains(t.symbol)
64+
65+
def usedOnce(t: Tree): Boolean = usedN(t, 1)
66+
def neverUsed(t: Tree): Boolean = usedN(t, 0)
5067
}

‎compiler/src/dotty/tools/dotc/transform/localopt/InlineLocalObjects.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import transform.SymUtils._
1414
import config.Printers.simplify
1515

1616
/** Inline case classes as vals, this essentially (local) implements multi
17-
* parameter value classes. The main motivation is to get ride of all the
18-
* intermediate tuples coming from pattern matching expressions.
19-
*/
17+
* parameter value classes. The main motivation is to get ride of all the
18+
* intermediate tuples coming from pattern matching expressions.
19+
*/
2020
class InlineLocalObjects(implicit val ctx: Context) extends Optimisation {
2121
import ast.tpd._
2222

‎compiler/src/dotty/tools/dotc/transform/localopt/Jumpjump.scala

+9-7
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@ class Jumpjump(implicit val ctx: Context) extends Optimisation {
2020
val visitor: Tree => Unit = {
2121
case defdef: DefDef if defdef.symbol.is(Label) =>
2222
defdef.rhs match {
23-
case Apply(t, args) if t.symbol.is(Label) &&
24-
TypeErasure.erasure(defdef.symbol.info.finalResultType).classSymbol ==
25-
TypeErasure.erasure(t.symbol.info.finalResultType).classSymbol &&
26-
args.size == defdef.vparamss.map(_.size).sum &&
27-
args.zip(defdef.vparamss.flatten).forall(x => x._1.symbol eq x._2.symbol) &&
28-
!(defdef.symbol eq t.symbol) =>
29-
defined(defdef.symbol) = t.symbol
23+
case Apply(t, args)
24+
if t.symbol.is(Label) &&
25+
TypeErasure.erasure(defdef.symbol.info.finalResultType).classSymbol ==
26+
TypeErasure.erasure(t.symbol.info.finalResultType).classSymbol &&
27+
args.size == defdef.vparamss.map(_.size).sum &&
28+
args.zip(defdef.vparamss.flatten).forall(x => x._1.symbol eq x._2.symbol) &&
29+
!(defdef.symbol eq t.symbol) =>
30+
31+
defined(defdef.symbol) = t.symbol
3032
case _ =>
3133
}
3234
case _ =>

‎compiler/src/dotty/tools/dotc/transform/localopt/RemoveUnnecessaryNullChecks.scala

+25-10
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import scala.collection.mutable
2424

2525
val checkGood = mutable.HashMap[Symbol, Set[Symbol]]()
2626

27-
def isGood(t: Symbol) = {
27+
def isGood(t: Symbol): Boolean = {
2828
t.exists && initializedVals.contains(t) && {
2929
var changed = true
3030
var set = Set(t)
@@ -43,21 +43,31 @@ import scala.collection.mutable
4343
val rhsName = rhs.symbol.name
4444
if (!vd.symbol.is(Mutable) && !rhs.isEmpty) {
4545
def checkNonNull(t: Tree, target: Symbol): Boolean = t match {
46-
case Block(_ , expr) => checkNonNull(expr, target)
47-
case If(_, thenp, elsep) => checkNonNull(thenp, target) && checkNonNull(elsep, target)
48-
case t: New => true
46+
case Block(_ , expr) =>
47+
checkNonNull(expr, target)
48+
49+
case If(_, thenp, elsep) =>
50+
checkNonNull(thenp, target) && checkNonNull(elsep, target)
51+
52+
case _: New | _: This => true
53+
4954
case t: Apply if t.symbol.isPrimaryConstructor => true
55+
5056
case t: Literal => t.const.value != null
51-
case t: This => true
57+
5258
case t: Ident if !t.symbol.owner.isClass =>
5359
checkGood.put(target, checkGood.getOrElse(target, Set.empty) + t.symbol)
5460
true
61+
5562
case t: Apply if !t.symbol.owner.isClass =>
5663
checkGood.put(target, checkGood.getOrElse(target, Set.empty) + t.symbol)
5764
true
65+
5866
case t: Typed =>
5967
checkNonNull(t.expr, target)
68+
6069
case _ => t.tpe.isNotNull
70+
6171
}
6272
if (checkNonNull(vd.rhs, vd.symbol))
6373
initializedVals += vd.symbol
@@ -74,14 +84,19 @@ import scala.collection.mutable
7484
case _ => false
7585
}
7686
val transformation: Tree => Tree = {
77-
case check@Apply(Select(lhs, _), List(rhs)) =>
87+
case check @ Apply(Select(lhs, _), List(rhs)) =>
7888
val sym = check.symbol
79-
if ( ((sym == defn.Object_eq) || (sym == defn.Object_ne)) &&
80-
((isNullLiteral(lhs) && isGood(rhs.symbol)) || (isNullLiteral(rhs) && isGood(lhs.symbol)))) {
81-
if (sym == defn.Object_eq) Block(List(lhs, rhs), Literal(Constant(false)))
82-
else if(sym == defn.Object_ne) Block(List(lhs, rhs), Literal(Constant(true)))
89+
val eqOrNe = sym == defn.Object_eq || sym == defn.Object_ne
90+
val nullLhs = isNullLiteral(lhs) && isGood(rhs.symbol)
91+
val nullRhs = isNullLiteral(rhs) && isGood(lhs.symbol)
92+
93+
if (eqOrNe && nullLhs || nullRhs) {
94+
def block(b: Boolean) = Block(List(lhs, rhs), Literal(Constant(b)))
95+
if (sym == defn.Object_eq) block(false)
96+
else if (sym == defn.Object_ne) block(true)
8397
else check
8498
} else check
99+
85100
case t => t
86101
}
87102
transformation

‎compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala

+6-6
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,18 @@ class Simplify extends MiniPhaseTransform with IdentityDenotTransformer {
2222

2323
private def beforeErasure(implicit ctx: Context): List[Optimisation] =
2424
new InlineCaseIntrinsics ::
25-
new RemoveUnnecessaryNullChecks ::
25+
new RemoveUnnecessaryNullChecks :: // Bootstraps!
2626
new InlineOptions ::
27-
new InlineLabelsCalledOnce ::
27+
new InlineLabelsCalledOnce :: // On it!
2828
new Valify(this) ::
2929
new Devalify ::
30-
new Jumpjump ::
31-
new DropGoodCasts ::
32-
new DropNoEffects(this) ::
30+
new Jumpjump :: // Bootstraps!
31+
new DropGoodCasts :: // Bootstraps!
32+
new DropNoEffects(this) :: // Bootstraps!
3333
// new InlineLocalObjects :: // followCases needs to be fixed, see ./tests/pos/rbtree.scala
3434
// new Varify :: // varify could stop other transformations from being applied. postponed.
3535
// new BubbleUpNothing ::
36-
new ConstantFold ::
36+
new ConstantFold :: // Bootstraps!
3737
Nil
3838

3939
private def afterErasure(implicit ctx: Context): List[Optimisation] =

‎compiler/src/dotty/tools/dotc/transform/localopt/Varify.scala

+13-12
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,23 @@ import scala.collection.mutable
3030
val possibleRenames = mutable.HashMap[Symbol, Set[Symbol]]()
3131

3232
val visitor: Tree => Unit = {
33-
case t: ValDef
34-
if t.symbol.is(Param) =>
33+
case t: ValDef if t.symbol.is(Param) =>
3534
paramsTimesUsed += (t.symbol -> 0)
36-
case valDef: ValDef
37-
if valDef.symbol.is(Mutable) =>
35+
36+
case valDef: ValDef if valDef.symbol.is(Mutable) =>
3837
valDef.rhs.foreachSubTree { subtree =>
3938
if (paramsTimesUsed.contains(subtree.symbol) &&
4039
valDef.symbol.info.widenDealias <:< subtree.symbol.info.widenDealias) {
4140
val newSet = possibleRenames.getOrElse(valDef.symbol, Set.empty) + subtree.symbol
4241
possibleRenames.put(valDef.symbol, newSet)
4342
}
4443
}
45-
case t: RefTree
46-
if paramsTimesUsed.contains(t.symbol) =>
44+
45+
case t: RefTree if paramsTimesUsed.contains(t.symbol) =>
4746
val param = t.symbol
4847
val current = paramsTimesUsed.get(param)
4948
current foreach { c => paramsTimesUsed += (param -> (c + 1)) }
49+
5050
case _ =>
5151
}
5252

@@ -57,19 +57,20 @@ import scala.collection.mutable
5757
.filter(x => x._2.nonEmpty)
5858
.map(x => (x._1, x._2.head))
5959
.toMap
60+
6061
val transformation: Tree => Tree = {
61-
case t: RefTree
62-
if renames.contains(t.symbol) =>
62+
case t: RefTree if renames.contains(t.symbol) =>
6363
ref(renames(t.symbol))
64-
case t: ValDef
65-
if renames.contains(t.symbol) =>
64+
65+
case t: ValDef if renames.contains(t.symbol) =>
6666
val replaced = renames(t.symbol)
6767
if (t.rhs.symbol == replaced) EmptyTree
6868
else ref(replaced).becomes(t.rhs)
69-
case t: ValDef
70-
if paramCandidates.contains(t.symbol) =>
69+
70+
case t: ValDef if paramCandidates.contains(t.symbol) =>
7171
t.symbol.flags = Mutable
7272
t
73+
7374
case t => t
7475
}
7576
transformation

0 commit comments

Comments
 (0)