Fix Lambda desugaring with JDK17
Bug: b/236695789
Change-Id: I4826d8c005b6bc8aefe6064207e43ad395a1fdba
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/ClassConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/ClassConverter.java
index e195eac..36f717a 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/ClassConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/ClassConverter.java
@@ -174,6 +174,9 @@
definition.markNotProcessed();
}
methodProcessor.processMethod(method, instructionDesugaringEventConsumer);
+ if (interfaceProcessor != null) {
+ interfaceProcessor.processMethod(method, instructionDesugaringEventConsumer);
+ }
},
executorService);
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/AccessorMethodSourceCode.java b/src/main/java/com/android/tools/r8/ir/desugar/AccessorMethodSourceCode.java
index 8236617..6137206 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/AccessorMethodSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/AccessorMethodSourceCode.java
@@ -24,6 +24,7 @@
ForwardMethodBuilder.builder(appView.dexItemFactory()).setStaticSource(accessor);
switch (type) {
case INVOKE_INSTANCE:
+ case INVOKE_INTERFACE:
{
forwardMethodBuilder.setVirtualTarget(target, isInterface);
break;
@@ -43,8 +44,6 @@
forwardMethodBuilder.setConstructorTarget(target, appView.dexItemFactory());
break;
}
- case INVOKE_INTERFACE:
- throw new Unreachable("Accessor for an interface method?");
default:
throw new Unreachable();
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
index fb51c23..46266fc 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
@@ -322,6 +322,8 @@
case INVOKE_SUPER:
throw new Unimplemented("Method references to super methods are not yet supported");
case INVOKE_INTERFACE:
+ // TODO(b/238085938): Investigate if we can do something smart if
+ // canAccessModifyLambdaImplMethod().
return createInterfaceMethodTarget(accessedFrom);
case INVOKE_CONSTRUCTOR:
return createConstructorTarget(accessedFrom);
@@ -423,19 +425,23 @@
}
}
+ // Create targets for interface methods.
+ private Target createInterfaceMethodTarget(ProgramMethod accessedFrom) {
+ assert descriptor.implHandle.type.isInvokeInterface();
+ return createInstanceOrInterfaceTarget(accessedFrom);
+ }
+
// Create targets for instance method referenced directly without
// lambda$ methods. It may require creation of accessors in some cases.
private Target createInstanceMethodTarget(ProgramMethod accessedFrom) {
assert descriptor.implHandle.type.isInvokeInstance() ||
descriptor.implHandle.type.isInvokeDirect();
+ return createInstanceOrInterfaceTarget(accessedFrom);
+ }
+ private Target createInstanceOrInterfaceTarget(ProgramMethod accessedFrom) {
if (doesNotNeedAccessor(accessedFrom)) {
- return new NoAccessorMethodTarget(
- descriptor.implHandle.asMethod(),
- descriptor.implHandle.type.isInvokeDirect()
- ? Type.DIRECT
- : descriptor.implHandle.type.isInvokeStatic() ? Type.STATIC : Type.VIRTUAL,
- descriptor.implHandle.isInterface);
+ return NoAccessorMethodTarget.create(descriptor);
}
// We need to generate an accessor method in `accessedFrom` class/interface
@@ -459,12 +465,8 @@
.createMethod(
accessedFrom.getHolderType(), accessorProto, generateUniqueLambdaMethodName());
- return new ClassMethodWithAccessorTarget(
- descriptor.implHandle.asMethod(),
- descriptor.implHandle.isInterface,
- descriptor.implHandle.type,
- accessorMethod,
- appView);
+ return ClassMethodWithAccessorTarget.create(
+ descriptor.implHandle, accessedFrom, accessorMethod, appView);
}
// Create targets for static method referenced directly without
@@ -473,8 +475,7 @@
assert descriptor.implHandle.type.isInvokeStatic();
if (doesNotNeedAccessor(accessedFrom)) {
- return new NoAccessorMethodTarget(
- descriptor.implHandle.asMethod(), Type.STATIC, descriptor.implHandle.isInterface);
+ return NoAccessorMethodTarget.create(descriptor);
}
// We need to generate an accessor method in `accessedFrom` class/interface
@@ -487,12 +488,8 @@
accessedFrom.getHolderType(),
descriptor.implHandle.asMethod().proto,
generateUniqueLambdaMethodName());
- return new ClassMethodWithAccessorTarget(
- descriptor.implHandle.asMethod(),
- descriptor.implHandle.isInterface,
- descriptor.implHandle.type,
- accessorMethod,
- appView);
+ return ClassMethodWithAccessorTarget.create(
+ descriptor.implHandle, accessedFrom, accessorMethod, appView);
}
// Create targets for constructor referenced directly without lambda$ methods.
@@ -503,8 +500,7 @@
assert implHandle.type.isInvokeConstructor();
if (doesNotNeedAccessor(accessedFrom)) {
- return new NoAccessorMethodTarget(
- descriptor.implHandle.asMethod(), Type.DIRECT, descriptor.implHandle.isInterface);
+ return NoAccessorMethodTarget.create(descriptor);
}
// We need to generate an accessor method in `accessedFrom` class/interface for
@@ -520,20 +516,8 @@
.dexItemFactory()
.createMethod(
accessedFrom.getHolderType(), accessorProto, generateUniqueLambdaMethodName());
- return new ClassMethodWithAccessorTarget(
- descriptor.implHandle.asMethod(),
- descriptor.implHandle.isInterface,
- descriptor.implHandle.type,
- accessorMethod,
- appView);
- }
-
- // Create targets for interface methods.
- private Target createInterfaceMethodTarget(ProgramMethod accessedFrom) {
- assert descriptor.implHandle.type.isInvokeInterface();
- assert doesNotNeedAccessor(accessedFrom);
- return new NoAccessorMethodTarget(
- descriptor.implHandle.asMethod(), Type.INTERFACE, descriptor.implHandle.isInterface);
+ return ClassMethodWithAccessorTarget.create(
+ descriptor.implHandle, accessedFrom, accessorMethod, appView);
}
private DexString generateUniqueLambdaMethodName() {
@@ -594,6 +578,13 @@
// Used for targeting methods referenced directly without creating accessors.
public static final class NoAccessorMethodTarget extends Target {
+ static NoAccessorMethodTarget create(LambdaDescriptor descriptor) {
+ return new NoAccessorMethodTarget(
+ descriptor.implHandle.asMethod(),
+ descriptor.implHandle.type.toInvokeType(),
+ descriptor.implHandle.isInterface);
+ }
+
NoAccessorMethodTarget(DexMethod method, Type invokeType, boolean isInterface) {
super(method, invokeType, isInterface);
}
@@ -789,21 +780,37 @@
private final AppView<?> appView;
private final DexMethod implMethod;
+ private final boolean implMethodIsInterface;
private final MethodHandleType type;
- ClassMethodWithAccessorTarget(
+ static ClassMethodWithAccessorTarget create(
+ DexMethodHandle implHandle,
+ ProgramMethod accessedFrom,
+ DexMethod accessorMethod,
+ AppView<?> appView) {
+ return new ClassMethodWithAccessorTarget(
+ implHandle.asMethod(),
+ implHandle.isInterface,
+ implHandle.type,
+ accessorMethod,
+ accessedFrom.getHolder().isInterface(),
+ appView);
+ }
+
+ private ClassMethodWithAccessorTarget(
DexMethod implMethod,
boolean isInterface,
MethodHandleType type,
DexMethod accessorMethod,
+ boolean accessorIsInterface,
AppView<?> appView) {
- super(accessorMethod, Invoke.Type.STATIC, isInterface);
+ super(accessorMethod, Invoke.Type.STATIC, accessorIsInterface);
this.appView = appView;
this.implMethod = implMethod;
+ this.implMethodIsInterface = isInterface;
this.type = type;
}
-
@Override
ProgramMethod ensureAccessibility(
ForcefullyMovedLambdaMethodConsumer forcefullyMovedLambdaMethodConsumer,
@@ -831,7 +838,7 @@
.setAccessFlags(MethodAccessFlags.createPublicStaticSynthetic())
.setCode(
AccessorMethodSourceCode.build(
- implMethod, isInterface, type, callTarget, appView))
+ implMethod, implMethodIsInterface, type, callTarget, appView))
// The api level is computed when tracing.
.disableAndroidApiLevelCheck()
.build());
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaDescriptor.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaDescriptor.java
index d6b10b0..df7be1b 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaDescriptor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaDescriptor.java
@@ -203,15 +203,18 @@
/** Checks if call site needs a accessor when referenced from `accessedFrom`. */
boolean needsAccessor(ProgramMethod accessedFrom) {
- if (implHandle.type.isInvokeInterface()) {
- // Interface methods must be public.
+ // The invoke-interface may target a private method through nest based access in JDK 17.
+ // If the targetAccessFlags are missing we assume no accessor is needed since that was D8/R8's
+ // behavior prior to the introduction of bridges on invoke-interface.
+ if (implHandle.type.isInvokeInterface()
+ && (targetAccessFlags == null || targetAccessFlags.isPublic())) {
return false;
}
boolean staticTarget = implHandle.type.isInvokeStatic();
boolean instanceTarget = implHandle.type.isInvokeInstance() || implHandle.type.isInvokeDirect();
boolean initTarget = implHandle.type.isInvokeConstructor();
- assert instanceTarget || staticTarget || initTarget;
+ assert instanceTarget || staticTarget || initTarget || implHandle.type.isInvokeInterface();
assert !implHandle.type.isInvokeDirect()
|| (targetAccessFlags.isPrivate()
&& !targetAccessFlags.isConstructor()
diff --git a/src/test/examplesJava17/lambda/Lambda.java b/src/test/examplesJava17/lambda/Lambda.java
new file mode 100644
index 0000000..d0c1035
--- /dev/null
+++ b/src/test/examplesJava17/lambda/Lambda.java
@@ -0,0 +1,46 @@
+// Copyright (c) 2022, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package lambda;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class Lambda {
+
+ interface StringPredicate {
+
+ boolean test(String t);
+
+ default StringPredicate or(StringPredicate other) {
+ return (t) -> test(t) || other.test(t);
+ }
+ }
+
+ public static void main(String[] args) {
+ ArrayList<String> strings = new ArrayList<>();
+ strings.add("abc");
+ strings.add("abb");
+ strings.add("bbc");
+ strings.add("aac");
+ strings.add("acc");
+ StringPredicate aaStart = Lambda::aaStart;
+ StringPredicate bbNot = Lambda::bbNot;
+ StringPredicate full = aaStart.or(bbNot);
+ for (String string : ((List<String>) strings.clone())) {
+ if (full.test(string)) {
+ strings.remove(string);
+ }
+ }
+ System.out.println(strings);
+ }
+
+ private static boolean aaStart(String str) {
+ return str.startsWith("aa");
+ }
+
+ private static boolean bbNot(String str) {
+ return !str.contains("bb");
+ }
+}
diff --git a/src/test/examplesJava17/nest/NestLambda.java b/src/test/examplesJava17/nest/NestLambda.java
index 2c690b5..c60a8fb 100644
--- a/src/test/examplesJava17/nest/NestLambda.java
+++ b/src/test/examplesJava17/nest/NestLambda.java
@@ -4,8 +4,6 @@
package nest;
-import java.util.function.Consumer;
-
public class NestLambda {
private void print(Object o) {
@@ -16,9 +14,14 @@
return new Inner();
}
- class Inner {
+ // Avoids java.util.Consumer to run below on Apis below 24.
+ interface ThisConsumer<T> {
+ void accept(T t);
+ }
- void exec(Consumer<Object> consumer) {
+ class Inner implements Itf {
+
+ void exec(ThisConsumer<String> consumer) {
consumer.accept("inner");
}
@@ -27,7 +30,22 @@
}
}
+ interface Itf {
+ private void printItf(Object o) {
+ System.out.println("printed from itf: " + o);
+ }
+ }
+
+ void exec(ThisConsumer<String> consumer) {
+ consumer.accept("here");
+ }
+
+ void execItfLambda(Itf itf) {
+ exec(itf::printItf);
+ }
+
public static void main(String[] args) {
new NestLambda().getInner().execLambda();
+ new NestLambda().execItfLambda(new Itf() {});
}
}
diff --git a/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaJava17Test.java b/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaJava17Test.java
new file mode 100644
index 0000000..6c9e899
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaJava17Test.java
@@ -0,0 +1,79 @@
+// Copyright (c) 2022, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.desugar.lambdas;
+
+import static com.android.tools.r8.utils.AndroidApiLevel.B;
+import static com.android.tools.r8.utils.FileUtils.JAR_EXTENSION;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRuntime.CfVm;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import org.junit.Assume;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class LambdaJava17Test extends TestBase {
+
+ public LambdaJava17Test(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ private static final Path JDK17_JAR =
+ Paths.get(ToolHelper.TESTS_BUILD_DIR, "examplesJava17").resolve("lambda" + JAR_EXTENSION);
+ private static final String MAIN = "lambda.Lambda";
+ private static final String EXPECTED_RESULT = "[abb, bbc]";
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters()
+ .withCfRuntimesStartingFromIncluding(CfVm.JDK17)
+ .withDexRuntimes()
+ .withAllApiLevels()
+ .enableApiLevelsForCf()
+ .build();
+ }
+
+ @Test
+ public void testReference() throws Exception {
+ Assume.assumeTrue(parameters.isCfRuntime());
+ testForJvm()
+ .addProgramFiles(JDK17_JAR)
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutputLines(EXPECTED_RESULT);
+ }
+
+ @Test
+ public void testJavaD8() throws Exception {
+ testForDesugaring(parameters)
+ .addProgramFiles(JDK17_JAR)
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutputLines(EXPECTED_RESULT);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ Assume.assumeTrue(parameters.isDexRuntime() || parameters.getApiLevel().equals(B));
+ testForR8(parameters.getBackend())
+ .addProgramFiles(JDK17_JAR)
+ .applyIf(
+ parameters.isCfRuntime() && !parameters.getApiLevel().isEqualTo(AndroidApiLevel.B),
+ // Alternatively we need to pass Jdk17 as library.
+ b -> b.addKeepRules("-dontwarn java.lang.invoke.StringConcatFactory"))
+ .setMinApi(parameters.getApiLevel())
+ .addKeepMainRule(MAIN)
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutputLines(EXPECTED_RESULT);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestLambdaJava17Test.java b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestLambdaJava17Test.java
index 94fbe71..a166409 100644
--- a/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestLambdaJava17Test.java
+++ b/src/test/java/com/android/tools/r8/desugar/nestaccesscontrol/NestLambdaJava17Test.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.desugar.nestaccesscontrol;
-import static com.android.tools.r8.utils.AndroidApiLevel.B;
import static com.android.tools.r8.utils.FileUtils.JAR_EXTENSION;
import com.android.tools.r8.TestBase;
@@ -12,8 +11,7 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.TestRuntime.CfVm;
import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ToolHelper.DexVm.Version;
-import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
import java.nio.file.Path;
import java.nio.file.Paths;
import org.junit.Assume;
@@ -32,7 +30,8 @@
private static final Path JDK17_JAR =
Paths.get(ToolHelper.TESTS_BUILD_DIR, "examplesJava17").resolve("nest" + JAR_EXTENSION);
private static final String MAIN = "nest.NestLambda";
- private static final String EXPECTED_RESULT = "printed: inner";
+ private static final String EXPECTED_RESULT =
+ StringUtils.lines("printed: inner", "printed from itf: here");
private final TestParameters parameters;
@@ -40,10 +39,8 @@
public static TestParametersCollection data() {
return getTestParameters()
.withCfRuntimesStartingFromIncluding(CfVm.JDK17)
- // The test requires the java.util.function. package.
- .withDexRuntimesStartingFromIncluding(Version.V7_0_0)
- .withApiLevel(AndroidApiLevel.N)
- .enableApiLevelsForCf()
+ .withDexRuntimes()
+ .withAllApiLevelsAlsoForCf()
.build();
}
@@ -53,20 +50,20 @@
testForJvm()
.addProgramFiles(JDK17_JAR)
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutputLines(EXPECTED_RESULT);
+ .assertSuccessWithOutput(EXPECTED_RESULT);
}
@Test
public void testJavaD8() throws Exception {
- testForDesugaring(parameters)
+ testForD8(parameters.getBackend())
.addProgramFiles(JDK17_JAR)
+ .setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutputLines(EXPECTED_RESULT);
+ .assertSuccessWithOutput(EXPECTED_RESULT);
}
@Test
public void testR8() throws Exception {
- Assume.assumeTrue(parameters.isDexRuntime() || parameters.getApiLevel().equals(B));
testForR8(parameters.getBackend())
.addProgramFiles(JDK17_JAR)
.applyIf(
@@ -76,6 +73,6 @@
.setMinApi(parameters.getApiLevel())
.addKeepMainRule(MAIN)
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutputLines(EXPECTED_RESULT);
+ .assertSuccessWithOutput(EXPECTED_RESULT);
}
}
