Skip to content

Commit 1191671

Browse files
Lift all non trivial prefixes for default parameters (#19739)
Checking if the prefix is pure is not enough to know if we need to list the prefix. In the case of default parameters, the prefix tree might be used several times to compute the default values. This expression should only be computed once and therefore it should be lifted if there is some computation/allocation involved. Furthermore, if the prefix contains a local definition, it must be lifted to avoid duplicating the definition. A similar situation could happen with dependent default parameters. This currently works as expected. Fixes #15315
2 parents 55df3f3 + 71b983b commit 1191671

File tree

3 files changed

+80
-2
lines changed

3 files changed

+80
-2
lines changed

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

+23-2
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,10 @@ abstract class Lifter {
122122
case TypeApply(fn, targs) =>
123123
cpy.TypeApply(tree)(liftApp(defs, fn), targs)
124124
case Select(pre, name) if isPureRef(tree) =>
125-
cpy.Select(tree)(liftPrefix(defs, pre), name)
125+
val liftedPrefix =
126+
if tree.symbol.is(HasDefaultParams) then liftPrefix(defs, pre)
127+
else liftNonIdempotentPrefix(defs, pre)
128+
cpy.Select(tree)(liftedPrefix, name)
126129
case Block(stats, expr) =>
127130
liftApp(defs ++= stats, expr)
128131
case New(tpt) =>
@@ -138,8 +141,26 @@ abstract class Lifter {
138141
*
139142
* unless `pre` is idempotent.
140143
*/
141-
def liftPrefix(defs: mutable.ListBuffer[Tree], tree: Tree)(using Context): Tree =
144+
def liftNonIdempotentPrefix(defs: mutable.ListBuffer[Tree], tree: Tree)(using Context): Tree =
142145
if (isIdempotentExpr(tree)) tree else lift(defs, tree)
146+
147+
/** Lift prefix `pre` of an application `pre.f(...)` to
148+
*
149+
* val x0 = pre
150+
* x0.f(...)
151+
*
152+
* unless `pre` is idempotent reference, a `this` reference, a literal value, or a or the prefix of an `init` (`New` tree).
153+
*
154+
* Note that default arguments will refer to the prefix, we do not want
155+
* to re-evaluate a complex expression each time we access a getter.
156+
*/
157+
def liftPrefix(defs: mutable.ListBuffer[Tree], tree: Tree)(using Context): Tree =
158+
tree match
159+
case tree: Literal => tree
160+
case tree: This => tree
161+
case tree: New => tree // prefix of <init> call
162+
case tree: RefTree if isIdempotentExpr(tree) => tree
163+
case _ => lift(defs, tree)
143164
}
144165

145166
/** No lifting at all */

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

+52
Original file line numberDiff line numberDiff line change
@@ -1733,6 +1733,58 @@ class DottyBytecodeTests extends DottyBytecodeTest {
17331733
assertSameCode(instructions, expected)
17341734
}
17351735
}
1736+
1737+
@Test def newInPrefixesOfDefaultParam = {
1738+
val source =
1739+
s"""class A:
1740+
| def f(x: Int = 1): Int = x
1741+
|
1742+
|class Test:
1743+
| def meth1() = (new A).f()
1744+
| def meth2() = { val a = new A; a.f() }
1745+
""".stripMargin
1746+
1747+
checkBCode(source) { dir =>
1748+
val clsIn = dir.lookupName("Test.class", directory = false).input
1749+
val clsNode = loadClassNode(clsIn)
1750+
val meth1 = getMethod(clsNode, "meth1")
1751+
val meth2 = getMethod(clsNode, "meth2")
1752+
1753+
val instructions1 = instructionsFromMethod(meth1)
1754+
val instructions2 = instructionsFromMethod(meth2)
1755+
1756+
assert(instructions1 == instructions2,
1757+
"`assert` was not properly inlined in `meth1`\n" +
1758+
diffInstructions(instructions1, instructions2))
1759+
}
1760+
}
1761+
1762+
@Test def newInDependentOfDefaultParam = {
1763+
val source =
1764+
s"""class A:
1765+
| def i: Int = 1
1766+
|
1767+
|class Test:
1768+
| def f(a: A)(x: Int = a.i): Int = x
1769+
| def meth1() = f(new A)()
1770+
| def meth2() = { val a = new A; f(a)() }
1771+
""".stripMargin
1772+
1773+
checkBCode(source) { dir =>
1774+
val clsIn = dir.lookupName("Test.class", directory = false).input
1775+
val clsNode = loadClassNode(clsIn)
1776+
val meth1 = getMethod(clsNode, "meth1")
1777+
val meth2 = getMethod(clsNode, "meth2")
1778+
1779+
val instructions1 = instructionsFromMethod(meth1)
1780+
val instructions2 = instructionsFromMethod(meth2)
1781+
1782+
assert(instructions1 == instructions2,
1783+
"`assert` was not properly inlined in `meth1`\n" +
1784+
diffInstructions(instructions1, instructions2))
1785+
}
1786+
}
1787+
17361788
}
17371789

17381790
object invocationReceiversTestCode {

tests/run/i15315.scala

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class A:
2+
def f(x: Int = 1): Int = x
3+
4+
@main def Test() =
5+
(new A{}).f()

0 commit comments

Comments
 (0)