Skip to content

Commit c0de3e7

Browse files
committed
SI-8359 Adjust parameter order of accessor method in Delambdafy
Under `-Ydelambdafy:method`, a public, static accessor method is created to expose the private method containing the body of the lambda. Currently this accessor method has its parameters in the same order structure as those of the lambda body method. What is this order? There are three categories of parameters: 1. lambda parameters 2. captured parameters (added by lambdalift) 3. self parameters (added to lambda bodies that end up in trait impl classes by mixin, and added unconditionally to the static accessor method.) These are currently emitted in order #3, #1, #2. Here are examples of the current behaviour: BEFORE (trait): ``` % cat sandbox/test.scala && scalac-hash v2.11.5 -Ydelambdafy:method sandbox/test.scala && javap -private -classpath . 'Test$class' trait Member; class Capture; trait LambdaParam trait Test { def member: Member def foo { val local = new Capture (arg: LambdaParam) => "" + arg + member + local } } Compiled from "test.scala" public abstract class Test$class { public static void foo(Test); private static final java.lang.String $anonfun$1(Test, LambdaParam, Capture); public static void $init$(Test); public static final java.lang.String accessor$1(Test, LambdaParam, Capture); } ``` BEFORE (class): ``` % cat sandbox/test.scala && scalac-hash v2.11.5 -Ydelambdafy:method sandbox/test.scala && javap -private -classpath . Test trait Member; class Capture; trait LambdaParam abstract class Test { def member: Member def foo { val local = new Capture (arg: LambdaParam) => "" + arg + member + local } } Compiled from "test.scala" public abstract class Test { public abstract Member member(); public void foo(); private final java.lang.String $anonfun$1(LambdaParam, Capture); public Test(); public static final java.lang.String accessor$1(Test, LambdaParam, Capture); } ``` Contrasting the class case with Java: ``` % cat sandbox/Test.java && javac -d . sandbox/Test.java && javap -private -classpath . Test public abstract class Test { public static class Member {}; public static class Capture {}; public static class LambaParam {}; public static interface I { public abstract Object c(LambaParam arg); } public abstract Member member(); public void test() { Capture local = new Capture(); I i1 = (LambaParam arg) -> "" + member() + local; } } Compiled from "Test.java" public abstract class Test { public Test(); public abstract Test$Member member(); public void test(); private java.lang.Object lambda$test$0(Test$Capture, Test$LambaParam); } ``` We can see that in Java 8 lambda parameters come after captures. If we want to use Java's LambdaMetafactory to spin up our anoymous FunctionN subclasses on the fly, our ordering must change. I can see three options for change: 1. Adjust `LambdaLift` to always prepend captured parameters, rather than appending them. I think we could leave `Mixin` as it is, it already prepends the self parameter. This would result a parameter ordering, in terms of the list above: #3, #2, #1. 2. More conservatively, do this just for methods known to hold lambda bodies. This might avoid needlessly breaking code that has come to depend on our binary encoding. 3. Adjust the parameters of the accessor method only. The body of this method can permute params before calling the lambda body method. This commit implements option #3. This is the lowest risk / impact to pave the way for experimentation with indy lambdas. But it isn't ideal as a long term solution, as indy lambdas actually don't need the accessor method at all, private methods can be used directly by LambdaMetaFactory, saving a little indirection. Option #1 might be worth a shot on the 2.12.x branch. Option #2 might even be feasible on 2.11.x. I've included a test that shows this in all in action. However, that is currently disabled, as we don't have a partest category for tests that require Java 8.
1 parent d14e065 commit c0de3e7

File tree

2 files changed

+74
-5
lines changed

2 files changed

+74
-5
lines changed

src/compiler/scala/tools/nsc/transform/Delambdafy.scala

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,20 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
140140
if (!thisProxy.exists) {
141141
target setFlag STATIC
142142
}
143-
val params = ((optionSymbol(thisProxy) map {proxy:Symbol => ValDef(proxy)}) ++ (target.paramss.flatten map ValDef.apply)).toList
143+
val targetParams = target.paramss.flatten map (param => ValDef(param))
144+
val arity = fun.vparams.length
145+
val numParamsBeforeMixin = enteringMixin(target.info.params.length)
146+
val numParams = target.info.params.length
147+
val numCapturedParams = numParamsBeforeMixin - arity
148+
val hasSelfParam = {
149+
val numSelfParams = numParams - arity - numCapturedParams
150+
assert((0 to 1) contains numSelfParams, target.info)
151+
numSelfParams > 0
152+
}
153+
val (selfParam, nonSelfParams) = targetParams.splitAt(if (hasSelfParam) 1 else 0)
154+
val (lambdaParams, captureParams) = nonSelfParams.splitAt(arity)
155+
val thisProxyParam: Option[ValDef] = optionSymbol(thisProxy) map {proxy:Symbol => ValDef(proxy)}
156+
val params = thisProxyParam.toList ++ selfParam ++ captureParams ++ lambdaParams
144157

