Remove unneeded visibility bridge methods
Bug: 123832949
Change-Id: I661efcf1a271829ec89b35130531688d5b81ded6
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 5fa0f23..53b47bc 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
@@ -74,6 +75,7 @@
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.VersionProperties;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.io.ByteStreams;
import com.google.common.io.Closer;
@@ -666,6 +668,24 @@
return;
}
+ // Remove unneeeded visibility bridges that have been inserted for member rebinding.
+ // This can only be done if we have AppInfoWithLiveness.
+ if (appView.appInfo().hasLiveness()) {
+ ImmutableSet.Builder<DexMethod> unneededVisibilityBridgeMethods = ImmutableSet.builder();
+ new VisibilityBridgeRemover(
+ appView.withLiveness(),
+ unneededVisibilityBridgeMethod ->
+ unneededVisibilityBridgeMethods.add(unneededVisibilityBridgeMethod.method))
+ .run();
+ appView.setUnneededVisibilityBridgeMethods(unneededVisibilityBridgeMethods.build());
+ } else {
+ // If we don't have AppInfoWithLiveness here, it must be because we are not shrinking. When
+ // we are not shrinking, we can't move visibility bridges. In principle, though, it would be
+ // possible to remove visibility bridges that have been synthesized by R8, but we currently
+ // do not have this information.
+ assert !options.isShrinking();
+ }
+
// Validity checks.
assert application.classes().stream().allMatch(DexClass::isValid);
assert appView.rootSet().verifyKeptItemsAreKept(application, appView.appInfo());
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index 95e8ab3..a3c686e 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -9,6 +9,8 @@
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
import com.android.tools.r8.shaking.VerticalClassMerger.VerticallyMergedClasses;
import com.android.tools.r8.utils.InternalOptions;
+import com.google.common.collect.ImmutableSet;
+import java.util.Set;
public class AppView<T extends AppInfo> implements DexDefinitionSupplier {
@@ -24,6 +26,7 @@
private GraphLense graphLense;
private final InternalOptions options;
private RootSet rootSet;
+ private Set<DexMethod> unneededVisibilityBridgeMethods = ImmutableSet.of();
private VerticallyMergedClasses verticallyMergedClasses;
private AppView(
@@ -137,6 +140,14 @@
this.rootSet = rootSet;
}
+ public Set<DexMethod> unneededVisibilityBridgeMethods() {
+ return unneededVisibilityBridgeMethods;
+ }
+
+ public void setUnneededVisibilityBridgeMethods(Set<DexMethod> unneededVisibilityBridgeMethods) {
+ this.unneededVisibilityBridgeMethods = unneededVisibilityBridgeMethods;
+ }
+
// Get the result of vertical class merging. Returns null if vertical class merging has not been
// run.
public VerticallyMergedClasses verticallyMergedClasses() {
diff --git a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
index ebfd5c7..c996aee 100644
--- a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
+++ b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
@@ -156,7 +156,8 @@
|| (staticTarget != null && staticTarget.method == item)
|| (directTargetHolder != null && directTargetHolder.isNotProgramClass())
|| (virtualTargetHolder != null && virtualTargetHolder.isNotProgramClass())
- || (staticTargetHolder != null && staticTargetHolder.isNotProgramClass());
+ || (staticTargetHolder != null && staticTargetHolder.isNotProgramClass())
+ || appView.unneededVisibilityBridgeMethods().contains(item);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
index 80316e2..88260d4 100644
--- a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
@@ -14,13 +14,22 @@
import com.google.common.collect.Sets;
import java.util.List;
import java.util.Set;
+import java.util.function.Consumer;
public class VisibilityBridgeRemover {
private final AppView<AppInfoWithLiveness> appView;
+ private final Consumer<DexEncodedMethod> unneededVisibilityBridgeConsumer;
public VisibilityBridgeRemover(AppView<AppInfoWithLiveness> appView) {
+ this(appView, null);
+ }
+
+ public VisibilityBridgeRemover(
+ AppView<AppInfoWithLiveness> appView,
+ Consumer<DexEncodedMethod> unneededVisibilityBridgeConsumer) {
this.appView = appView;
+ this.unneededVisibilityBridgeConsumer = unneededVisibilityBridgeConsumer;
}
private void removeUnneededVisibilityBridgesFromClass(DexProgramClass clazz) {
@@ -42,6 +51,9 @@
methodsToBeRemoved = Sets.newIdentityHashSet();
}
methodsToBeRemoved.add(method);
+ if (unneededVisibilityBridgeConsumer != null) {
+ unneededVisibilityBridgeConsumer.accept(method);
+ }
}
}
if (methodsToBeRemoved != null) {
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingBridgeRemovalTest.java b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingBridgeRemovalTest.java
new file mode 100644
index 0000000..77a9ea9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingBridgeRemovalTest.java
@@ -0,0 +1,70 @@
+// Copyright (c) 2019, 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.memberrebinding;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isBridge;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isSynthetic;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.memberrebinding.testclasses.MemberRebindingBridgeRemovalTestClasses;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class MemberRebindingBridgeRemovalTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ public MemberRebindingBridgeRemovalTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(
+ MemberRebindingBridgeRemovalTest.class, MemberRebindingBridgeRemovalTestClasses.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(StringUtils.lines("Hello world!"));
+ }
+
+ private void inspect(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(MemberRebindingBridgeRemovalTestClasses.B.class);
+ assertThat(classSubject, isPresent());
+
+ for (FoundMethodSubject methodSubject : classSubject.allMethods()) {
+ assertThat(methodSubject, not(isBridge()));
+ assertThat(methodSubject, not(isSynthetic()));
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ new MemberRebindingBridgeRemovalTestClasses.B().m();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/testclasses/MemberRebindingBridgeRemovalTestClasses.java b/src/test/java/com/android/tools/r8/memberrebinding/testclasses/MemberRebindingBridgeRemovalTestClasses.java
new file mode 100644
index 0000000..3e15e6e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/memberrebinding/testclasses/MemberRebindingBridgeRemovalTestClasses.java
@@ -0,0 +1,22 @@
+// Copyright (c) 2019, 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.memberrebinding.testclasses;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+
+public class MemberRebindingBridgeRemovalTestClasses {
+
+ @NeverMerge
+ static class A {
+
+ @NeverInline
+ public void m() {
+ System.out.println("Hello world!");
+ }
+ }
+
+ public static class B extends A {}
+}
