Reland "Remove redundant abstract method bridges"
This reverts commit 62b2b4b74d258e0ff4acf32e371e8dba5076f591.
Change-Id: I5b2933b47573d2de2bc639aba02b3c908a016b31
diff --git a/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
index 8847f5f..5cbb1b6 100644
--- a/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/RedundantBridgeRemover.java
@@ -10,12 +10,17 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.MethodResolutionResult;
+import com.android.tools.r8.graph.MethodResolutionResult.FailedResolutionResult;
+import com.android.tools.r8.graph.MethodResolutionResult.SingleResolutionResult;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.ir.optimize.info.bridge.BridgeInfo;
import com.android.tools.r8.optimize.InvokeSingleTargetExtractor.InvokeKind;
import com.android.tools.r8.optimize.redundantbridgeremoval.RedundantBridgeRemovalLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.KeepMethodInfo;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -31,12 +36,7 @@
}
private DexClassAndMethod getTargetForRedundantBridge(ProgramMethod method) {
- // Clean-up the predicate check.
- if (appView.appInfo().isPinned(method.getReference())) {
- return null;
- }
DexEncodedMethod definition = method.getDefinition();
- // TODO(b/197490164): Remove if method is abstract.
BridgeInfo bridgeInfo = definition.getOptimizationInfo().getBridgeInfo();
boolean isBridge = definition.isBridge() || bridgeInfo != null;
if (!isBridge || definition.isAbstract()) {
@@ -140,6 +140,16 @@
ProgramMethodSet bridgesToRemoveForClass = ProgramMethodSet.create();
clazz.forEachProgramMethod(
method -> {
+ KeepMethodInfo keepInfo = appView.getKeepInfo(method);
+ if (!keepInfo.isShrinkingAllowed(appView.options())
+ || !keepInfo.isOptimizationAllowed(appView.options())) {
+ return;
+ }
+ if (isRedundantAbstractBridge(method)) {
+ // Record that the redundant bridge should be removed.
+ bridgesToRemoveForClass.add(method);
+ return;
+ }
DexClassAndMethod target = getTargetForRedundantBridge(method);
if (target != null) {
// Record that the redundant bridge should be removed.
@@ -167,6 +177,55 @@
return bridgesToRemove;
}
+ private boolean isRedundantAbstractBridge(ProgramMethod method) {
+ if (!method.getAccessFlags().isAbstract() || method.getDefinition().getCode() != null) {
+ return false;
+ }
+ DexProgramClass holder = method.getHolder();
+ if (holder.getSuperType() == null) {
+ assert holder.getType() == appView.dexItemFactory().objectType;
+ return false;
+ }
+ MethodResolutionResult superTypeResolution =
+ appView.appInfo().resolveMethodOn(holder.getSuperType(), method.getReference(), false);
+ if (superTypeResolution.isMultiMethodResolutionResult()) {
+ return false;
+ }
+ // Check if there is a definition in the super type hieararchy that is also abstract and has the
+ // same visibility.
+ if (superTypeResolution.isSingleResolution()) {
+ DexClassAndMethod resolutionPair =
+ superTypeResolution.asSingleResolution().getResolutionPair();
+ return resolutionPair.getDefinition().isAbstract()
+ && resolutionPair
+ .getDefinition()
+ .isAtLeastAsVisibleAsOtherInSameHierarchy(method.getDefinition(), appView)
+ && (!resolutionPair.getHolder().isInterface() || holder.getInterfaces().isEmpty());
+ }
+ // Only check for interfaces if resolving the method on super type causes NoSuchMethodError.
+ FailedResolutionResult failedResolutionResult = superTypeResolution.asFailedResolution();
+ if (failedResolutionResult == null
+ || !failedResolutionResult.isNoSuchMethodErrorResult(holder, appView.appInfo())
+ || holder.getInterfaces().isEmpty()) {
+ return false;
+ }
+ for (DexType iface : holder.getInterfaces()) {
+ SingleResolutionResult<?> singleIfaceResult =
+ appView
+ .appInfo()
+ .resolveMethodOn(iface, method.getReference(), true)
+ .asSingleResolution();
+ if (singleIfaceResult == null
+ || !singleIfaceResult.getResolvedMethod().isAbstract()
+ || !singleIfaceResult
+ .getResolvedMethod()
+ .isAtLeastAsVisibleAsOtherInSameHierarchy(method.getDefinition(), appView)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
private void pruneApp(
Map<DexProgramClass, ProgramMethodSet> bridgesToRemove, ExecutorService executorService)
throws ExecutionException {
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeTest.java b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeTest.java
index d569899..6d96680 100644
--- a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeTest.java
+++ b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeTest.java
@@ -5,9 +5,8 @@
package com.android.tools.r8.memberrebinding;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isSynthetic;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoHorizontalClassMerging;
@@ -16,7 +15,6 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
-import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
import com.android.tools.r8.utils.codeinspector.HorizontallyMergedClassesInspector;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -58,10 +56,7 @@
inspector -> {
ClassSubject clazz = inspector.clazz(J.class);
assertThat(clazz, isPresent());
- // TODO(b/197490164): We should remove the bridge inserted here.
- assertEquals(1, clazz.allMethods().size());
- FoundMethodSubject foundMethodSubject = clazz.allMethods().get(0);
- assertThat(foundMethodSubject, isSynthetic());
+ assertTrue(clazz.allMethods().isEmpty());
});
}
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeWithSubTypeTest.java b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeWithSubTypeTest.java
index 4551dbe..dd2f0c1 100644
--- a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeWithSubTypeTest.java
+++ b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingRemoveInterfaceBridgeWithSubTypeTest.java
@@ -5,9 +5,8 @@
package com.android.tools.r8.memberrebinding;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isSynthetic;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoHorizontalClassMerging;
@@ -18,7 +17,6 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
-import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
import com.android.tools.r8.utils.codeinspector.HorizontallyMergedClassesInspector;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -63,10 +61,7 @@
inspector -> {
ClassSubject clazz = inspector.clazz(A.class);
assertThat(clazz, isPresent());
- // TODO(b/197490164): We should remove the bridge inserted here.
- assertEquals(1, clazz.virtualMethods().size());
- FoundMethodSubject foundMethodSubject = clazz.virtualMethods().get(0);
- assertThat(foundMethodSubject, isSynthetic());
+ assertTrue(clazz.virtualMethods().isEmpty());
});
}