145158
val methSym = oldClass.newMethod(unit.freshTermName(nme.accessor.toString() + "$"), target.pos, FINAL | BRIDGE | SYNTHETIC | PROTECTED | STATIC)
146159

@@ -155,9 +168,13 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
155168
oldClass.info.decls enter methSym
156169

157170
val body = localTyper.typed {
158-
val newTarget = Select(if (thisProxy.exists) gen.mkAttributedRef(paramSyms(0)) else gen.mkAttributedThis(oldClass), target)
159-
val newParams = paramSyms drop (if (thisProxy.exists) 1 else 0) map Ident
160-
Apply(newTarget, newParams)
171+
val qual = thisProxyParam match {
172+
case Some(t) => gen.mkAttributedRef(t.symbol)
173+
case _ => gen.mkAttributedThis(oldClass)
174+
}
175+
val newTarget = Select(qual, target)
176+
val args = (selfParam ++ lambdaParams ++ captureParams).map(t => Ident(t.symbol))
177+
Apply(newTarget, args)
161178
} setPos fun.pos
162179
val methDef = DefDef(methSym, List(params), body)
163180

@@ -186,7 +203,9 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
186203

187204
newClass.info.decls enter methSym
188205

189-
val Apply(_, oldParams) = fun.body
206+
val (selfParam, nonSelfParams) = fun.body.asInstanceOf[Apply].args.splitAt(if (accessor.symbol.owner.isTrait) 1 else 0)
207+
val (lambdaParams, capturedParams) = nonSelfParams.splitAt(fun.vparams.length)
208+
val oldParams = selfParam ++ capturedParams ++ lambdaParams
190209

191210
val body = localTyper typed Apply(Select(gen.mkAttributedThis(oldClass), accessor.symbol), (optionSymbol(thisProxy) map {tp => Select(gen.mkAttributedThis(newClass), tp)}).toList ++ oldParams)
192211
body.substituteSymbols(fun.vparams map (_.symbol), params map (_.symbol))
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
//
2+
// Tests that the static accessor method for lambda bodies
3+
// (generated under -Ydelambdafy:method) are compatible with
4+
// Java 8's LambdaMetafactory.
5+
//
6+
import java.lang.invoke._
7+
8+
class C {
9+
def test1: Unit = {
10+
(x: String) => x.reverse
11+
}
12+
def test2: Unit = {
13+
val capture1 = "capture1"
14+
(x: String) => capture1 + " " + x.reverse
15+
}
16+
def test3: Unit = {
17+
(x: String) => C.this + " " + x.reverse
18+
}
19+
}
20+
trait T {
21+
def test4: Unit = {
22+
(x: String) => x.reverse
23+
}
24+
}
25+
26+
// A functional interface. Function1 contains abstract methods that are filled in by mixin
27+
trait Function1ish[A, B] {
28+
def apply(a: A): B
29+
}
30+
31+
object Test {
32+
def lambdaFactory[A, B](hostClass: Class[_], instantiatedParam: Class[A], instantiatedRet: Class[B], accessorName: String,
33+
capturedParams: Array[(Class[_], AnyRef)] = Array()) = {
34+
val caller = MethodHandles.lookup
35+
val methodType = MethodType.methodType(classOf[AnyRef], Array[Class[_]](classOf[AnyRef]))
36+
val instantiatedMethodType = MethodType.methodType(instantiatedRet, Array[Class[_]](instantiatedParam))
37+
val (capturedParamTypes, captured) = capturedParams.unzip
38+
val targetMethodType = MethodType.methodType(instantiatedRet, capturedParamTypes :+ instantiatedParam)
39+
val invokedType = MethodType.methodType(classOf[Function1ish[_, _]], capturedParamTypes)
40+
val target = caller.findStatic(hostClass, accessorName, targetMethodType)
41+
val site = LambdaMetafactory.metafactory(caller, "apply", invokedType, methodType, target, instantiatedMethodType)
42+
site.getTarget.invokeWithArguments(captured: _*).asInstanceOf[Function1ish[A, B]]
43+
}
44+
def main(args: Array[String]) {
45+
println(lambdaFactory(classOf[C], classOf[String], classOf[String], "accessor$1").apply("abc"))
46+
println(lambdaFactory(classOf[C], classOf[String], classOf[String], "accessor$2", Array(classOf[String] -> "capture1")).apply("abc"))
47+
println(lambdaFactory(classOf[C], classOf[String], classOf[String], "accessor$3", Array(classOf[C] -> new C)).apply("abc"))
48+
println(lambdaFactory(Class.forName("T$class"), classOf[String], classOf[String], "accessor$4", Array(classOf[T] -> new T{})).apply("abc"))
49+
}
50+
}

0 commit comments

Comments
 (0)