Skip to content

Commit 50f9cfa

Browse files
authored
Merge pull request #3839 from dotty-staging/fix-#2939
Fix #2939: Avoid duplicating symbols in default arguments
2 parents 3ca1171 + b1908a0 commit 50f9cfa

File tree

5 files changed

+128
-75
lines changed

5 files changed

+128
-75
lines changed

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

+19-16
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,9 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
311311
import tpd._
312312

313313
/** The purity level of this statement.
314-
* @return pure if statement has no side effects
315-
* idempotent if running the statement a second time has no side effects
316-
* impure otherwise
314+
* @return Pure if statement has no side effects
315+
* Idempotent if running the statement a second time has no side effects
316+
* Impure otherwise
317317
*/
318318
private def statPurity(tree: Tree)(implicit ctx: Context): PurityLevel = unsplice(tree) match {
319319
case EmptyTree
@@ -322,17 +322,18 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
322322
| DefDef(_, _, _, _, _) =>
323323
Pure
324324
case vdef @ ValDef(_, _, _) =>
325-
if (vdef.symbol.flags is Mutable) Impure else exprPurity(vdef.rhs)
325+
if (vdef.symbol.flags is Mutable) Impure else exprPurity(vdef.rhs) `min` Pure
326326
case _ =>
327327
Impure
328328
// TODO: It seem like this should be exprPurity(tree)
329329
// But if we do that the repl/vars test break. Need to figure out why that's the case.
330330
}
331331

332332
/** The purity level of this expression.
333-
* @return pure if expression has no side effects
334-
* idempotent if running the expression a second time has no side effects
335-
* impure otherwise
333+
* @return SimplyPure if expression has no side effects and cannot contain local definitions
334+
* Pure if expression has no side effects
335+
* Idempotent if running the expression a second time has no side effects
336+
* Impure otherwise
336337
*
337338
* Note that purity and idempotency are different. References to modules and lazy
338339
* vals are impure (side-effecting) both because side-effecting code may be executed and because the first reference
@@ -345,7 +346,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
345346
| Super(_, _)
346347
| Literal(_)
347348
| Closure(_, _, _) =>
348-
Pure
349+
SimplyPure
349350
case Ident(_) =>
350351
refPurity(tree)
351352
case Select(qual, _) =>
@@ -366,7 +367,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
366367
if (args.isEmpty && fn.symbol.is(Stable)) exprPurity(fn)
367368
else if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol))
368369
// A constant expression with pure arguments is pure.
369-
minOf(exprPurity(fn), args.map(exprPurity))
370+
minOf(exprPurity(fn), args.map(exprPurity)) `min` Pure
370371
else Impure
371372
case Typed(expr, _) =>
372373
exprPurity(expr)
@@ -382,25 +383,26 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
382383

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

385-
def isPureExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) == Pure
386+
def isSimplyPure(tree: Tree)(implicit ctx: Context) = exprPurity(tree) == SimplyPure
387+
def isPureExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) >= Pure
386388
def isIdempotentExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) >= Idempotent
387389

388390
/** The purity level of this reference.
389391
* @return
390-
* pure if reference is (nonlazy and stable) or to a parameterized function
391-
* idempotent if reference is lazy and stable
392-
* impure otherwise
392+
* SimplyPure if reference is (nonlazy and stable) or to a parameterized function
393+
* Idempotent if reference is lazy and stable
394+
* Impure otherwise
393395
* @DarkDimius: need to make sure that lazy accessor methods have Lazy and Stable
394396
* flags set.
395397
*/
396398
private def refPurity(tree: Tree)(implicit ctx: Context): PurityLevel =
397-
if (!tree.tpe.widen.isParameterless) Pure
399+
if (!tree.tpe.widen.isParameterless) SimplyPure
398400
else if (!tree.symbol.isStable) Impure
399401
else if (tree.symbol.is(Lazy)) Idempotent // TODO add Module flag, sinxce Module vals or not Lazy from the start.
400-
else Pure
402+
else SimplyPure
401403

402404
def isPureRef(tree: Tree)(implicit ctx: Context) =
403-
refPurity(tree) == Pure
405+
refPurity(tree) == SimplyPure
404406
def isIdempotentRef(tree: Tree)(implicit ctx: Context) =
405407
refPurity(tree) >= Idempotent
406408

@@ -724,6 +726,7 @@ object TreeInfo {
724726
def min(that: PurityLevel) = new PurityLevel(x min that.x)
725727
}
726728

729+
val SimplyPure = new PurityLevel(3)
727730
val Pure = new PurityLevel(2)
728731
val Idempotent = new PurityLevel(1)
729732
val Impure = new PurityLevel(0)

compiler/src/dotty/tools/dotc/transform/PhantomArgLift.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import dotty.tools.dotc.core.Contexts._
55
import dotty.tools.dotc.core.NameKinds._
66
import dotty.tools.dotc.core.Types._
77
import dotty.tools.dotc.transform.MegaPhase.MiniPhase
8-
import dotty.tools.dotc.typer.EtaExpansion
8+
import dotty.tools.dotc.typer.LiftImpure
99

1010
import scala.collection.mutable.ListBuffer
1111

@@ -46,7 +46,7 @@ class PhantomArgLift extends MiniPhase {
4646
if (!hasImpurePhantomArgs(tree)) tree
4747
else {
4848
val buffer = ListBuffer.empty[Tree]
49-
val app = EtaExpansion.liftApp(buffer, tree)
49+
val app = LiftImpure.liftApp(buffer, tree)
5050
if (buffer.isEmpty) app
5151
else Block(buffer.result(), app)
5252
}

compiler/src/dotty/tools/dotc/typer/Applications.scala

+9-6
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import Names._
2222
import StdNames._
2323
import NameKinds.DefaultGetterName
2424
import ProtoTypes._
25-
import EtaExpansion._
2625
import Inferencing._
2726

2827
import collection.mutable
@@ -426,7 +425,8 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
426425
}
427426

