Skip to content

Commit c06dc84

Browse files
JakeWhartonJakeWharton
committed
Alter when accessor methods are generated.
For owned, non-private methods, omit the accessor since the generated class can access the method directly. For non-owned methods, generate the accessor when the method is protected since the generated class will not have access.
1 parent 365b819 commit c06dc84

File tree

11 files changed

+131
-62
lines changed

11 files changed

+131
-62
lines changed

‎end-to-end-tests/src/test/java/net/orfjackal/retrolambda/test/LambdaTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
package net.orfjackal.retrolambda.test;
66

7+
import net.orfjackal.retrolambda.test.anotherpackage.DifferentPackageBase;
78
import org.apache.commons.lang.SystemUtils;
89
import org.junit.Test;
910
import org.objectweb.asm.*;
@@ -150,6 +151,19 @@ public void method_references_to_constructors() throws Exception {
150151
assertThat(ref.call(), is(instanceOf(ArrayList.class)));
151152
}
152153

154+
@Test
155+
public void method_references_to_protected_supertype_methods() throws Exception {
156+
Callable<String> ref = new SubclassInMyPackage().thing();
157+
158+
Assert.assertThat(ref.call(), equalTo("Hello"));
159+
}
160+
161+
public static class SubclassInMyPackage extends DifferentPackageBase {
162+
public Callable<String> thing() {
163+
return DifferentPackageBase::value;
164+
}
165+
}
166+
153167
/**
154168
* Because the constructor is private, an access method must be generated for it
155169
* and also the NEW instruction must be done inside the access method.

‎end-to-end-tests/src/test/java/net/orfjackal/retrolambda/test/anotherpackage/DifferentPackageBase.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright © 2013-2016 Esko Luontola <www.orfjackal.net>
2+
// This software is released under the Apache License 2.0.
3+
// The license text is at http://www.apache.org/licenses/LICENSE-2.0
4+
5+
package net.orfjackal.retrolambda.test.anotherpackage;
6+
7+
public class DifferentPackageBase {
8+
protected static String value() {
9+
return "Hello";
10+
}
11+
}

‎retrolambda/src/main/java/net/orfjackal/retrolambda/Retrolambda.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public static void run(Config config) throws Throwable {
3838

3939
Thread.currentThread().setContextClassLoader(new NonDelegatingClassLoader(asUrls(classpath)));
4040

41-
ClassHierarchyAnalyzer analyzer = new ClassHierarchyAnalyzer();
41+
ClassAnalyzer analyzer = new ClassAnalyzer();
4242
OutputDirectory outputDirectory = new OutputDirectory(outputDir);
4343
Transformers transformers = new Transformers(bytecodeVersion, defaultMethodsEnabled, analyzer);
4444
LambdaClassSaver lambdaClassSaver = new LambdaClassSaver(outputDirectory, transformers);

‎retrolambda/src/main/java/net/orfjackal/retrolambda/Transformers.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ public class Transformers {
1717

1818
private final int targetVersion;
1919
private final boolean defaultMethodsEnabled;
20-
private final ClassHierarchyAnalyzer analyzer;
20+
private final ClassAnalyzer analyzer;
2121

22-
public Transformers(int targetVersion, boolean defaultMethodsEnabled, ClassHierarchyAnalyzer analyzer) {
22+
public Transformers(int targetVersion, boolean defaultMethodsEnabled, ClassAnalyzer analyzer) {
2323
this.targetVersion = targetVersion;
2424
this.defaultMethodsEnabled = defaultMethodsEnabled;
2525
this.analyzer = analyzer;
@@ -48,7 +48,7 @@ public byte[] backportClass(ClassReader reader) {
4848
next = new UpdateRelocatedMethodInvocations(next, analyzer);
4949
next = new AddMethodDefaultImplementations(next, analyzer);
5050
}
51-
next = new BackportLambdaInvocations(next);
51+
next = new BackportLambdaInvocations(next, analyzer);
5252
return next;
5353
});
5454
}
@@ -59,7 +59,7 @@ public List<byte[]> backportInterface(ClassReader reader) {
5959
// the wrong one of them is written to disk last.
6060
ClassNode lambdasBackported = new ClassNode();
6161
ClassVisitor next = lambdasBackported;
62-
next = new BackportLambdaInvocations(next);
62+
next = new BackportLambdaInvocations(next, analyzer);
6363
reader.accept(next, 0);
6464

6565
List<byte[]> results = new ArrayList<>();

‎retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/AddMethodDefaultImplementations.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@
1111

1212
public class AddMethodDefaultImplementations extends ClassVisitor {
1313

14-
private final ClassHierarchyAnalyzer analyzer;
14+
private final ClassAnalyzer analyzer;
1515
private String className;
1616

17-
public AddMethodDefaultImplementations(ClassVisitor next, ClassHierarchyAnalyzer analyzer) {
17+
public AddMethodDefaultImplementations(ClassVisitor next, ClassAnalyzer analyzer) {
1818
super(ASM5, next);
1919
this.analyzer = analyzer;
2020
}

‎retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassHierarchyAnalyzer.java renamed to ‎retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassAnalyzer.java

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import static net.orfjackal.retrolambda.util.Flags.*;
1515
import static org.objectweb.asm.Opcodes.*;
1616

17-
public class ClassHierarchyAnalyzer {
17+
public class ClassAnalyzer {
1818

1919
private final Map<Type, ClassInfo> classes = new HashMap<>();
2020
private final Map<MethodRef, MethodRef> relocatedMethods = new HashMap<>();
@@ -45,10 +45,16 @@ public void visit(int version, int access, String name, String signature, String
4545

4646
@Override
4747
public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
48-
if (isConstructor(name) || isStaticMethod(access)) {
49-
return null;
48+
int tag;
49+
if (isConstructor(name)) {
50+
tag = H_INVOKESPECIAL;
51+
} else if (isStaticMethod(access)) {
52+
tag = H_INVOKESTATIC;
53+
} else {
54+
tag = H_INVOKEVIRTUAL;
5055
}
51-
c.addMethod(new MethodRef(H_INVOKEVIRTUAL, owner, name, desc), new MethodKind.Implemented());
56+
57+
c.addMethod(access, new MethodRef(tag, owner, name, desc), new MethodKind.Implemented());
5258
return null;
5359
}
5460

@@ -71,12 +77,12 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si
7177
MethodRef method = new MethodRef(Handles.accessToTag(access, true), owner, name, desc);
7278

7379
if (isAbstractMethod(access)) {
74-
c.addMethod(method, new MethodKind.Abstract());
80+
c.addMethod(access, method, new MethodKind.Abstract());
7581

7682
} else if (isDefaultMethod(access)) {
7783
MethodRef defaultImpl = new MethodRef(H_INVOKESTATIC, companion, name, Bytecode.prependArgumentType(desc, Type.getObjectType(owner)));
7884
c.enableCompanionClass();
79-
c.addMethod(method, new MethodKind.Default(defaultImpl));
85+
c.addMethod(access, method, new MethodKind.Default(defaultImpl));
8086

8187
} else if (isInstanceLambdaImplMethod(access)) {
8288
relocatedMethods.put(method, new MethodRef(H_INVOKESTATIC, companion, name, Bytecode.prependArgumentType(desc, Type.getObjectType(owner))));

‎retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassInfo.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ public List<MethodInfo> getMethods() {
4444
return Collections.unmodifiableList(methods);
4545
}
4646

47-
public void addMethod(MethodRef method, MethodKind kind) {
48-
methods.add(new MethodInfo(method.tag, method.getSignature(), Type.getObjectType(method.owner), kind));
47+
public void addMethod(int access, MethodRef method, MethodKind kind) {
48+
methods.add(new MethodInfo(access, method.tag, method.getSignature(), Type.getObjectType(method.owner), kind));
4949
}
5050

5151
public Optional<Type> getCompanionClass() {

‎retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/MethodInfo.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,19 @@
1111

1212
public class MethodInfo {
1313

14+
public final int access;
1415
public final int tag;
1516
public final MethodSignature signature;
1617
public final Type owner;
1718
public final MethodKind kind;
1819

19-
public MethodInfo(String name, String desc, Class<?> owner, MethodKind kind) { // only for tests, so we can ignore the tag
20-
this(-1, new MethodSignature(name, desc), Type.getType(owner), kind);
20+
public MethodInfo(String name, String desc, Class<?> owner, MethodKind kind) {
21+
// only for tests, so we can ignore the tag and access
22+
this(0, -1, new MethodSignature(name, desc), Type.getType(owner), kind);
2123
}
2224

23-
public MethodInfo(int tag, MethodSignature signature, Type owner, MethodKind kind) {
25+
public MethodInfo(int access, int tag, MethodSignature signature, Type owner, MethodKind kind) {
26+
this.access = access;
2427
this.tag = tag;
2528
this.signature = signature;
2629
this.owner = owner;
@@ -58,7 +61,7 @@ public String toString() {
5861
.addValue(signature)
5962
.addValue(owner)
6063
.addValue(kind)
61-
.addValue("(" + tag + ")")
64+
.addValue("(tag=" + tag + ", access=" + access + ")")
6265
.toString();
6366
}
6467
}

‎retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/UpdateRelocatedMethodInvocations.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@
1111

1212
public class UpdateRelocatedMethodInvocations extends ClassVisitor {
1313

14-
private final ClassHierarchyAnalyzer analyzer;
14+
private final ClassAnalyzer analyzer;
1515

16-
public UpdateRelocatedMethodInvocations(ClassVisitor next, ClassHierarchyAnalyzer analyzer) {
16+
public UpdateRelocatedMethodInvocations(ClassVisitor next, ClassAnalyzer analyzer) {
1717
super(ASM5, next);
1818
this.analyzer = analyzer;
1919
}

‎retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaInvocations.java

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
package net.orfjackal.retrolambda.lambdas;
66

7+
import net.orfjackal.retrolambda.interfaces.*;
78
import net.orfjackal.retrolambda.util.Bytecode;
89
import org.objectweb.asm.*;
910

@@ -18,10 +19,12 @@ public class BackportLambdaInvocations extends ClassVisitor {
1819

1920
private int classAccess;
2021
private String className;
22+
private final ClassAnalyzer analyzer;
2123
private final Map<Handle, Handle> lambdaAccessToImplMethods = new LinkedHashMap<>();
2224

23-
public BackportLambdaInvocations(ClassVisitor next) {
25+
public BackportLambdaInvocations(ClassVisitor next, ClassAnalyzer analyzer) {
2426
super(ASM5, next);
27+
this.analyzer = analyzer;
2528
}
2629

2730
@Override
@@ -56,20 +59,52 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si
5659

5760
Handle getLambdaAccessMethod(Handle implMethod) {
5861
if (!implMethod.getOwner().equals(className)) {
59-
return implMethod;
60-
}
61-
if (isInterface(classAccess)) {
62-
// the method will be relocated to a companion class
63-
return implMethod;
62+
if (isNonOwnedMethodVisible(implMethod)) {
63+
return implMethod;
64+
}
65+
} else {
66+
if (isInterface(classAccess)) {
67+
// the method will be relocated to a companion class
68+
return implMethod;
69+
}
70+
if (isOwnedMethodVisible(implMethod)) {
71+
// The method is visible to the companion class and therefore doesn't need an accessor.
72+
return implMethod;
73+
}
6474
}
65-
// TODO: do not generate an access method if the impl method is not private (probably not implementable with a single pass)
6675
String name = "access$lambda$" + lambdaAccessToImplMethods.size();
6776
String desc = getLambdaAccessMethodDesc(implMethod);
6877
Handle accessMethod = new Handle(H_INVOKESTATIC, className, name, desc);
6978
lambdaAccessToImplMethods.put(accessMethod, implMethod);
7079
return accessMethod;
7180
}
7281

82+
private boolean isOwnedMethodVisible(Handle implMethod) {
83+
MethodSignature implSignature = new MethodSignature(implMethod.getName(), implMethod.getDesc());
84+
85+
Collection<MethodInfo> methods = analyzer.getMethods(Type.getObjectType(implMethod.getOwner()));
86+
for (MethodInfo method : methods) {
87+
if (method.signature.equals(implSignature)) {
88+
// The method will be visible to the companion class if the private flag is absent.
89+
return (method.access & ACC_PRIVATE) == 0;
90+
}
91+
}
92+
throw new IllegalStateException("Non-analyzed method " + implMethod + ". Report this as a bug.");
93+
}
94+
95+
private boolean isNonOwnedMethodVisible(Handle implMethod) {
96+
MethodSignature implSignature = new MethodSignature(implMethod.getName(), implMethod.getDesc());
97+
98+
Collection<MethodInfo> methods = analyzer.getMethods(Type.getObjectType(implMethod.getOwner()));
99+
for (MethodInfo method : methods) {
100+
if (method.signature.equals(implSignature)) {
101+
// The method will be visible to the companion class if the protected flag is absent.
102+
return (method.access & ACC_PROTECTED) == 0;
103+
}
104+
}
105+
return true;
106+
}
107+
73108
private String getLambdaAccessMethodDesc(Handle implMethod) {
74109
if (implMethod.getTag() == H_INVOKESTATIC) {
75110
// static method call -> keep as-is

0 commit comments

Comments
 (0)