Introduce relaxed method signature equivalence.
This equivalence is in between MethodJavaSignatureEquivalence
and MethodSignatureEquivalence: it is stronger than the former in that
it considers the return type of methods; it is weaker than the latter
in that it allows covariant return type: a method in a sub type can
return a narrower type.
Method minifier uses this equivalence for non aggressive renaming,
which still satisfies multi interfaces and lambda renaming that
require fine-grained method wrapping as well as satisfies the case
with covariant return type that requires less strict wrapping.
Bug: 112185748, 76461545
Change-Id: Ib462dbd81e598e4b11b7f787da4f5cfe3e3bca3b
diff --git a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
index c6729e8..97f1228 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.MethodSignatureEquivalence;
+import com.android.tools.r8.utils.MethodSignatureRelaxedEquivalence;
import com.android.tools.r8.utils.Timing;
import com.google.common.base.Equivalence;
import com.google.common.base.Equivalence.Wrapper;
@@ -90,11 +91,15 @@
*/
class MethodNameMinifier extends MemberNameMinifier<DexMethod, DexProto> {
- private final Equivalence<DexMethod> equivalence = MethodSignatureEquivalence.get();
+ private final Equivalence<DexMethod> equivalence;
private final Map<DexCallSite, DexString> callSiteRenaming = new IdentityHashMap<>();
MethodNameMinifier(AppInfoWithSubtyping appInfo, RootSet rootSet, InternalOptions options) {
super(appInfo, rootSet, options);
+ equivalence =
+ overloadAggressively
+ ? MethodSignatureEquivalence.get()
+ : new MethodSignatureRelaxedEquivalence(appInfo);
}
@Override
@@ -337,10 +342,12 @@
}
private void addStatesToGlobalMapForMethod(
- DexEncodedMethod method, Set<NamingState<DexProto, ?>> collectedStates,
+ DexEncodedMethod method,
+ Set<NamingState<DexProto, ?>> collectedStates,
Map<Wrapper<DexMethod>, Set<NamingState<DexProto, ?>>> globalStateMap,
Map<Wrapper<DexMethod>, Set<DexMethod>> sourceMethodsMap,
- Map<Wrapper<DexMethod>, NamingState<DexProto, ?>> originStates, DexType originInterface) {
+ Map<Wrapper<DexMethod>, NamingState<DexProto, ?>> originStates,
+ DexType originInterface) {
Wrapper<DexMethod> key = equivalence.wrap(method.method);
Set<NamingState<DexProto, ?>> stateSet =
globalStateMap.computeIfAbsent(key, k -> new HashSet<>());
diff --git a/src/main/java/com/android/tools/r8/utils/MethodSignatureRelaxedEquivalence.java b/src/main/java/com/android/tools/r8/utils/MethodSignatureRelaxedEquivalence.java
new file mode 100644
index 0000000..74d2d98
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/MethodSignatureRelaxedEquivalence.java
@@ -0,0 +1,36 @@
+// Copyright (c) 2018, 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.utils;
+
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.DexMethod;
+import com.google.common.base.Equivalence;
+
+/**
+ * Implements an equivalence on {@link DexMethod} that does not take the holder into account but
+ * allow covariant return type: a method in a sub type can return a narrower type.
+ */
+public class MethodSignatureRelaxedEquivalence extends Equivalence<DexMethod> {
+
+ private final AppInfo appInfo;
+
+ public MethodSignatureRelaxedEquivalence(AppInfo appInfo) {
+ this.appInfo = appInfo;
+ }
+
+ @Override
+ protected boolean doEquivalent(DexMethod a, DexMethod b) {
+ return a.name.equals(b.name) && a.proto.parameters.equals(b.proto.parameters)
+ && (a.proto.returnType.equals(b.proto.returnType)
+ || (a.proto.returnType.isStrictSubtypeOf(b.proto.returnType, appInfo)
+ && a.getHolder().isStrictSubtypeOf(b.getHolder(), appInfo))
+ || (b.proto.returnType.isStrictSubtypeOf(a.proto.returnType, appInfo)
+ && b.getHolder().isStrictSubtypeOf(a.getHolder(), appInfo)));
+ }
+
+ @Override
+ protected int doHash(DexMethod method) {
+ return method.name.hashCode() * 31 + method.proto.parameters.hashCode();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/CovariantReturnTypeInSubInterfaceTest.java b/src/test/java/com/android/tools/r8/naming/CovariantReturnTypeInSubInterfaceTest.java
index 6602d0c..20f46b4 100644
--- a/src/test/java/com/android/tools/r8/naming/CovariantReturnTypeInSubInterfaceTest.java
+++ b/src/test/java/com/android/tools/r8/naming/CovariantReturnTypeInSubInterfaceTest.java
@@ -7,15 +7,20 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
+import com.android.tools.r8.OutputMode;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.VmTestRunner;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
import java.util.List;
-import org.junit.Ignore;
import org.junit.Test;
+import org.junit.runner.RunWith;
interface SuperInterface {
Super foo();
@@ -42,14 +47,14 @@
class SuperImplementer implements SuperInterface {
@Override
public Super foo() {
- return new Super();
+ return new Sub();
}
}
-class SubImplementer implements SubInterface {
+class SubImplementer extends SuperImplementer implements SubInterface {
@Override
public Sub foo() {
- return new Sub();
+ return (Sub) super.foo();
}
}
@@ -57,19 +62,22 @@
public static void main(String[] args) {
SubImplementer subImplementer = new SubImplementer();
Super sup = subImplementer.foo();
- System.out.println(sup.bar());
+ System.out.print(sup.bar());
}
}
+@RunWith(VmTestRunner.class)
public class CovariantReturnTypeInSubInterfaceTest extends TestBase {
- @Ignore("b/112185748")
- @Test
- public void test() throws Exception {
+ private void test(boolean overloadAggressively) throws Exception {
+ String mainName = TestMain.class.getCanonicalName();
+ String aggressive =
+ overloadAggressively ? "-overloadaggressively" : "# Not overload aggressively";
List<String> config = ImmutableList.of(
"-printmapping",
"-useuniqueclassmembernames",
- "-keep class " + TestMain.class.getCanonicalName() + " {",
+ aggressive,
+ "-keep class " + mainName + " {",
" public void main(...);",
"}",
"-keep,allowobfuscation class **.Super* {",
@@ -88,7 +96,10 @@
SubImplementer.class,
TestMain.class
);
- AndroidApp processedApp = compileWithR8(app, String.join(System.lineSeparator(), config));
+ AndroidApp processedApp =
+ compileWithR8(app, String.join(System.lineSeparator(), config), options -> {
+ options.enableInlining = false;
+ });
CodeInspector inspector = new CodeInspector(processedApp);
ClassSubject superInterface = inspector.clazz(SuperInterface.class);
assertThat(superInterface, isRenamed());
@@ -101,6 +112,24 @@
Sub.class.getCanonicalName(), "foo", ImmutableList.of());
assertThat(foo2, isRenamed());
assertEquals(foo1.getFinalName(), foo2.getFinalName());
+
+ ProcessResult javaResult = ToolHelper.runJava(ToolHelper.getClassPathForTests(), mainName);
+ assertEquals(0, javaResult.exitCode);
+ Path outDex = temp.getRoot().toPath().resolve("dex.zip");
+ processedApp.writeToZip(outDex, OutputMode.DexIndexed);
+ ProcessResult artResult = ToolHelper.runArtNoVerificationErrorsRaw(outDex.toString(), mainName);
+ assertEquals(0, artResult.exitCode);
+ assertEquals(javaResult.stdout, artResult.stdout);
+ }
+
+ @Test
+ public void test_notAggressively() throws Exception {
+ test(false);
+ }
+
+ @Test
+ public void test_aggressively() throws Exception {
+ test(true);
}
}
diff --git a/src/test/java/com/android/tools/r8/naming/InterfaceRenamingTestRunner.java b/src/test/java/com/android/tools/r8/naming/InterfaceRenamingTestRunner.java
index e1fa3d0..ef6e0c8 100644
--- a/src/test/java/com/android/tools/r8/naming/InterfaceRenamingTestRunner.java
+++ b/src/test/java/com/android/tools/r8/naming/InterfaceRenamingTestRunner.java
@@ -14,12 +14,15 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.VmTestRunner;
import com.android.tools.r8.origin.Origin;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import org.junit.Test;
+import org.junit.runner.RunWith;
+@RunWith(VmTestRunner.class)
public class InterfaceRenamingTestRunner extends TestBase {
static final Class CLASS = InterfaceRenamingTest.class;
static final Class[] CLASSES = InterfaceRenamingTest.CLASSES;
diff --git a/src/test/java/com/android/tools/r8/naming/LambdaRenamingTestRunner.java b/src/test/java/com/android/tools/r8/naming/LambdaRenamingTestRunner.java
index 02bca3a..56dce3a 100644
--- a/src/test/java/com/android/tools/r8/naming/LambdaRenamingTestRunner.java
+++ b/src/test/java/com/android/tools/r8/naming/LambdaRenamingTestRunner.java
@@ -16,13 +16,16 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.VmTestRunner;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.FileUtils;
import java.io.IOException;
import java.nio.file.Path;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+@RunWith(VmTestRunner.class)
public class LambdaRenamingTestRunner extends TestBase {
static final Class CLASS = LambdaRenamingTest.class;
static final Class[] CLASSES = LambdaRenamingTest.CLASSES;
