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

Method references to private methods #17

Closed
naixx opened this issue May 13, 2014 · 14 comments
Closed

Method references to private methods #17

naixx opened this issue May 13, 2014 · 14 comments

Comments

@naixx
Copy link

naixx commented May 13, 2014

Hi. The problem is:

{
  someObserable.finallyDo(this::myProblemMethod);
}

private void myProblemMethod(){}

The code is compiled and dexed to run on Davlik VM. But in runtime on an android device I get warnings and the method isn't called, of course.

 DexOpt: illegal method access (ca;.myProblemMethod ()V from (Fragment$$Lambda$1;)
 I/dalvikvm﹕ Could not find method myProblemMethod, referenced from method Fragment$$Lambda$1.call
VFY: unable to resolve virtual method 43525:

The code works as a lambda if myProblemMethod is package private, public, everything except private. The method can be called explicitly. But it doesn't work as a method reference. Warnings appear only once.

So, is it a problem, that retrolambda somehow processes references, so that dex excludes them from the build?

luontola added a commit that referenced this issue May 13, 2014
@luontola
Copy link
Owner

Thanks. I was able to reproduce it. I'll have a look at solving it soon, maybe tomorrow.

I'm guessing that this has something to do with how in Java 8 the lambda classes have some special handling to that they can call private methods of other classes. Retrolamdba should probably detect when a method reference targets a private method and make it package private.

@luontola luontola changed the title Davlik optimisation of method references Method references to private methods May 13, 2014
@luontola
Copy link
Owner

This isn't a Dalvik specific problem, so I renamed the issue.

@naixx
Copy link
Author

naixx commented May 13, 2014

Great to hear, that the problem is identified!

@luontola
Copy link
Owner

Still one bug remains (seems to depend on JDK 8 build). Working on it...

@luontola luontola reopened this May 14, 2014
@luontola
Copy link
Owner

The fix is included in Retrolambda 1.2.2. It should show up in Maven Central in a couple of hours.

@naixx
Copy link
Author

naixx commented May 14, 2014

Looks like magic, thanks!

@naixx
Copy link
Author

naixx commented May 16, 2014

Hello, @orfjackal !

After updating to the new version, I've catched a crash.

 @Test
    public void call_of_method_with_reference(){
        class InnerClass {
            void foo(){
                Runnable r = LambdaTest.this::privateVoidMethod;
                r.run();
            }
        }
        new InnerClass().foo();
        privateVoidMethod();
        assertThat(privateInstanceMethod(), is("foo"));
    }

The code structure is like this. I haven't checked it in a test.

@luontola
Copy link
Owner

Please submit a SSCCE. You example doesn't compile and I haven't managed to reproduce the problem.

@naixx
Copy link
Author

naixx commented May 18, 2014

@orfjackal ok, finally I managed to reproduce the problem.

void thisMethodIsAlsoReferenced() {
    Observable.from("foo").lift((Operator<String, String>) (subscriber) -> {
        Runnable ref = MyOuterClass.this::problemPrivateMethod; //we make a reference in an anonymous class
        ref.run();
        return subscriber;
    }).subscribe(s -> {
        L.e(s);
    });
    problemPrivateMethod(); //java.lang.NoSuchMethodError
}

Unfortunately, I can't reproduce the problem in tests.

@luontola
Copy link
Owner

Please make that example short and self contained (http://www.sscce.org/) so that I can run it. Or send the me .class files before and after processing that code with Retrolambda.

Does the problem happen with Oracle JVM or only Dalvik VM?

@naixx
Copy link
Author

naixx commented May 18, 2014

I tried to make a simple junit test in a fork of your repo, but the problem didn't appear in that case.
The only way to reproduce is run on Android device.

https://github.com/naixx/PrivateMethodAndroidBug
Here is an Android project, that reproduces the problem with Davlik VM. I hope it matches SSCCE a bit :)

The warnings I get:

I/dalvikvm﹕ Could not find method com.example.testbug.app.MainActivity.problemPrivateMethod, referenced from method com.example.testbug.app.MainActivity.methodCall
W/dalvikvm﹕ VFY: unable to resolve direct method 8443: Lcom/example/testbug/app/MainActivity;.problemPrivateMethod ()V

And error on method usage.

I've also added classes before and after retrolambda from a real project. The problem method is cleanup and problemPrivateMethod

@luontola
Copy link
Owner

How to run that project? I can compile it with gradlew clean build but I haven't done any Android development and I can't figure out the command for starting the application.

@naixx
Copy link
Author

naixx commented May 18, 2014

If you don't have device, the easiest way is to run it through emulator or Genymotion. After build to run it on device or emulator type adb install ResultedFile.apk.
You can also try Android Studio for that propose. It is the simplest way.
I hope, this can help.

@luontola
Copy link
Owner

Thanks. I was able to reproduce it. Let's continue discussion in #18

Arneball added a commit to Arneball/retrolambda that referenced this issue Aug 11, 2014
public interface MyInterface {
    default String def() {
        return "[" + toString() + ", " + "def]";
    }

    default String join(MyInterface other) {
        return def() + other.def();
    }
}

class C implements MyInterface{}  compiles to:

public class testpackage.MyClass implements testpackage.MyInterface {
  public testpackage.MyClass();
    Code:
       0: aload_0
       1: invokespecial luontola#16                 // Method java/lang/Object."<init>":()V
       4: return

  public java.lang.String join(testpackage.MyInterface);
    Code:
       0: aload_0
       1: aload_1
       2: invokestatic  luontola#46                 // Method testpackage/MyInterfacehelper.join:(Ltestpackage/MyInterface;Ltestpackage/MyInterface;)Ljava/lang/String;
       5: areturn

  public java.lang.String def();
    Code:
       0: aload_0
       1: invokestatic  luontola#49                 // Method testpackage/MyInterfacehelper.def:(Ltestpackage/MyInterface;)Ljava/lang/String;
       4: areturn
}

Where testpackage.MyInterfacehelper is compiled to
public class testpackage.MyInterfacehelper {
  private testpackage.MyInterfacehelper();
    Code:
       0: aload_0
       1: invokespecial luontola#9                  // Method java/lang/Object."<init>":()V
       4: return

  public static java.lang.String def(testpackage.MyInterface);
    Code:
       0: new           luontola#13                 // class java/lang/StringBuilder
       3: dup
       4: invokespecial luontola#14                 // Method java/lang/StringBuilder."<init>":()V
       7: ldc           luontola#16                 // String [
       9: invokevirtual luontola#20                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      12: aload_0
      13: invokevirtual luontola#24                 // Method java/lang/Object.toString:()Ljava/lang/String;
      16: invokevirtual luontola#20                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      19: ldc           luontola#26                 // String ,
      21: invokevirtual luontola#20                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      24: ldc           luontola#28                 // String def]
      26: invokevirtual luontola#20                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      29: invokevirtual luontola#29                 // Method java/lang/StringBuilder.toString:()Ljava/lang/String;
      32: areturn

  public static java.lang.String join(testpackage.MyInterface, testpackage.MyInterface);
    Code:
       0: new           luontola#13                 // class java/lang/StringBuilder
       3: dup
       4: invokespecial luontola#14                 // Method java/lang/StringBuilder."<init>":()V
       7: aload_0
       8: invokeinterface luontola#35,  1           // InterfaceMethod testpackage/MyInterface.def:()Ljava/lang/String;
      13: invokevirtual luontola#20                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      16: aload_1
      17: invokeinterface luontola#35,  1           // InterfaceMethod testpackage/MyInterface.def:()Ljava/lang/String;
      22: invokevirtual luontola#20                 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder;
      25: invokevirtual luontola#29                 // Method java/lang/StringBuilder.toString:()Ljava/lang/String;
      28: areturn
}

==============
interface StaticTest {
    static <T> T staticMethod(T t) {
        return t;
    }
}
compiles to
public class testpackage.StaticTesthelper {
  private testpackage.StaticTesthelper();
    Code:
       0: aload_0
       1: invokespecial luontola#9                  // Method java/lang/Object."<init>":()V
       4: return

  public static <T> T staticMethod$static(T);
    Code:
       0: aload_0
       1: areturn
}

========

Brigde methods are generated properly in example:
public interface BridgeParent<T> {
    T get();
}
public interface StringBridges extends BridgeParent<String> {
    @OverRide
    default String get() {
        return "default method";
    }

    default String concrete() {
        return "concrete";
    }
}
public class testpackage.StringBridgeshelper
  SourceFile: "testpackage/StringBridges.java"
  minor version: 0
  major version: 50
  flags: ACC_PUBLIC, ACC_SUPER
Constant pool:
   luontola#1 = Utf8               testpackage/StringBridgeshelper
   luontola#2 = Class              luontola#1             //  testpackage/StringBridgeshelper
   luontola#3 = Utf8               java/lang/Object
   luontola#4 = Class              luontola#3             //  java/lang/Object
   luontola#5 = Utf8               testpackage/StringBridges.java
   luontola#6 = Utf8               <init>
   luontola#7 = Utf8               ()V
   luontola#8 = NameAndType        luontola#6:luontola#7          //  "<init>":()V
   luontola#9 = Methodref          luontola#4.luontola#8          //  java/lang/Object."<init>":()V
  luontola#10 = Utf8               get
  luontola#11 = Utf8               (Ltestpackage/StringBridges;)Ljava/lang/String;
  luontola#12 = Utf8               default method
  luontola#13 = String             luontola#12            //  default method
  luontola#14 = Utf8               concrete
  luontola#15 = String             luontola#14            //  concrete
  luontola#16 = Utf8               (Ltestpackage/StringBridges;)Ljava/lang/Object;
  luontola#17 = Utf8               testpackage/StringBridges
  luontola#18 = Class              luontola#17            //  testpackage/StringBridges
  luontola#19 = Utf8               ()Ljava/lang/String;
  luontola#20 = NameAndType        luontola#10:luontola#19        //  get:()Ljava/lang/String;
  luontola#21 = InterfaceMethodref luontola#18.luontola#20        //  testpackage/StringBridges.get:()Ljava/lang/String;
  luontola#22 = Utf8               Code
  luontola#23 = Utf8               LineNumberTable
  luontola#24 = Utf8               SourceFile
{
  private testpackage.StringBridgeshelper();
    descriptor: ()V
    flags: ACC_PRIVATE
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokespecial luontola#9                  // Method java/lang/Object."<init>":()V
         4: return

  public static java.lang.String get(testpackage.StringBridges);
    descriptor: (Ltestpackage/StringBridges;)Ljava/lang/String;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=1, locals=1, args_size=1
         0: ldc           luontola#13                 // String default method
         2: areturn
      LineNumberTable:
        line 9: 0

  public static java.lang.String concrete(testpackage.StringBridges);
    descriptor: (Ltestpackage/StringBridges;)Ljava/lang/String;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=1, locals=1, args_size=1
         0: ldc           luontola#15                 // String concrete
         2: areturn
      LineNumberTable:
        line 13: 0

  public static java.lang.Object get(testpackage.StringBridges);
    descriptor: (Ltestpackage/StringBridges;)Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_STATIC, ACC_BRIDGE, ACC_SYNTHETIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokeinterface luontola#21,  1           // InterfaceMethod testpackage/StringBridges.get:()Ljava/lang/String;
         6: areturn
      LineNumberTable:
        line 6: 0
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants