Version 2.0.25
Cherry-pick: Mark default methods as live if a sub-interface is
instantiated
CL: https://r8-review.googlesource.com/48019
Cherry-pick: Resolve default methods in sub-interfaces
CL: https://r8-review.googlesource.com/48018
Cherry-pick: Rename callsite different from the implementing
interfaces' methods
CL: https://r8-review.googlesource.com/48007
Bug: 148447828
Bug: 148168065
Bug: 148368442
Change-Id: I18a711bbd3a404e17aa3dd6bd7d9d5f5bca72b9e
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 1b43f56..d2ef72e 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.0.24";
+ public static final String LABEL = "2.0.25";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
index 4854741..229d8eb 100644
--- a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
@@ -341,12 +341,7 @@
// public void bar() { }
// }
//
- if (resolvedMethod.hasCode()) {
- DexProgramClass holder = resolvedHolder.asProgramClass();
- if (appInfo.hasAnyInstantiatedLambdas(holder)) {
- result.add(resolvedMethod);
- }
- }
+ addIfDefaultMethodWithLambdaInstantiations(appInfo, resolvedMethod, result);
DexMethod method = resolvedMethod.method;
Consumer<DexEncodedMethod> addIfNotAbstract =
@@ -356,10 +351,8 @@
}
};
// Default methods are looked up when looking at a specific subtype that does not override
- // them.
- // Otherwise, we would look up default methods that are actually never used. However, we have
- // to
- // add bridge methods, otherwise we can remove a bridge that will be used.
+ // them. Otherwise, we would look up default methods that are actually never used. However, we
+ // have to add bridge methods, otherwise we can remove a bridge that will be used.
Consumer<DexEncodedMethod> addIfNotAbstractAndBridge =
m -> {
if (!m.accessFlags.isAbstract() && m.accessFlags.isBridge()) {
@@ -374,7 +367,11 @@
if (clazz.isInterface()) {
ResolutionResult targetMethods = appInfo.resolveMethodOnInterface(clazz, method);
if (targetMethods.isSingleResolution()) {
- addIfNotAbstractAndBridge.accept(targetMethods.getSingleTarget());
+ // Sub-interfaces can have default implementations that override the resolved method.
+ // Therefore we have to add default methods in sub interfaces.
+ DexEncodedMethod singleTarget = targetMethods.getSingleTarget();
+ addIfDefaultMethodWithLambdaInstantiations(appInfo, singleTarget, result);
+ addIfNotAbstractAndBridge.accept(singleTarget);
}
} else {
ResolutionResult targetMethods = appInfo.resolveMethodOnClass(clazz, method);
@@ -385,6 +382,19 @@
}
return result;
}
+
+ private void addIfDefaultMethodWithLambdaInstantiations(
+ AppInfoWithSubtyping appInfo, DexEncodedMethod method, Set<DexEncodedMethod> result) {
+ if (method == null) {
+ return;
+ }
+ if (method.hasCode()) {
+ DexProgramClass holder = appInfo.definitionForProgramType(method.method.holder);
+ if (appInfo.hasAnyInstantiatedLambdas(holder)) {
+ result.add(method);
+ }
+ }
+ }
}
abstract static class EmptyResult extends ResolutionResult {
diff --git a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
index 6c097a4..a1789b0 100644
--- a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.desugar.LambdaDescriptor;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.DisjointSets;
import com.android.tools.r8.utils.MethodJavaSignatureEquivalence;
@@ -241,6 +242,7 @@
private final Set<DexCallSite> callSites = new HashSet<>();
private final Map<DexMethod, Set<InterfaceReservationState>> methodStates = new HashMap<>();
+ private final List<DexMethod> callSiteCollidingMethods = new ArrayList<>();
void addState(DexMethod method, InterfaceReservationState interfaceState) {
methodStates.computeIfAbsent(method, m -> new HashSet<>()).add(interfaceState);
@@ -248,6 +250,7 @@
void appendMethodGroupState(InterfaceMethodGroupState state) {
callSites.addAll(state.callSites);
+ callSiteCollidingMethods.addAll(state.callSiteCollidingMethods);
for (DexMethod key : state.methodStates.keySet()) {
methodStates.computeIfAbsent(key, k -> new HashSet<>()).addAll(state.methodStates.get(key));
}
@@ -460,6 +463,37 @@
groupState.addCallSite(callSite);
callSiteMethods.add(wrapped);
}
+ if (callSiteMethods.isEmpty()) {
+ return;
+ }
+ // For intersection types, we have to iterate all the multiple interfaces to look for
+ // methods with the same signature.
+ List<DexType> implementedInterfaces =
+ LambdaDescriptor.getInterfaces(callSite, appView.appInfo());
+ if (implementedInterfaces != null) {
+ for (int i = 1; i < implementedInterfaces.size(); i++) {
+ // Add the merging state for all additional implemented interfaces into the state
+ // for the group, if the name is different, to ensure that we do not pick the same
+ // name.
+ DexClass iface = appView.definitionFor(implementedInterfaces.get(i));
+ assert iface.isInterface();
+ for (DexEncodedMethod implementedMethod : implementedMethods) {
+ for (DexEncodedMethod virtualMethod : iface.virtualMethods()) {
+ boolean differentName =
+ !implementedMethod.method.name.equals(virtualMethod.method.name);
+ if (differentName
+ && MethodJavaSignatureEquivalence.getEquivalenceIgnoreName()
+ .equivalent(implementedMethod.method, virtualMethod.method)) {
+ InterfaceMethodGroupState interfaceMethodGroupState =
+ globalStateMap.computeIfAbsent(
+ equivalence.wrap(implementedMethod.method),
+ k -> new InterfaceMethodGroupState());
+ interfaceMethodGroupState.callSiteCollidingMethods.add(virtualMethod.method);
+ }
+ }
+ }
+ }
+ }
if (callSiteMethods.size() > 1) {
// Implemented interfaces have different protos. Unify them.
Wrapper<DexMethod> mainKey = callSiteMethods.iterator().next();
@@ -550,6 +584,27 @@
callSiteRenamings.put(callSite, newName);
}
}
+
+ // After all naming is completed for callsites, we must ensure to rename all interface methods
+ // that can collide with the callsite method name.
+ for (Wrapper<DexMethod> interfaceMethodGroup : nonReservedMethodGroups) {
+ InterfaceMethodGroupState groupState = globalStateMap.get(interfaceMethodGroup);
+ if (groupState.callSiteCollidingMethods.isEmpty()) {
+ continue;
+ }
+ DexMethod key = interfaceMethodGroup.get();
+ MethodNamingState<?> keyNamingState = minifierState.getNamingState(key.holder);
+ DexString existingRenaming = keyNamingState.newOrReservedNameFor(key);
+ assert existingRenaming != null;
+ for (DexMethod collidingMethod : groupState.callSiteCollidingMethods) {
+ DexString newNameInGroup = newNameInGroup(collidingMethod, keyNamingState, groupState);
+ minifierState.putRenaming(collidingMethod, newNameInGroup);
+ MethodNamingState<?> methodNamingState =
+ minifierState.getNamingState(collidingMethod.holder);
+ methodNamingState.addRenaming(newNameInGroup, collidingMethod);
+ keyNamingState.addRenaming(newNameInGroup, collidingMethod);
+ }
+ }
timing.end();
timing.end(); // end compute timing
@@ -568,6 +623,12 @@
return newName;
}
+ private DexString newNameInGroup(
+ DexMethod method, MethodNamingState<?> namingState, InterfaceMethodGroupState groupState) {
+ // Check if the name is available in all states.
+ return namingState.nextName(method, (candidate, ignore) -> groupState.isAvailable(candidate));
+ }
+
private void patchUpChildrenInReservationStates() {
for (Map.Entry<DexType, InterfaceReservationState> entry : interfaceStateMap.entrySet()) {
for (DexType parent : entry.getValue().iface.interfaces.values) {
diff --git a/src/main/java/com/android/tools/r8/naming/MethodNamingState.java b/src/main/java/com/android/tools/r8/naming/MethodNamingState.java
index 7926de0..85c2f0f 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNamingState.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNamingState.java
@@ -62,8 +62,12 @@
return candidate;
}
}
+ return nextName(method, isAvailable);
+ }
+
+ DexString nextName(DexMethod method, BiPredicate<DexString, DexMethod> isAvailable) {
InternalNewNameState internalState = getOrCreateInternalState(method);
- newName = namingStrategy.next(method, internalState, isAvailable);
+ DexString newName = namingStrategy.next(method, internalState, isAvailable);
assert newName != null;
return newName;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 1b76214..6d418cd 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2058,6 +2058,7 @@
return;
}
+ // TODO(mkroghj): Remove pinnedItems check here.
if (instantiatedTypes.contains(clazz)
|| instantiatedInterfaceTypes.contains(clazz)
|| pinnedItems.contains(clazz.type)) {
@@ -2076,8 +2077,8 @@
if (currentClass == null || currentClass.lookupVirtualMethod(possibleTarget) != null) {
continue;
}
- // TODO(zerny): Why does not not confer with lambdas and pinned too?
- if (instantiatedTypes.contains(currentClass)) {
+ if (instantiatedTypes.contains(currentClass)
+ || instantiatedInterfaceTypes.contains(currentClass)) {
markVirtualMethodAsLive(
clazz,
encodedPossibleTarget,
diff --git a/src/test/java/com/android/tools/r8/naming/IntersectionLambdaTest.java b/src/test/java/com/android/tools/r8/naming/IntersectionLambdaTest.java
new file mode 100644
index 0000000..9a55cee
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/IntersectionLambdaTest.java
@@ -0,0 +1,84 @@
+// Copyright (c) 2020, 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.naming;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+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 IntersectionLambdaTest extends TestBase {
+
+ private static final String[] EXPECTED = new String[] {"Lambda.foo", "J.bar", "K.baz"};
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public IntersectionLambdaTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testRuntime() throws IOException, CompilationFailedException, ExecutionException {
+ testForRuntime(parameters)
+ .addInnerClasses(IntersectionLambdaTest.class)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
+ }
+
+ @Test
+ public void testR8() throws IOException, CompilationFailedException, ExecutionException {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(IntersectionLambdaTest.class)
+ .enableMergeAnnotations()
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
+ }
+
+ @FunctionalInterface
+ @NeverMerge
+ public interface I {
+ void foo();
+ }
+
+ public interface J {
+ default void bar() {
+ System.out.println("J.bar");
+ }
+ }
+
+ public interface K {
+ default void baz() {
+ System.out.println("K.baz");
+ }
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ callI((I & J & K) () -> System.out.println("Lambda.foo"));
+ }
+
+ private static void callI(I i) {
+ i.foo();
+ ((J) i).bar();
+ ((K) i).baz();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/IntersectionWithInheritanceLambdaTest.java b/src/test/java/com/android/tools/r8/naming/IntersectionWithInheritanceLambdaTest.java
new file mode 100644
index 0000000..f784a8f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/IntersectionWithInheritanceLambdaTest.java
@@ -0,0 +1,78 @@
+// Copyright (c) 2020, 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.naming;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+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 IntersectionWithInheritanceLambdaTest extends TestBase {
+
+ private static final String[] EXPECTED = new String[] {"Lambda.foo", "J.bar", "K.baz"};
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public IntersectionWithInheritanceLambdaTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testRuntime() throws IOException, CompilationFailedException, ExecutionException {
+ testForRuntime(parameters)
+ .addInnerClasses(IntersectionWithInheritanceLambdaTest.class)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
+ }
+
+ @Test
+ public void testR8() throws IOException, CompilationFailedException, ExecutionException {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(IntersectionWithInheritanceLambdaTest.class)
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
+ }
+
+ public interface J {
+ default void bar() {
+ System.out.println("J.bar");
+ }
+ }
+
+ public interface K extends J {
+ void foo();
+
+ default void baz() {
+ System.out.println("K.baz");
+ }
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ callFooBarBaz(() -> System.out.println("Lambda.foo"));
+ }
+
+ private static void callFooBarBaz(K k) {
+ k.foo();
+ k.bar();
+ k.baz();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/resolution/interfacetargets/DefaultMethodAsOverrideWithLambdaTest.java b/src/test/java/com/android/tools/r8/resolution/interfacetargets/DefaultMethodAsOverrideWithLambdaTest.java
new file mode 100644
index 0000000..94fc918
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/resolution/interfacetargets/DefaultMethodAsOverrideWithLambdaTest.java
@@ -0,0 +1,108 @@
+// Copyright (c) 2020, 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.resolution.interfacetargets;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+/** This is a reproduction of b/148168065 */
+@RunWith(Parameterized.class)
+public class DefaultMethodAsOverrideWithLambdaTest extends TestBase {
+
+ private static final String[] EXPECTED = new String[] {"Lambda.foo", "J.bar"};
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public DefaultMethodAsOverrideWithLambdaTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testRuntime() throws IOException, CompilationFailedException, ExecutionException {
+ testForRuntime(parameters)
+ .addInnerClasses(DefaultMethodAsOverrideWithLambdaTest.class)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
+ }
+
+ @Test
+ public void testR8() throws IOException, CompilationFailedException, ExecutionException {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(DefaultMethodAsOverrideWithLambdaTest.class)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
+ }
+
+ @FunctionalInterface
+ @NeverMerge
+ public interface I {
+ void foo();
+
+ default void bar() {
+ System.out.println("I.bar");
+ }
+ }
+
+ @NeverMerge
+ public interface J extends I {
+
+ @Override
+ default void bar() {
+ System.out.println("J.bar");
+ }
+ }
+
+ @NeverClassInline
+ public static class A implements I {
+
+ @Override
+ @NeverInline
+ public void foo() {
+ System.out.println("A.foo");
+ }
+
+ @Override
+ public void bar() {
+ System.out.println("A.bar");
+ }
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ if (args.length == 0) {
+ callI((J) () -> System.out.println("Lambda.foo"));
+ } else {
+ callI(new A());
+ }
+ }
+
+ private static void callI(I i) {
+ i.foo();
+ i.bar();
+ }
+ }
+}
