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

Desugaring a method reference implicitly requires Java 7 APIs (Android API 19+). #2522

Closed
JakeWharton opened this issue Feb 10, 2017 · 8 comments
Assignees

Comments

@JakeWharton
Copy link

Description of the problem / feature request / question:

When desugaring a method reference for Android, the lambda meta factory emits a call to Objects.requireNonNull before passing the captured instance to the generated class. This method was added in Java 7 which is Android API 19+ only.

I'm not sure what API level you're supporting with this feature, but all other emitted bytecode is certainly Java 6 compatible which would give you Android API 9+.

If possible, provide a minimal example to reproduce the problem:

public final class MainActivity extends Activity {
  @Override protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);

    Executor executor = Executors.newSingleThreadExecutor();
    Greeter greeter = new Greeter();
    executor.execute(greeter::sayHi);
  }

  static class Greeter {
    void sayHi() {
      System.out.println("Hi!");
    }
  }
}

Environment info

  • Operating System: OSX 10.12.3
  • Bazel version (output of bazel info release): master @ 89512a7

Anything else, information or logs or outputs that would be helpful?

Here's the bytecode output after desugaring:

      28: aload_3
      29: dup
      30: invokestatic  #41                 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
      33: pop
      34: invokestatic  #44                 // Method com/google/bazel/example/android/activities/MainActivity$$Lambda$7.get$Lambda:(Lcom/google/bazel/example/android/activities/MainActivity$Greeter;)Ljava/lang/Runnable;
      37: invokeinterface #35,  2           // InterfaceMethod java/util/concurrent/Executor.execute:(Ljava/lang/Runnable;)V
@kevin1e100
Copy link
Contributor

You should be able to avoid this by including -XDallowBetterNullChecks=false into the javacopts, either using bazel's --javacopts command line flag or your android_library target's javacopts attribute. could you give that a try please? we'll look into a fix that's more turnkey.

@aj-michael
Copy link
Contributor

Thanks for the detailed report!

Some more background, (as provided by @kevin1e100), the bazel android desugarer is not actually writing out Objects.requireNonNull, but javac9 is synthetically adding it (see https://bugs.openjdk.java.net/browse/JDK-8074306). Bazel bundles javac9 in https://github.com/bazelbuild/bazel/blob/master/third_party/java/jdk/langtools, which is being used with javacopts -source 8 -target 8 if you use --experimental_desugar_for_android. So javac9 sees the target level and thinks its fine to write synthetic java7 methods, such as Objects.requireNonNull.

@JakeWharton
Copy link
Author

A quick try of that flag didn't seem to affect the output. And thanks for the info around the compiler. I should have known because the error-prone issue has been getting some activity of late as well.

@aj-michael
Copy link
Contributor

The allowBetterNullChecks flag that @kevin1e100 mentioned is a fork that is not in Bazel's javac and is not in the javac we have at https://github.com/bazelbuild/bazel/blob/master/third_party/java/jdk/langtools.

One solution would be to have the desugaring tool remove this methods. @kevin1e100 do you think its worth patching Bazel's javac in the mean time to add that flag?

@kevin1e100
Copy link
Contributor

Err sorry about that. I'll see how we can work around this, stay tuned

@cushon
Copy link
Contributor

cushon commented Feb 14, 2017

One solution would be to have the desugaring tool remove this methods

I think we need this even with the javac patch; we're going to want it to work with stock JDK 9's eventually.

do you think its worth patching Bazel's javac in the mean time to add that flag?

I'm on it.

@cushon
Copy link
Contributor

cushon commented Feb 23, 2017

Support for -XDallowBetterNullChecks=false was added in 2e689c2.

@aj-michael
Copy link
Contributor

b35a0c0 updated the desugarer to desugar calls to requireNonNull(Object o) so I'm going to close this issue. Unfortunately, that commit won't be in the Bazel 0.4.5 release that is happening Monday, but it will be in the next one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants