10000 Cleanups after code review · retronym/scala@131402f · GitHub
[go: up one dir, main page]

Skip to content

Commit 131402f

Browse files
committed
Cleanups after code review
- Remove unused references to "addTargetMethods" - Require that `targetMethodMap` is provided
1 parent 498a2ce commit 131402f

File tree

6 files changed

+68
-56
lines changed

6 files changed

+68
-56
lines changed

src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,13 @@ abstract class BTypes {
124124
*/
125125
val indyLambdaImplMethods: mutable.AnyRefMap[InternalName, mutable.LinkedHashSet[asm.Handle]] = recordPerRunCache(mutable.AnyRefMap())
126126
def addIndyLambdaImplMethod(hostClass: InternalName, handle: Seq[asm.Handle]): Unit = {
127-
indyLambdaImplMethods.getOrElseUpdate(hostClass, mutable.LinkedHashSet()) ++= handle
127+
if (handle.nonEmpty)
128+
indyLambdaImplMethods.getOrElseUpdate(hostClass, mutable.LinkedHashSet()) ++= handle
128129
}
129-
def getIndyLambdaImplMethods(hostClass: InternalName): List[asm.Handle] = {
130+
def getIndyLambdaImplMethods(hostClass: InternalName): Iterable[asm.Handle] = {
130131
indyLambdaImplMethods.getOrNull(hostClass) match {
131132
case null => Nil
132-
case xs => xs.toList.distinct
133+
case xs => xs
133134
}
134135
}
135136

src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -289,19 +289,6 @@ class CoreBTypes[BTFS <: BTypesFromSymbols[_ <: Global]](val bTypes: BTFS) {
289289
coreBTypes.jliCallSiteRef
290290
).descriptor,
291291
/* itf = */ coreBTypes.srLambdaDeserialize.isInterface.get)
292-
lazy val lambdaDeserializeAddTargets =
293-
new scala.tools.asm.Handle(scala.tools.asm.Opcodes.H_INVOKESTATIC,
294-
coreBTypes.srLambdaDeserialize.internalName, "bootstrapAddTargets",
295-
MethodBType(
296-
List(
297-
coreBTypes.jliMethodHandlesLookupRef,
298-
coreBTypes.StringRef,
299-
coreBTypes.jliMethodTypeRef,
300-
ArrayBType(coreBTypes.jliMethodHandleRef)
301-
),
302-
coreBTypes.jliCallSiteRef
303-
).descriptor,
304-
/* itf = */ coreBTypes.srLambdaDeserialize.isInterface.get)
305292
}
306293

