Do not generate accessor method for non-private methods.

JakeWharton · JakeWharton · commit d6985daca3b2 · 2016-01-10T22:17:24.000-08:00
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/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/AddMethodDefaultImplementations.java b/retrolambda/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/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassAnalyzer.java b/retrolambda/src/main/java/net/orfjackal/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/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/UpdateRelocatedMethodInvocations.java b/retrolambda/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
@@ -62,14 +65,30 @@ Handle getLambdaAccessMethod(Handle implMethod) {
             // the method will be relocated to a companion class
             return implMethod;
         }
-        // TODO: do not generate an access method if the impl method is not private (probably not implementable with a single pass)
+        if (isOwnedMethodVisible(implMethod)) {
+            // The method is visible to the companion class and therefore doesn't need an accessor.
+            return implMethod;
+        }
         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 String getLambdaAccessMethodDesc(Handle implMethod) {
         if (implMethod.getTag() == H_INVOKESTATIC) {
             // static method call -> keep as-is
diff --git a/retrolambda/src/test/java/net/orfjackal/retrolambda/ClassAnalyzerTest.java b/retrolambda/src/test/java/net/orfjackal/retrolambda/ClassAnalyzerTest.java

Original file line number	Diff line number	Diff line change
`@@ -44,8 +44,8 @@ public List<MethodInfo> getMethods() {`
`44`	`44`	`return Collections.unmodifiableList(methods);`
`45`	`45`	`}`
`46`	`46`
`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));`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	`public Optional<Type> getCompanionClass() {`
Original file line number	Diff line number	Diff line change
`@@ -11,9 +11,9 @@`
`11`	`11`
`12`	`12`	`public class UpdateRelocatedMethodInvocations extends ClassVisitor {`
`13`	`13`
`14`		`- private final ClassHierarchyAnalyzer analyzer;`
	`14`	`+ private final ClassAnalyzer analyzer;`
`15`	`15`
`16`		`- public UpdateRelocatedMethodInvocations(ClassVisitor next, ClassHierarchyAnalyzer analyzer) {`
	`16`	`+ public UpdateRelocatedMethodInvocations(ClassVisitor next, ClassAnalyzer analyzer) {`
`17`	`17`	`super(ASM5, next);`
`18`	`18`	`this.analyzer = analyzer;`
`19`	`19`	`}`