Refactored: added MethodRef.tag, removed the hack

Fixes #48
luontola · Apr 14, 2015 · 7146fa7 · 7146fa7
1 parent 8ce0fff
commit 7146fa7
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 58 deletions.
diff --git a/README.md b/README.md
@@ -207,6 +207,9 @@ Version History
 - Fixed a hack which caused lambdas in interfaces to be backported twice,
   possibly producing broken method calls in the bytecode
   ([Issue #48](https://github.com/orfjackal/retrolambda/issues/48))
+- Fixed the handling of non-static lambda implementation methods in
+  interfaces, i.e. lambdas which capture `this`
+  ([Issue #48](https://github.com/orfjackal/retrolambda/issues/48))
 - Removes generic method signatures from the default method implementation
   methods which are placed in the interface's companion class, to avoid
   them getting out of sync with their erased method descriptors

diff --git a/...a/src/main/java/net/orfjackal/retrolambda/interfaces/AddMethodDefaultImplementations.java b/...a/src/main/java/net/orfjackal/retrolambda/interfaces/AddMethodDefaultImplementations.java
@@ -30,8 +30,8 @@ public void visitEnd() {
         for (MethodInfo method : analyzer.getDefaultMethods(Type.getObjectType(className))) {
             Bytecode.generateDelegateMethod(cv,
                     ACC_PUBLIC | ACC_SYNTHETIC,
-                    method.toMethodRef().toHandle(H_INVOKEVIRTUAL),
-                    method.getDefaultMethodImpl().toHandle(H_INVOKESTATIC));
+                    method.toMethodRef().toHandle(),
+                    method.getDefaultMethodImpl().toHandle());
         }
         super.visitEnd();
     }

diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassHierarchyAnalyzer.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassHierarchyAnalyzer.java
@@ -4,6 +4,7 @@
 
 package net.orfjackal.retrolambda.interfaces;
 
+import net.orfjackal.retrolambda.lambdas.Handles;
 import net.orfjackal.retrolambda.util.*;
 import org.objectweb.asm.*;
 
@@ -46,7 +47,7 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si
                 if (isConstructor(name) || isStaticMethod(access)) {
                     return null;
                 }
-                c.addMethod(new MethodRef(owner, name, desc), new MethodKind.Implemented());
+                c.addMethod(new MethodRef(H_INVOKEVIRTUAL, owner, name, desc), new MethodKind.Implemented());
                 return null;
             }
 
@@ -66,22 +67,22 @@ 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) {
-                MethodRef method = new MethodRef(owner, name, desc);
+                MethodRef method = new MethodRef(Handles.accessToTag(access, true), owner, name, desc);
 
                 if (isAbstractMethod(access)) {
                     c.addMethod(method, new MethodKind.Abstract());
 
                 } else if (isDefaultMethod(access)) {
-                    MethodRef defaultImpl = new MethodRef(companion, name, Bytecode.prependArgumentType(desc, Type.getObjectType(owner)));
+                    MethodRef defaultImpl = new MethodRef(H_INVOKESTATIC, companion, name, Bytecode.prependArgumentType(desc, Type.getObjectType(owner)));
                     c.enableCompanionClass();
                     c.addMethod(method, new MethodKind.Default(defaultImpl));
 
                 } else if (isInstanceLambdaImplMethod(access)) {
-                    relocatedMethods.put(method, new MethodRef(companion, name, Bytecode.prependArgumentType(desc, Type.getObjectType(owner))));
+                    relocatedMethods.put(method, new MethodRef(H_INVOKESTATIC, companion, name, Bytecode.prependArgumentType(desc, Type.getObjectType(owner))));
                     c.enableCompanionClass();
 
                 } else if (isStaticMethod(access)) {
-                    relocatedMethods.put(method, new MethodRef(companion, name, desc));
+                    relocatedMethods.put(method, new MethodRef(H_INVOKESTATIC, companion, name, desc));
                     c.enableCompanionClass();
                 }
                 return null;
@@ -140,6 +141,13 @@ private ClassInfo getClass(Type type) {
     }
 
     public MethodRef getMethodCallTarget(MethodRef original) {
+        if (original.tag == H_INVOKESPECIAL) {
+            // change Interface.super.defaultMethod() calls to static calls on the companion class
+            MethodRef impl = getMethodDefaultImplementation(original);
+            if (impl != null) {
+                return impl;
+            }
+        }
         return relocatedMethods.getOrDefault(original, original);
     }
 

diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassInfo.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/ClassInfo.java
@@ -47,7 +47,7 @@ public List<MethodInfo> getMethods() {
     }
 
     public void addMethod(MethodRef method, MethodKind kind) {
-        methods.add(new MethodInfo(method.getSignature(), Type.getObjectType(method.owner), kind));
+        methods.add(new MethodInfo(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,15 +11,17 @@
 
 public class MethodInfo {
 
+    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) {
-        this(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
+        this(-1, new MethodSignature(name, desc), Type.getType(owner), kind);
     }
 
-    public MethodInfo(MethodSignature signature, Type owner, MethodKind kind) {
+    public MethodInfo(int tag, MethodSignature signature, Type owner, MethodKind kind) {
+        this.tag = tag;
         this.signature = signature;
         this.owner = owner;
         this.kind = kind;
@@ -30,14 +32,15 @@ public MethodRef getDefaultMethodImpl() {
     }
 
     public MethodRef toMethodRef() {
-        return new MethodRef(owner.getInternalName(), signature.name, signature.desc);
+        return new MethodRef(tag, owner.getInternalName(), signature.name, signature.desc);
     }
 
     @Override
     public boolean equals(Object obj) {
         if (!(obj instanceof MethodInfo)) {
             return false;
         }
+        // NOTE: the tag does not not affect method equality, because e.g. super calls have different tag but same method
         MethodInfo that = (MethodInfo) obj;
         return this.signature.equals(that.signature)
                 && this.owner.equals(that.owner)
@@ -55,6 +58,7 @@ public String toString() {
                 .addValue(signature)
                 .addValue(owner)
                 .addValue(kind)
+                .addValue("(" + tag + ")")
                 .toString();
     }
 }
diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/MethodRef.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/interfaces/MethodRef.java
@@ -5,6 +5,7 @@
 package net.orfjackal.retrolambda.interfaces;
 
 import com.google.common.base.MoreObjects;
+import net.orfjackal.retrolambda.lambdas.Handles;
 import org.objectweb.asm.*;
 
 import java.util.Objects;
@@ -13,15 +14,17 @@ public final class MethodRef {
 
     // TODO: replace MethodRef with ASM's Handle, or merge with MethodInfo?
 
+    public final int tag;
     public final String owner;
     public final String name;
     public final String desc;
 
-    public MethodRef(Class<?> owner, String name, String desc) {
-        this(Type.getInternalName(owner), name, desc);
+    public MethodRef(int tag, Class<?> owner, String name, String desc) {
+        this(tag, Type.getInternalName(owner), name, desc);
     }
 
-    public MethodRef(String owner, String name, String desc) {
+    public MethodRef(int tag, String owner, String name, String desc) {
+        this.tag = tag;
         this.owner = owner;
         this.name = name;
         this.desc = desc;
@@ -31,7 +34,11 @@ public MethodSignature getSignature() {
         return new MethodSignature(name, desc);
     }
 
-    public Handle toHandle(int tag) {
+    public int getOpcode() {
+        return Handles.getOpcode(toHandle());
+    }
+
+    public Handle toHandle() {
         return new Handle(tag, owner, name, desc);
     }
 
@@ -40,6 +47,7 @@ public boolean equals(Object obj) {
         if (!(obj instanceof MethodRef)) {
             return false;
         }
+        // NOTE: the tag does not not affect method equality, because e.g. super calls have different tag but same method
         MethodRef that = (MethodRef) obj;
         return this.owner.equals(that.owner)
                 && this.name.equals(that.name)
@@ -57,6 +65,7 @@ public String toString() {
                 .addValue(owner)
                 .addValue(name)
                 .addValue(desc)
+                .addValue("(" + tag + ")")
                 .toString();
     }
 }
diff --git a/.../src/main/java/net/orfjackal/retrolambda/interfaces/UpdateRelocatedMethodInvocations.java b/.../src/main/java/net/orfjackal/retrolambda/interfaces/UpdateRelocatedMethodInvocations.java
@@ -4,6 +4,7 @@
 
 package net.orfjackal.retrolambda.interfaces;
 
+import net.orfjackal.retrolambda.lambdas.Handles;
 import org.objectweb.asm.*;
 
 import static org.objectweb.asm.Opcodes.ASM5;
@@ -30,23 +31,9 @@ public UpdateMethodCalls(MethodVisitor next) {
 
         @Override
         public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
-            MethodRef method = new MethodRef(owner, name, desc);
-
-            // change Interface.super.defaultMethod() calls to static calls on the companion class
-            // TODO: move this inside getMethodCallTarget (also opcode, so must first change MethodRef to Handle)
-            if (opcode == Opcodes.INVOKESPECIAL) {
-                MethodRef impl = analyzer.getMethodDefaultImplementation(method);
-                if (impl != null) {
-                    opcode = Opcodes.INVOKESTATIC;
-                    method = impl;
-                }
-                if (name.startsWith("lambda$captureThis$")) { // FIXME: remove me
-                    opcode = Opcodes.INVOKESTATIC;
-                }
-            }
-
+            MethodRef method = new MethodRef(Handles.opcodeToTag(opcode), owner, name, desc);
             method = analyzer.getMethodCallTarget(method);
-            super.visitMethodInsn(opcode, method.owner, method.name, method.desc, itf);
+            super.visitMethodInsn(method.getOpcode(), method.owner, method.name, method.desc, itf);
         }
     }
 }
diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/Handles.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/Handles.java
@@ -1,9 +1,10 @@
-// Copyright © 2013-2014 Esko Luontola <www.orfjackal.net>
+// Copyright © 2013-2015 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.lambdas;
 
+import net.orfjackal.retrolambda.util.Flags;
 import org.objectweb.asm.Handle;
 
 import static org.objectweb.asm.Opcodes.*;
@@ -27,4 +28,33 @@ public static int getOpcode(Handle handle) {
                 throw new IllegalArgumentException("Unsupported tag " + tag + " in " + handle);
         }
     }
+
+    public static int opcodeToTag(int opcode) {
+        switch (opcode) {
+            case INVOKEVIRTUAL:
+                return H_INVOKEVIRTUAL;
+            case INVOKESTATIC:
+                return H_INVOKESTATIC;
+            case INVOKESPECIAL:
+                return H_INVOKESPECIAL;
+            case INVOKEINTERFACE:
+                return H_INVOKEINTERFACE;
+            default:
+                throw new IllegalArgumentException("Unsupported opcode " + opcode);
+        }
+    }
+
+    public static int accessToTag(int access, boolean itf) {
+        if (Flags.hasFlag(access, ACC_STATIC)) {
+            return H_INVOKESTATIC;
+        }
+        if (Flags.hasFlag(access, ACC_PRIVATE)) {
+            return H_INVOKESPECIAL;
+        }
+        if (itf) {
+            return H_INVOKEINTERFACE;
+        } else {
+            return H_INVOKEVIRTUAL;
+        }
+    }
 }