307294
/**

src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
7676
* host a static field in the enclosing class. This allows us to add this method to interfaces
7777
* that define lambdas in default methods.
7878
*/
79-
def addLambdaDeserialize(classNode: ClassNode, implMethods: List[Handle]): Unit = {
79+
def addLambdaDeserialize(classNode: ClassNode, implMethods: Iterable[Handle]): Unit = {
8080
val cw = classNode
8181

8282
// Make sure to reference the ClassBTypes of all types that are used in the code generated
@@ -87,13 +87,12 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
8787

8888
val nilLookupDesc = MethodBType(Nil, jliMethodHandlesLookupRef).descriptor
8989
val serlamObjDesc = MethodBType(jliSerializedLambdaRef :: Nil, ObjectRef).descriptor
90-
val addTargetMethodsObjDesc = MethodBType(ObjectRef :: Nil, UNIT).descriptor
9190

9291
{
9392
val mv = cw.visitMethod(ACC_PRIVATE + ACC_STATIC + ACC_SYNTHETIC, "$deserializeLambda$", serlamObjDesc, null, null)
9493
mv.visitCode()
9594
mv.visitVarInsn(ALOAD, 0)
96-
mv.visitInvokeDynamicInsn("lambdaDeserialize", serlamObjDesc, lambdaDeserializeBootstrapHandle, implMethods: _*)
95+
mv.visitInvokeDynamicInsn("lambdaDeserialize", serlamObjDesc, lambdaDeserializeBootstrapHandle, implMethods.toArray: _*)
9796
mv.visitInsn(ARETURN)
9897
mv.visitEnd()
9998
}
@@ -102,8 +101,8 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
102101
/**
103102
* Clone the instructions in `methodNode` into a new [[InsnList]], mapping labels according to
104103
* the `labelMap`. Returns the new instruction list and a map from old to new instructions, and
105-
* a boolean indicating if the instruction list contains an instantiation of a serializable SAM
106-
* type.
104+
* a list of lambda implementation methods references by invokedynamic[LambdaMetafactory] for a
105+
* serializable SAM types.
107106
*/
108107
def cloneInstructions(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode], keepLineNumbers: Boolean): (InsnList, Map[AbstractInsnNode, AbstractInsnNode], List[Handle]) = {
109108
val javaLabelMap = labelMap.asJava

src/library/scala/runtime/LambdaDeserialize.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,10 @@
22

33

44
import java.lang.invoke.*;
5-
import java.util.ArrayList;
6-
import java.util.Arrays;
7-
import java.util.Collections;
85
import java.util.HashMap;
96

107
public final class LambdaDeserialize {
118
public static final MethodType DESERIALIZE_LAMBDA_MT = MethodType.fromMethodDescriptorString("(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object;", LambdaDeserialize.class.getClassLoader());
12-
public static final MethodType ADD_TARGET_METHODS_MT = MethodType.fromMethodDescriptorString("([Ljava/lang/invoke/MethodHandle;)V", LambdaDeserialize.class.getClassLoader());
139

1410
private MethodHandles.Lookup lookup;
1511
private final HashMap<String, MethodHandle> cache = new HashMap<>();
@@ -37,6 +33,6 @@ public static CallSite bootstrap(MethodHandles.Lookup lookup, String invokedName
3733
return new ConstantCallSite(exact);
3834
}
3935
public static String nameAndDescriptorKey(String name, String descriptor) {
40-
return name + " " + descriptor;
36+
return name + descriptor;
4137
}
4238
}

src/library/scala/runtime/LambdaDeserializer.scala

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ object LambdaDeserializer {
3333
*/
3434
def deserializeLambda(lookup: MethodHandles.Lookup, cache: java.util.Map[String, MethodHandle],
3535
targetMethodMap: java.util.Map[String, MethodHandle], serialized: SerializedLambda): AnyRef = {
36+
assert(targetMethodMap != null)
3637
def slashDot(name: String) = name.replaceAll("/", ".")
3738
val loader = lookup.lookupClass().getClassLoader
3839
val implClass = loader.loadClass(slashDot(serialized.getImplClass))
@@ -71,14 +72,10 @@ object LambdaDeserializer {
7172

7273
// Lookup the implementation method
7374
val implMethod: MethodHandle = try {
74-
if (targetMethodMap != null) {
75-
if (targetMethodMap.containsKey(key)) {
76-
targetMethodMap.get(key)
77-
} else {
78-
throw new IllegalArgumentException("Illegal lambda deserialization")
79-
}
75+
if (targetMethodMap.containsKey(key)) {
76+
targetMethodMap.get(key)
8077
} else {
81-
findMember(lookup, getImplMethodKind, implClass, getImplMethodName, implMethodSig)
78+
throw new IllegalArgumentException("Illegal lambda deserialization")
8279
}
8380
} catch {
8481
case e: ReflectiveOperationException => throw new IllegalArgumentException("Illegal lambda deserialization", e)
@@ -124,18 +121,4 @@ object LambdaDeserializer {
124121
// is cleaner if we uniformly add a single marker, so I'm leaving it in place.
125122
"java.io.Serializable"
126123
}
127-
128-
private def findMember(lookup: MethodHandles.Lookup, kind: Int, owner: Class[_],
129-
name: String, signature: MethodType): MethodHandle = {
130-
kind match {
131-
case MethodHandleInfo.REF_invokeStatic =>
132-
lookup.findStatic(owner, name, signature)
133-
case MethodHandleInfo.REF_newInvokeSpecial =>
134-
lookup.findConstructor(owner, signature)
135-
case MethodHandleInfo.REF_invokeVirtual | MethodHandleInfo.REF_invokeInterface =>
136-
lookup.findVirtual(owner, name, signature)
137-
case MethodHandleInfo.REF_invokeSpecial =>
138-
lookup.findSpecial(owner, name, signature, owner)
139-
}
140-
}
141124
}

test/junit/scala/runtime/LambdaDeserializerTest.java

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44
import org.junit.Test;
55

66
import java.io.Serializable;
7-
import java.lang.invoke.MethodHandle;
8-
import java.lang.invoke.MethodHandles;
9-
import java.lang.invoke.SerializedLambda;
7+
import java.lang.invoke.*;
108
import java.lang.reflect.Method;
119
import java.util.Arrays;
1210
import java.util.HashMap;
@@ -85,19 +83,20 @@ public void cachedStatic() {
8583
public void implMethodNameChanged() {
8684
F1<Boolean, String> f1 = lambdaHost.lambdaBackedByStaticImplMethod();
8785
SerializedLambda sl = writeReplace(f1);
88-
checkIllegalAccess(copySerializedLambda(sl, sl.getImplMethodName() + "___", sl.getImplMethodSignature()));
86+
checkIllegalAccess(sl, copySerializedLambda(sl, sl.getImplMethodName() + "___", sl.getImplMethodSignature()));
8987
}
9088

9189
@Test
9290
public void implMethodSignatureChanged() {
9391
F1<Boolean, String> f1 = lambdaHost.lambdaBackedByStaticImplMethod();
9492
SerializedLambda sl = writeReplace(f1);
95-
checkIllegalAccess(copySerializedLambda(sl, sl.getImplMethodName(), sl.getImplMethodSignature().replace("Boolean", "Integer")));
93+
checkIllegalAccess(sl, copySerializedLambda(sl, sl.getImplMethodName(), sl.getImplMethodSignature().replace("Boolean", "Integer")));
9694
}
9795

98-
private void checkIllegalAccess(SerializedLambda serialized) {
96+
private void checkIllegalAccess(SerializedLambda allowed, SerializedLambda requested) {
9997
try {
100-
LambdaDeserializer.deserializeLambda(MethodHandles.lookup(), null, null, serialized);
98+
HashMap<String, MethodHandle> allowedMap = createAllowedMap(LambdaHost.lookup(), allowed);
99+
LambdaDeserializer.deserializeLambda(MethodHandles.lookup(), null, allowedMap, requested);
101100
throw new AssertionError();
102101
} catch (IllegalArgumentException iae) {
103102
if (!iae.getMessage().contains("Illegal lambda deserialization")) {
@@ -123,19 +122,64 @@ private Class<?> loadClass(String className) {
123122
throw new RuntimeException(e);
124123
}
125124
}
125+
126126
private <A, B> A reconstitute(A f1) {
127127
return reconstitute(f1, null);
128128
}
129129

130130
@SuppressWarnings("unchecked")
131131
private <A, B> A reconstitute(A f1, java.util.HashMap<String, MethodHandle> cache) {
132132
try {
133-
return (A) LambdaDeserializer.deserializeLambda(LambdaHost.lookup(), cache, null, writeReplace(f1));
133+
return deserizalizeLambdaCreatingAllowedMap(f1, cache, LambdaHost.lookup());
134134
} catch (Exception e) {
135135
throw new RuntimeException(e);
136136
}
137137
}
138138

139+
private <A> A deserizalizeLambdaCreatingAllowedMap(A f1, HashMap<String, MethodHandle> cache, MethodHandles.Lookup lookup) {
140+
SerializedLambda serialized = writeReplace(f1);
141+
HashMap<String, MethodHandle> allowed = createAllowedMap(lookup, serialized);
142+
return (A) LambdaDeserializer.deserializeLambda(lookup, cache, allowed, serialized);
143+
}
144+
145+
private HashMap<String, MethodHandle> createAllowedMap(MethodHandles.Lookup lookup, SerializedLambda serialized) {
146+
Class<?> implClass = classForName(serialized.getImplClass().replace("/", "."), lookup.lookupClass().getClassLoader());
147+
MethodHandle implMethod = findMember(lookup, serialized.getImplMethodKind(), implClass, serialized.getImplMethodName(), MethodType.fromMethodDescriptorString(serialized.getImplMethodSignature(), lookup.lookupClass().getClassLoader()));
148+
HashMap<String, MethodHandle> allowed = new HashMap<>();
149+
allowed.put(LambdaDeserialize.nameAndDescriptorKey(serialized.getImplMethodName(), serialized.getImplMethodSignature()), implMethod);
150+
return allowed;
151+
}
152+
153+
private Class<?> classForName(String className, ClassLoader classLoader) {
154+
try {
155+
return Class.forName(className, true, classLoader);
156+
} catch (ClassNotFoundException e) {
157+
throw new RuntimeException(e);
158+
}
159+
}
160+
161+
private MethodHandle findMember(MethodHandles.Lookup lookup, int kind, Class<?> owner,
162+
String name, MethodType signature) {
163+
try {
164+
switch (kind) {
165+
case MethodHandleInfo.REF_invokeStatic:
166+
return lookup.findStatic(owner, name, signature);
167+
case MethodHandleInfo.REF_newInvokeSpecial:
168+
return lookup.findConstructor(owner, signature);
169+
case MethodHandleInfo.REF_invokeVirtual:
170+
case MethodHandleInfo.REF_invokeInterface:
171+
return lookup.findVirtual(owner, name, signature);
172+
case MethodHandleInfo.REF_invokeSpecial:
173+
return lookup.findSpecial(owner, name, signature, owner);
174+
default:
175+
throw new IllegalArgumentException();
176+
}
177+
} catch (NoSuchMethodException | IllegalAccessException e) {
178+
throw new RuntimeException(e);
179+
}
180+
}
181+
182+
139183
private <A> SerializedLambda writeReplace(A f1) {
140184
try {
141185
Method writeReplace = f1.getClass().getDeclaredMethod("writeReplace");
@@ -189,5 +233,7 @@ public static MethodHandles.Lookup lookup() {
189233
}
190234

191235
interface I {
192-
default String i() { return "i"; };
236+
default String i() {
237+
return "i";
238+
}
193239
}

0 commit comments

Comments
 (0)
0