Skip to content

Commit a118ecd

Browse files
committed
WIP: Add prototype improving default args encoding
The following changes implement a more flexible encoding of default args that allows default implementation to depend on parameters defined on the left (either previous parameters lists or parameters in the same parameter list defined on the left). This encoding breaks binary compatibility so it targets 2.13. It consists of changes in the synthetic method generation of defaults and the call sites of these methods. The implementation of synthetic methods now depends on any parameter defined at the left of a default arg, even if it their body does not use these parameters. This is done so that changing the body of the default args does not change the signature and hence break binary compatibility. More explanation on this implementation can be found in the [SIP proposal](scala/docs.scala-lang#653). WIP: This commit still needs more work. This is what's missing: 1. Reusing the results of default args. Currently, invocations to default methods are inlined at the call-site and a lot of invocations can be reused. Declaring more than one default arg considerably increases bytecode size. My idea to tackle this issue is to save the results of the invocation in vals before the the call-site invocation. Example: ``` // definition def foo(a: Int, b: Int = a, c: Int = a + b): Int = a + b + c // invocation foo(1) ``` will modify the call-site like: ``` foo(1, foo$default$2(1), foo$default$3(1, foo$default$2(1))) ``` As you see, invocations to 1 and `foo$default$2` could be optimized: ``` val a$default = 1 val b$default = foo$default$2(a$default) val c$default = foo$default$3(a$default, b$default) foo(a$default, b$default, c$default) ``` This generates more bytecode for call-sites omitting default arguments, but avoids unnecessary calls that could cause bad performance (especially if more than 2 default args are defined in a method). 2. Generate more tests for the default arguments in class.
1 parent 827d69d commit a118ecd

File tree

4 files changed

+121
-5
lines changed

4 files changed

+121
-5
lines changed

src/compiler/scala/tools/nsc/typechecker/Namers.scala

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,14 +1388,24 @@ trait Namers extends MethodSynthesis {
13881388
var ownerNamer: Option[Namer] = None
13891389
var moduleNamer: Option[(ClassDef, Namer)] = None
13901390
var posCounter = 1
1391+
var previousParamsCounter = 0
13911392

13921393
// For each value parameter, create the getter method if it has a
13931394
// default argument. previous denotes the parameter lists which
13941395
// are on the left side of the current one. These get added to the
13951396
// default getter. Example:
13961397
//
1397-
// def foo(a: Int)(b: Int = a) becomes
1398-
// foo$default$1(a: Int) = a
1398+
// def foo(a: Int, b: Int = a)(c: Int = a + b) becomes
1399+
// foo$default$1(a: Int) = a // for b
1400+
// foo$default$2(a: Int, b: Int) = a + b // for c
1401+
//
1402+
// This scheme allows a default argument on the right to depend
1403+
// on every argument on its left and applies for arguments both
1404+
// in the same parameter list and in previous curried parameter lists.
1405+
// Note that even if the parameters on the left are not used in the
1406+
// implementation of a default arg, the parameters are encoded in
1407+
// the signature of the synthetic method so that future changes
1408+
// in the implementation of default args does not break binary compat.
13991409
//
14001410
vparamss.foldLeft(Nil: List[List[ValDef]]) { (previous, vparams) =>
14011411
assert(!overrides || vparams.length == baseParamss.head.length, ""+ meth.fullName + ", "+ overridden.fullName)
@@ -1427,7 +1437,8 @@ trait Namers extends MethodSynthesis {
14271437
val name = nme.defaultGetterName(meth.name, posCounter)
14281438

14291439
var defTparams = rtparams
1430-
val defVparamss = mmap(rvparamss.take(previous.length)){ rvp =>
1440+
val leftRvparams = rvparams.take(posCounter - previousParamsCounter - 1)
1441+
val leftRvparamss = mmap(rvparamss.take(previous.length) ++ List(leftRvparams)){ rvp =>
14311442
copyValDef(rvp)(mods = rvp.mods &~ DEFAULTPARAM, rhs = EmptyTree)
14321443
}
14331444

@@ -1485,7 +1496,7 @@ trait Namers extends MethodSynthesis {
14851496
val defRhs = rvparam.rhs
14861497

14871498
val defaultTree = atPos(vparam.pos.focus) {
1488-
DefDef(Modifiers(paramFlagsToDefaultGetter(meth.flags), ddef.mods.privateWithin) | oflag, name, defTparams, defVparamss, defTpt, defRhs)
1499+
DefDef(Modifiers(paramFlagsToDefaultGetter(meth.flags), ddef.mods.privateWithin) | oflag, name, defTparams, leftRvparamss, defTpt, defRhs)
14891500
}
14901501
if (!isConstr)
14911502
methOwner.resetFlag(INTERFACE) // there's a concrete member now
@@ -1502,6 +1513,7 @@ trait Namers extends MethodSynthesis {
15021513
if (overrides) baseParams = baseParams.tail
15031514
})
15041515
if (overrides) baseParamss = baseParamss.tail
1516+
previousParamsCounter += vparams.length
15051517
previous :+ vparams
15061518
}
15071519
}

src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,7 @@ trait NamesDefaults { self: Analyzer =>
429429
if (givenArgs.length < params.length) {
430430
val (missing, positional) = missingParams(givenArgs, params, nameOfNamedArg)
431431
if (missing forall (_.hasDefault)) {
432+
val generatedDefaults = mutable.HashMap[Symbol, Tree]()
432433
val defaultArgs = missing flatMap (p => {
433434
val defGetter = defaultGetter(p, context)
434435
// TODO #3649 can create spurious errors when companion object is gone (because it becomes unlinked from scope)
@@ -441,8 +442,23 @@ trait NamesDefaults { self: Analyzer =>
441442
}
442443
default1 = if (targs.isEmpty) default1
443444
else TypeApply(default1, targs.map(_.duplicate))
444-
val default2 = (default1 /: previousArgss)((tree, args) =>
445+
446+
var valuesArgs = givenArgs
447+
val leftArgsInParamList = params.take(params.indexOf(p))
448+
val valuesArgsOnLeft = leftArgsInParamList.map { arg =>
449+
generatedDefaults.get(arg) match {
450+
case Some(default) => default
451+
case None =>
452+
val value = valuesArgs.head
453+
valuesArgs = givenArgs.tail
454+
value
455+
}
456+
}
457+
val allLeftArgs: List[List[Tree]] = previousArgss ++ List(valuesArgsOnLeft)
458+
val default2 = (default1 /: allLeftArgs)((tree, args) =>
445459
Apply(tree, args.map(_.duplicate)))
460+
461+
generatedDefaults += (p-> default2)
446462
Some(atPos(pos) {
447463
if (positional) default2
448464
else AssignOrNamedArg(Ident(p.name), default2)
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
class A(a: Int, val b: Int = a)
2+
3+
object Test extends App {
4+
val a = new A(1)
5+
val a2 = new A(1, 1)
6+
assert(a.b == a2.b)
7+
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
object Foo {
2+
// Currently works
3+
def substring(s: String, start: Int = 0)(end: Int = s.length) =
4+
s.substring(start, end)
5+
6+
// SIP proposes to make these invocations valid
7+
def foo(a: Int, b: Int = a) = (a, b)
8+
def foo2(a: Int, b: Int = a, c: Int = a + b) = (a, b, c)
9+
def foo3(a: Int, b: Int = a)(c: Int = a + b) = (a, b, c)
10+
def foo4(a: Int, b: Int = a, c: Int = a + b) = (a, b, c)
11+
def foo5(a: Int = 10, b: Int = a, c: Int = a + b) = (a, b, c)
12+
def foo6(a: String, b: Int = a.toInt, c: Int = a.toInt + b) = (a.toInt, b, c)
13+
14+
def fooHK[T](a: T, b: T = a) = (a, b)
15+
def fooHK2[T](a: T, b: T = a)(c: T = a) = (a, b, c)
16+
def bind(name: String, boundType: String, value: Any, modifiers: List[String] = Nil): List[String] = modifiers
17+
}
18+
19+
object Test extends App {
20+
Foo.substring("")()
21+
22+
val call0 = Foo.foo(1)
23+
val call02 = Foo.foo(1, 1)
24+
assert(call0 == call02)
25+
26+
val a = 1
27+
val call1 = Foo.foo(a)
28+
val call12 = Foo.foo(a, 1)
29+
assert(call1 == call12)
30+
31+
val call2 = Foo.foo2(1)
32+
val call22 = Foo.foo2(1, 1)
33+
val call23 = Foo.foo2(1, 1, 2)
34+
assert(call2 == call22)
35+
assert(call2 == call23)
36+
37+
val call3 = Foo.foo2(a)
38+
val call32 = Foo.foo2(a, 1)
39+
val call33 = Foo.foo2(a, 1, 2)
40+
assert(call3 == call32)
41+
assert(call3 == call33)
42+
43+
val call4 = Foo.foo3(1)()
44+
val call42 = Foo.foo3(1, 1)()
45+
val call43 = Foo.foo3(1, 1)(2)
46+
assert(call4 == call42)
47+
assert(call4 == call43)
48+
49+
val call5 = Foo.foo3(a)()
50+
val call52 = Foo.foo3(a, 1)()
51+
val call53 = Foo.foo3(a, 1)(2)
52+
assert(call5 == call52)
53+
assert(call5 == call53)
54+
55+
val bar = 4
56+
val call6 = Foo.foo4(a, c = bar)
57+
assert(call6 == (a, a, bar), s"$call6 != ($a,$a,$bar)")
58+
59+
val call7 = Foo.foo4(a, b = bar)
60+
assert(call7 == (a, bar, 5), s"$call7 != ($a,$bar,5)")
61+
62+
val call8 = Foo.foo5()
63+
val call82 = Foo.foo5(c = 3)
64+
assert(call82 == (10, 10, 3), s"$call82 != (10, 10, 3)")
65+
66+
val call9 = Foo.foo6("1")
67+
assert(call9._1.intValue() == 1, s"${call9._1} != 1")
68+
assert(call9._2.intValue() == 1, s"${call9._2} != 1")
69+
assert(call9._3.intValue() == 2, s"${call9._3} != 2")
70+
71+
val asdf = "asdfasdf"
72+
val call100 = Foo.fooHK(asdf)
73+
assert(call100 == (asdf,asdf), s"$call100 != ($asdf,$asdf)")
74+
75+
val call110 = Foo.fooHK2(asdf)()
76+
assert(call110 == (asdf,asdf,asdf), s"$call110 != ($asdf,$asdf,$asdf)")
77+
78+
val value = ""
79+
val call120 = Foo.bind("", value.asInstanceOf[AnyRef].getClass.getName, value)
80+
assert(call120 == Nil, s"$call120 != Nil")
81+
}

0 commit comments

Comments
 (0)