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.
luontola · Apr 27, 2016 · fb6101a · fb6101a
1 parent 365b819
commit fb6101a
Show file tree

Hide file tree

Showing 11 changed files with 131 additions and 62 deletions.
diff --git a/end-to-end-tests/src/test/java/net/orfjackal/retrolambda/test/LambdaTest.java b/end-to-end-tests/src/test/java/net/orfjackal/retrolambda/test/LambdaTest.java
@@ -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.*;
@@ -150,6 +151,19 @@ 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> ref = new SubclassInMyPackage().thing();
+
+        assertThat(ref.call(), equalTo("Hello"));
+    }
+
+    public static class SubclassInMyPackage extends DifferentPackageBase {
+        public Callable<String> thing() {
+            return DifferentPackageBase::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.

diff --git a/...sts/src/test/java/net/orfjackal/retrolambda/test/anotherpackage/DifferentPackageBase.java b/...sts/src/test/java/net/orfjackal/retrolambda/test/anotherpackage/DifferentPackageBase.java
@@ -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";
+    }
+}
diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/Retrolambda.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/Retrolambda.java
@@ -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);

diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/Transformers.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/Transformers.java
@@ -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;
@@ -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;
         });
     }
@@ -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<>();

diff --git a/...a/src/main/java/net/orfjackal/retrolambda/interfaces/AddMethodDefaultImplementations.java b/...a/src/main/java/net/orfjackal/retrolambda/interfaces/AddMethodDefaultImplementations.java
@@ -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;
     }

diff --git a/...da/interfaces/ClassHierarchyAnalyzer.java → ...retrolambda/interfaces/ClassAnalyzer.java b/...da/interfaces/ClassHierarchyAnalyzer.java → ...retrolambda/interfaces/ClassAnalyzer.java
@@ -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<>();
@@ -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;
+                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;
             }
 
@@ -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))));

diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassInfo.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassInfo.java
@@ -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() {

diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/MethodInfo.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/MethodInfo.java
@@ -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;
@@ -58,7 +61,7 @@ public String toString() {
                 .addValue(signature)
                 .addValue(owner)
                 .addValue(kind)
-                .addValue("(" + tag + ")")
+                .addValue("(tag=" + tag + ", access=" + access + ")")
                 .toString();
     }
 }
diff --git a/.../src/main/java/net/orfjackal/retrolambda/interfaces/UpdateRelocatedMethodInvocations.java b/.../src/main/java/net/orfjackal/retrolambda/interfaces/UpdateRelocatedMethodInvocations.java
@@ -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;
     }

diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaInvocations.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaInvocations.java
@@ -4,6 +4,7 @@
 
 package net.orfjackal.retrolambda.lambdas;
 
+import net.orfjackal.retrolambda.interfaces.*;
 import net.orfjackal.retrolambda.util.Bytecode;
 import org.objectweb.asm.*;
 
@@ -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
@@ -56,20 +59,52 @@ 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) {
+        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