Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alter when accessor methods are generated. #84

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package net.orfjackal.retrolambda.test;

import net.orfjackal.retrolambda.test.anotherpackage.DifferentPackageBase;
import org.apache.commons.lang.SystemUtils;
import org.junit.Test;
import org.objectweb.asm.*;
Expand Down Expand Up @@ -150,6 +151,27 @@ public void method_references_to_constructors() throws Exception {
assertThat(ref.call(), is(instanceOf(ArrayList.class)));
}

@Test
public void method_references_to_protected_supertype_methods() throws Exception {
Callable<String> ref1 = new SubclassInMyPackage().thing();
assertThat(ref1.call(), equalTo("Hello"));

Callable<String> ref2 = new SubclassInSamePackage().thing();
assertThat(ref2.call(), equalTo("Hello"));
}

public static class SubclassInMyPackage extends DifferentPackageBase {
public Callable<String> thing() {
return DifferentPackageBase::value;
}
}

public static class SubclassInSamePackage extends SamePackageBase {
public Callable<String> thing() {
return SamePackageBase::value;
}
}

/**
* Because the constructor is private, an access method must be generated for it
* and also the NEW instruction must be done inside the access method.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright © 2013-2016 Esko Luontola <www.orfjackal.net>
// This software is released under the Apache License 2.0.
// The license text is at http://www.apache.org/licenses/LICENSE-2.0

package net.orfjackal.retrolambda.test;

public class SamePackageBase {
protected static String value() {
return "Hello";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright © 2013-2016 Esko Luontola <www.orfjackal.net>
// This software is released under the Apache License 2.0.
// The license text is at http://www.apache.org/licenses/LICENSE-2.0

package net.orfjackal.retrolambda.test.anotherpackage;

public class DifferentPackageBase {
protected static String value() {
return "Hello";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static void run(Config config) throws Throwable {

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

ClassHierarchyAnalyzer analyzer = new ClassHierarchyAnalyzer();
ClassAnalyzer analyzer = new ClassAnalyzer();
OutputDirectory outputDirectory = new OutputDirectory(outputDir);
Transformers transformers = new Transformers(bytecodeVersion, defaultMethodsEnabled, analyzer);
LambdaClassSaver lambdaClassSaver = new LambdaClassSaver(outputDirectory, transformers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ public class Transformers {

private final int targetVersion;
private final boolean defaultMethodsEnabled;
private final ClassHierarchyAnalyzer analyzer;
private final ClassAnalyzer analyzer;

public Transformers(int targetVersion, boolean defaultMethodsEnabled, ClassHierarchyAnalyzer analyzer) {
public Transformers(int targetVersion, boolean defaultMethodsEnabled, ClassAnalyzer analyzer) {
this.targetVersion = targetVersion;
this.defaultMethodsEnabled = defaultMethodsEnabled;
this.analyzer = analyzer;
Expand Down Expand Up @@ -48,7 +48,7 @@ public byte[] backportClass(ClassReader reader) {
next = new UpdateRelocatedMethodInvocations(next, analyzer);
next = new AddMethodDefaultImplementations(next, analyzer);
}
next = new BackportLambdaInvocations(next);
next = new BackportLambdaInvocations(next, analyzer);
return next;
});
}
Expand All @@ -59,7 +59,7 @@ public List<byte[]> backportInterface(ClassReader reader) {
// the wrong one of them is written to disk last.
ClassNode lambdasBackported = new ClassNode();
ClassVisitor next = lambdasBackported;
next = new BackportLambdaInvocations(next);
next = new BackportLambdaInvocations(next, analyzer);
reader.accept(next, 0);

List<byte[]> results = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@

public class AddMethodDefaultImplementations extends ClassVisitor {

private final ClassHierarchyAnalyzer analyzer;
private final ClassAnalyzer analyzer;
private String className;

public AddMethodDefaultImplementations(ClassVisitor next, ClassHierarchyAnalyzer analyzer) {
public AddMethodDefaultImplementations(ClassVisitor next, ClassAnalyzer analyzer) {
super(ASM5, next);
this.analyzer = analyzer;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import static net.orfjackal.retrolambda.util.Flags.*;
import static org.objectweb.asm.Opcodes.*;

public class ClassHierarchyAnalyzer {
public class ClassAnalyzer {

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

@Override
public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
if (isConstructor(name) || isStaticMethod(access)) {
return null;
int tag;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the static methods is the mechanism through which I'm going to implement point 2 on #81. By having a list of the private static methods which are synthetic and looking at invokedynamic's to those methods we can remove the private bit on the static method and skip the accessor.

if (isConstructor(name)) {
tag = H_INVOKESPECIAL;
} else if (isStaticMethod(access)) {
tag = H_INVOKESTATIC;
} else {
tag = H_INVOKEVIRTUAL;
}
c.addMethod(new MethodRef(H_INVOKEVIRTUAL, owner, name, desc), new MethodKind.Implemented());

c.addMethod(access, new MethodRef(tag, owner, name, desc), new MethodKind.Implemented());
return null;
}

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

if (isAbstractMethod(access)) {
c.addMethod(method, new MethodKind.Abstract());
c.addMethod(access, method, new MethodKind.Abstract());

} else if (isDefaultMethod(access)) {
MethodRef defaultImpl = new MethodRef(H_INVOKESTATIC, companion, name, Bytecode.prependArgumentType(desc, Type.getObjectType(owner)));
c.enableCompanionClass();
c.addMethod(method, new MethodKind.Default(defaultImpl));
c.addMethod(access, method, new MethodKind.Default(defaultImpl));

} else if (isInstanceLambdaImplMethod(access)) {
relocatedMethods.put(method, new MethodRef(H_INVOKESTATIC, companion, name, Bytecode.prependArgumentType(desc, Type.getObjectType(owner))));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public List<MethodInfo> getMethods() {
return Collections.unmodifiableList(methods);
}

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

public Optional<Type> getCompanionClass() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,19 @@

public class MethodInfo {

public final int access;
public final int tag;
public final MethodSignature signature;
public final Type owner;
public final MethodKind kind;

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

public MethodInfo(int tag, MethodSignature signature, Type owner, MethodKind kind) {
public MethodInfo(int access, int tag, MethodSignature signature, Type owner, MethodKind kind) {
this.access = access;
this.tag = tag;
this.signature = signature;
this.owner = owner;
Expand Down Expand Up @@ -58,7 +61,7 @@ public String toString() {
.addValue(signature)
.addValue(owner)
.addValue(kind)
.addValue("(" + tag + ")")
.addValue("(tag=" + tag + ", access=" + access + ")")
.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@

public class UpdateRelocatedMethodInvocations extends ClassVisitor {

private final ClassHierarchyAnalyzer analyzer;
private final ClassAnalyzer analyzer;

public UpdateRelocatedMethodInvocations(ClassVisitor next, ClassHierarchyAnalyzer analyzer) {
public UpdateRelocatedMethodInvocations(ClassVisitor next, ClassAnalyzer analyzer) {
super(ASM5, next);
this.analyzer = analyzer;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package net.orfjackal.retrolambda.lambdas;

import net.orfjackal.retrolambda.interfaces.*;
import net.orfjackal.retrolambda.util.Bytecode;
import org.objectweb.asm.*;

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

private int classAccess;
private String className;
private final ClassAnalyzer analyzer;
private final Map<Handle, Handle> lambdaAccessToImplMethods = new LinkedHashMap<>();

public BackportLambdaInvocations(ClassVisitor next) {
public BackportLambdaInvocations(ClassVisitor next, ClassAnalyzer analyzer) {
super(ASM5, next);
this.analyzer = analyzer;
}

@Override
Expand Down Expand Up @@ -56,20 +59,58 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si

Handle getLambdaAccessMethod(Handle implMethod) {
if (!implMethod.getOwner().equals(className)) {
return implMethod;
}
if (isInterface(classAccess)) {
// the method will be relocated to a companion class
return implMethod;
if (isNonOwnedMethodVisible(implMethod)) {
return implMethod;
}
} else {
if (isInterface(classAccess)) {
// the method will be relocated to a companion class
return implMethod;
}
if (isOwnedMethodVisible(implMethod)) {
// The method is visible to the companion class and therefore doesn't need an accessor.
return implMethod;
}
}
// TODO: do not generate an access method if the impl method is not private (probably not implementable with a single pass)
String name = "access$lambda$" + lambdaAccessToImplMethods.size();
String desc = getLambdaAccessMethodDesc(implMethod);
Handle accessMethod = new Handle(H_INVOKESTATIC, className, name, desc);
lambdaAccessToImplMethods.put(accessMethod, implMethod);
return accessMethod;
}

private boolean isOwnedMethodVisible(Handle implMethod) {
MethodSignature implSignature = new MethodSignature(implMethod.getName(), implMethod.getDesc());

Collection<MethodInfo> methods = analyzer.getMethods(Type.getObjectType(implMethod.getOwner()));
for (MethodInfo method : methods) {
if (method.signature.equals(implSignature)) {
// The method will be visible to the companion class if the private flag is absent.
return (method.access & ACC_PRIVATE) == 0;
}
}
throw new IllegalStateException("Non-analyzed method " + implMethod + ". Report this as a bug.");
}

private boolean isNonOwnedMethodVisible(Handle implMethod) {
String classPackage = className.substring(0, className.lastIndexOf('/'));
String implPackage = implMethod.getOwner().substring(0, implMethod.getOwner().lastIndexOf('/'));
if (classPackage.equals(implPackage)) {
return true; // All method visibilities in the same package will be visible.
}

MethodSignature implSignature = new MethodSignature(implMethod.getName(), implMethod.getDesc());

Collection<MethodInfo> methods = analyzer.getMethods(Type.getObjectType(implMethod.getOwner()));
for (MethodInfo method : methods) {
if (method.signature.equals(implSignature)) {
// The method will be visible to the companion class if the protected flag is absent.
return (method.access & ACC_PROTECTED) == 0;
}
}
return true;
}

private String getLambdaAccessMethodDesc(Handle implMethod) {
if (implMethod.getTag() == H_INVOKESTATIC) {
// static method call -> keep as-is
Expand Down
Loading