428427
def tryDefault(n: Int, args1: List[Arg]): Unit = {
429-
if (!isJavaAnnotConstr(methRef.symbol)) liftFun()
428+
if (!isJavaAnnotConstr(methRef.symbol))
429+
liftFun()
430430
val getter = findDefaultGetter(n + numArgs(normalizedFun))
431431
if (getter.isEmpty) missingArg(n)
432432
else {
@@ -571,10 +571,13 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
571571

572572
def normalizedFun = myNormalizedFun
573573

574+
private def lifter(implicit ctx: Context) =
575+
if (methRef.symbol.hasDefaultParams) LiftComplex else LiftImpure
576+
574577
override def liftFun(): Unit =
575578
if (liftedDefs == null) {
576579
liftedDefs = new mutable.ListBuffer[Tree]
577-
myNormalizedFun = liftApp(liftedDefs, myNormalizedFun)
580+
myNormalizedFun = lifter.liftApp(liftedDefs, myNormalizedFun)
578581
}
579582

580583
/** The index of the first difference between lists of trees `xs` and `ys`
@@ -608,13 +611,13 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
608611

609612
// lift arguments in the definition order
610613
val argDefBuf = mutable.ListBuffer.empty[Tree]
611-
typedArgs = liftArgs(argDefBuf, methType, typedArgs)
614+
typedArgs = lifter.liftArgs(argDefBuf, methType, typedArgs)
612615

613616
// Lifted arguments ordered based on the original order of typedArgBuf and
614617
// with all non-explicit default parameters at the end in declaration order.
615618
val orderedArgDefs = {
616619
// List of original arguments that are lifted by liftArgs
617-
val impureArgs = typedArgBuf.filterNot(isPureExpr)
620+
val impureArgs = typedArgBuf.filterNot(lifter.noLift)
618621
// Assuming stable sorting all non-explicit default parameters will remain in the end with the same order
619622
val defaultParamIndex = args.size
620623
// Mapping of index of each `liftable` into original args ordering
@@ -746,7 +749,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
746749
val Apply(Select(lhs, name), rhss) = tree
747750
val lhs1 = typedExpr(lhs)
748751
val liftedDefs = new mutable.ListBuffer[Tree]
749-
val lhs2 = untpd.TypedSplice(liftAssigned(liftedDefs, lhs1))
752+
val lhs2 = untpd.TypedSplice(LiftComplex.liftAssigned(liftedDefs, lhs1))
750753
val assign = untpd.Assign(lhs2,
751754
untpd.Apply(untpd.Select(lhs2, name.asSimpleName.dropRight(1)), rhss))
752755
wrapDefs(liftedDefs, typed(assign))

compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala

+63-51
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,44 @@ import Decorators._
1313
import Names._
1414
import StdNames._
1515
import NameKinds.UniqueName
16-
import Trees._
1716
import Inferencing._
1817
import util.Positions._
1918
import collection.mutable
19+
import Trees._
2020

21-
object EtaExpansion {
22-
21+
/** A class that handles argument lifting. Argument lifting is needed in the following
22+
* scenarios:
23+
* - eta expansion
24+
* - applications with default arguments
25+
* - applications with out-of-order named arguments
26+
* Lifting generally lifts impure expressions only, except in the case of possible
27+
* default arguments, where we lift also complex pure expressions, since in that case
28+
* arguments can be duplicated as arguments to default argument methods.
29+
*/
30+
abstract class Lifter {
2331
import tpd._
2432

33+
/** Test indicating `expr` does not need lifting */
34+
def noLift(expr: Tree)(implicit ctx: Context): Boolean
35+
36+
/** The corresponding lifter for pass-by-name arguments */
37+
protected def exprLifter: Lifter = NoLift
38+
39+
/** The flags of a lifted definition */
40+
protected def liftedFlags: FlagSet = EmptyFlags
41+
42+
/** The tree of a lifted definition */
43+
protected def liftedDef(sym: TermSymbol, rhs: Tree)(implicit ctx: Context): MemberDef = ValDef(sym, rhs)
44+
2545
private def lift(defs: mutable.ListBuffer[Tree], expr: Tree, prefix: TermName = EmptyTermName)(implicit ctx: Context): Tree =
26-
if (isPureExpr(expr)) expr
46+
if (noLift(expr)) expr
2747
else {
2848
val name = UniqueName.fresh(prefix)
29-
val liftedType = fullyDefinedType(expr.tpe.widen, "lifted expression", expr.pos)
30-
val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, liftedType, coord = positionCoord(expr.pos))
31-
defs += ValDef(sym, expr).withPos(expr.pos.focus)
32-
ref(sym.termRef).withPos(expr.pos)
49+
var liftedType = fullyDefinedType(expr.tpe.widen, "lifted expression", expr.pos)
50+
if (liftedFlags.is(Method)) liftedType = ExprType(liftedType)
51+
val lifted = ctx.newSymbol(ctx.owner, name, liftedFlags, liftedType, coord = positionCoord(expr.pos))
52+
defs += liftedDef(lifted, expr).withPos(expr.pos.focus)
53+
ref(lifted.termRef).withPos(expr.pos)
3354
}
3455

3556
/** Lift out common part of lhs tree taking part in an operator assignment such as
@@ -49,7 +70,7 @@ object EtaExpansion {
4970
}
5071

5172
/** Lift a function argument, stripping any NamedArg wrapper */
52-
def liftArg(defs: mutable.ListBuffer[Tree], arg: Tree, prefix: TermName = EmptyTermName)(implicit ctx: Context): Tree =
73+
private def liftArg(defs: mutable.ListBuffer[Tree], arg: Tree, prefix: TermName = EmptyTermName)(implicit ctx: Context): Tree =
5374
arg match {
5475
case arg @ NamedArg(name, arg1) => cpy.NamedArg(arg)(name, lift(defs, arg1, prefix))
5576
case arg => lift(defs, arg, prefix)
@@ -61,12 +82,12 @@ object EtaExpansion {
6182
def liftArgs(defs: mutable.ListBuffer[Tree], methRef: Type, args: List[Tree])(implicit ctx: Context) =
6283
methRef.widen match {
6384
case mt: MethodType =>
64-
(args, mt.paramNames, mt.paramInfos).zipped map { (arg, name, tp) =>
65-
if (tp.isInstanceOf[ExprType]) arg
66-
else liftArg(defs, arg, if (name.firstPart contains '$') EmptyTermName else name)
85+
(args, mt.paramNames, mt.paramInfos).zipped.map { (arg, name, tp) =>
86+
val lifter = if (tp.isInstanceOf[ExprType]) exprLifter else this
87+
lifter.liftArg(defs, arg, if (name.firstPart contains '$') EmptyTermName else name)
6788
}
6889
case _ =>
69-
args map (liftArg(defs, _))
90+
args.map(liftArg(defs, _))
7091
}
7192

7293
/** Lift out function prefix and all arguments from application
@@ -108,6 +129,35 @@ object EtaExpansion {
108129
case New(_) => tree
109130
case _ => if (isIdempotentExpr(tree)) tree else lift(defs, tree)
110131
}
132+
}
133+
134+
/** No lifting at all */
135+
object NoLift extends Lifter {
136+
def noLift(expr: tpd.Tree)(implicit ctx: Context) = true
137+
}
138+
139+
/** Lift all impure arguments */
140+
class LiftImpure extends Lifter {
141+
def noLift(expr: tpd.Tree)(implicit ctx: Context) = tpd.isPureExpr(expr)
142+
}
143+
object LiftImpure extends LiftImpure
144+
145+
/** Lift all impure or complex arguments */
146+
class LiftComplex extends Lifter {
147+
def noLift(expr: tpd.Tree)(implicit ctx: Context) = tpd.isSimplyPure(expr)
148+
override def exprLifter = LiftToDefs
149+
}
150+
object LiftComplex extends LiftComplex
151+
152+
/** Lift all impure or complex arguments to `def`s */
153+
object LiftToDefs extends LiftComplex {
154+
override def liftedFlags: FlagSet = Method
155+
override def liftedDef(sym: TermSymbol, rhs: tpd.Tree)(implicit ctx: Context) = tpd.DefDef(sym, rhs)
156+
}
157+
158+
/** Lifter for eta expansion */
159+
object EtaExpansion extends LiftImpure {
160+
import tpd._
111161

112162
/** Eta-expanding a tree means converting a method reference to a function value.
113163
* @param tree The tree to expand
@@ -152,41 +202,3 @@ object EtaExpansion {
152202
if (defs.nonEmpty) untpd.Block(defs.toList map (untpd.TypedSplice(_)), fn) else fn
153203
}
154204
}
155-
156-
/** <p> not needed
157-
* Expand partial function applications of type `type`.
158-
* </p><pre>
159-
* p.f(es_1)...(es_n)
160-
* ==> {
161-
* <b>private synthetic val</b> eta$f = p.f // if p is not stable
162-
* ...
163-
* <b>private synthetic val</b> eta$e_i = e_i // if e_i is not stable
164-
* ...
165-
* (ps_1 => ... => ps_m => eta$f([es_1])...([es_m])(ps_1)...(ps_m))
166-
* }</pre>
167-
* <p>
168-
* tree is already attributed
169-
* </p>
170-
def etaExpandUntyped(tree: Tree)(implicit ctx: Context): untpd.Tree = { // kept as a reserve for now
171-
def expand(tree: Tree): untpd.Tree = tree.tpe match {
172-
case mt @ MethodType(paramNames, paramTypes) if !mt.isImplicit =>
173-
val paramsArgs: List[(untpd.ValDef, untpd.Tree)] =
174-
(paramNames, paramTypes).zipped.map { (name, tp) =>
175-
val droppedStarTpe = defn.underlyingOfRepeated(tp)
176-
val param = ValDef(
177-
Modifiers(Param), name,
178-
untpd.TypedSplice(TypeTree(droppedStarTpe)), untpd.EmptyTree)
179-
var arg: untpd.Tree = Ident(name)
180-
if (defn.isRepeatedParam(tp))
181-
arg = Typed(arg, Ident(tpnme.WILDCARD_STAR))
182-
(param, arg)
183-
}
184-
val (params, args) = paramsArgs.unzip
185-
untpd.Function(params, Apply(untpd.TypedSplice(tree), args))
186-
}
187-
188-
val defs = new mutable.ListBuffer[Tree]
189-
val tree1 = liftApp(defs, tree)
190-
Block(defs.toList map untpd.TypedSplice, expand(tree1))
191-
}
192-
*/

tests/run/i2939.scala

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import scala.collection.mutable._
2+
3+
class Tag(val name: String, val buffer: Buffer[Tag] = ArrayBuffer()) {
4+
def space(n: Int = 0): String = {
5+
s"${" " * n}<$name>\n" +
6+
(if(buffer.isEmpty) "" else buffer.map(_.space(n + 4)).mkString("", "\n", "\n")) +
7+
s"${" " * n}</$name>"
8+
}
9+
10+
def apply[U](f: implicit Tag => U)(implicit tag: Tag = null): this.type = {
11+
f(this)
12+
if(tag != null) tag.buffer += this
13+
this
14+
}
15+
16+
override def toString(): String = space(0)
17+
}
18+
19+
object Tag {
20+
implicit def toTag(symbol: Symbol): Tag = new Tag(symbol.name)
21+
22+
def main(args: Array[String]): Unit = {
23+
}
24+
}
25+
26+
27+
object Test {
28+
def foo(x: Int => Int)(y: Int = 0) = {}
29+
def bar(x: => Int)(y: Int = 0) = {}
30+
31+
def main(args: Array[String]): Unit = {
32+
foo(x => x)()
33+
bar(args.length)()
34+
}
35+
}

0 commit comments

Comments
 (0)