Handle parameter type changes in Proguard -if rules
Bug: 110141157
Change-Id: I22b607fa459444752e17249c9c876027caf2ae52
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardMemberRule.java b/src/main/java/com/android/tools/r8/shaking/ProguardMemberRule.java
index 0a2e4f9..a84afb2 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardMemberRule.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardMemberRule.java
@@ -259,9 +259,7 @@
break;
}
for (int i = 0; i < parameters.length; i++) {
- // TODO(b/110141157): The names of the parameter types may have changed as a result of
- // vertical class merging. We should use the original type names.
- if (!arguments.get(i).matches(parameters[i])) {
+ if (!matches(arguments.get(i), parameters[i], appView)) {
return false;
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java
index 32fd8da..6205f85 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java
@@ -59,7 +59,6 @@
}
// TODO(b/110141157):
-// - Add tests where the parameter type of a kept method changes.
// - Add tests where the type of a kept field changes.
// - Add tests where fields and methods get renamed due to naming conflicts.
// - Add tests where the type in a implements/extends clause has changed.
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedParameterTypeTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedParameterTypeTest.java
new file mode 100644
index 0000000..4533cef
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedParameterTypeTest.java
@@ -0,0 +1,38 @@
+// 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.shaking.ifrule.verticalclassmerging;
+
+public class MergedParameterTypeTest extends MergedTypeBaseTest {
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ method(new B());
+ }
+
+ public static void method(A obj) {
+ System.out.print(obj.getClass().getTypeName());
+ }
+ }
+
+ public MergedParameterTypeTest(Backend backend, boolean enableVerticalClassMerging) {
+ super(backend, enableVerticalClassMerging);
+ }
+
+ @Override
+ public Class<?> getTestClass() {
+ return TestClass.class;
+ }
+
+ @Override
+ public String getConditionForProguardIfRule() {
+ return "-if class **$TestClass { void method(**$A); }";
+ }
+
+ @Override
+ public String getExpectedStdout() {
+ return B.class.getTypeName();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedReturnTypeTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedReturnTypeTest.java
index f5c8411..9f90bb1 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedReturnTypeTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedReturnTypeTest.java
@@ -4,47 +4,12 @@
package com.android.tools.r8.shaking.ifrule.verticalclassmerging;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
-
-import com.android.tools.r8.TestBase;
-import com.android.tools.r8.ir.optimize.Inliner.Reason;
-import com.android.tools.r8.utils.AndroidApp;
-import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.StringUtils;
-import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import java.util.Collection;
-import java.util.List;
-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 MergedReturnTypeTest extends TestBase {
-
- private static final List<Class> CLASSES =
- ImmutableList.of(A.class, B.class, C.class, TestClass.class);
-
- static class A {}
-
- static class B extends A {
-
- @Override
- public String toString() {
- return "B";
- }
- }
-
- static class C {}
+public class MergedReturnTypeTest extends MergedTypeBaseTest {
static class TestClass {
public static void main(String[] args) {
- System.out.print(method());
+ System.out.print(method().getClass().getTypeName());
}
public static A method() {
@@ -52,48 +17,22 @@
}
}
- private final Backend backend;
- private final boolean enableVerticalClassMerging;
-
public MergedReturnTypeTest(Backend backend, boolean enableVerticalClassMerging) {
- this.backend = backend;
- this.enableVerticalClassMerging = enableVerticalClassMerging;
+ super(backend, enableVerticalClassMerging);
}
- @Parameters(name = "Backend: {0}, vertical class merging: {1}")
- public static Collection<Object[]> data() {
- // We don't run this on Proguard, as Proguard does not merge A into B.
- return ImmutableList.of(
- new Object[] {Backend.DEX, true},
- new Object[] {Backend.DEX, false},
- new Object[] {Backend.CF, true},
- new Object[] {Backend.CF, false});
+ @Override
+ public Class<?> getTestClass() {
+ return TestClass.class;
}
- @Test
- public void testIfRule() throws Exception {
- String expected = "B";
- assertEquals(expected, runOnJava(TestClass.class));
-
- String config =
- StringUtils.joinLines(
- "-keep class **$TestClass {",
- " public static void main(java.lang.String[]);",
- "}",
- "-if class **$TestClass { public static **$A method(); }",
- "-keep class **$C");
- AndroidApp output = compileWithR8(readClasses(CLASSES), config, this::configure, backend);
- assertEquals(expected, runOnVM(output, TestClass.class, backend));
-
- CodeInspector inspector = new CodeInspector(output);
- assertThat(inspector.clazz(C.class), isPresent());
+ @Override
+ public String getConditionForProguardIfRule() {
+ return "-if class **$TestClass { **$A method(); }";
}
- private void configure(InternalOptions options) {
- options.enableMinification = false;
- options.enableVerticalClassMerging = enableVerticalClassMerging;
-
- // TODO(b/110148109): Allow ordinary method inlining when -if rules work with inlining.
- options.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE);
+ @Override
+ public String getExpectedStdout() {
+ return B.class.getTypeName();
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedTypeBaseTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedTypeBaseTest.java
new file mode 100644
index 0000000..853394f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedTypeBaseTest.java
@@ -0,0 +1,87 @@
+// 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.shaking.ifrule.verticalclassmerging;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ir.optimize.Inliner.Reason;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public abstract class MergedTypeBaseTest extends TestBase {
+
+ private final List<Class> CLASSES = ImmutableList.of(A.class, B.class, C.class, getTestClass());
+
+ static class A {}
+
+ static class B extends A {}
+
+ static class C {}
+
+ private final Backend backend;
+ private final boolean enableVerticalClassMerging;
+
+ public MergedTypeBaseTest(Backend backend, boolean enableVerticalClassMerging) {
+ this.backend = backend;
+ this.enableVerticalClassMerging = enableVerticalClassMerging;
+ }
+
+ @Parameters(name = "Backend: {0}, vertical class merging: {1}")
+ public static Collection<Object[]> data() {
+ // We don't run this on Proguard, as Proguard does not merge A into B.
+ return ImmutableList.of(
+ new Object[] {Backend.DEX, true},
+ new Object[] {Backend.DEX, false},
+ new Object[] {Backend.CF, true},
+ new Object[] {Backend.CF, false});
+ }
+
+ public abstract Class<?> getTestClass();
+
+ public abstract String getConditionForProguardIfRule();
+
+ public abstract String getExpectedStdout();
+
+ @Test
+ public void testIfRule() throws Exception {
+ String expected = getExpectedStdout();
+ assertEquals(expected, runOnJava(getTestClass()));
+
+ String config =
+ StringUtils.joinLines(
+ "-keep class " + getTestClass().getTypeName() + " {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ getConditionForProguardIfRule(),
+ "-keep class " + C.class.getTypeName());
+ AndroidApp output = compileWithR8(readClasses(CLASSES), config, this::configure, backend);
+ assertEquals(expected, runOnVM(output, getTestClass(), backend));
+
+ CodeInspector inspector = new CodeInspector(output);
+ assertThat(inspector.clazz(MergedReturnTypeTest.C.class), isPresent());
+ }
+
+ private void configure(InternalOptions options) {
+ options.enableMinification = false;
+ options.enableVerticalClassMerging = enableVerticalClassMerging;
+
+ // TODO(b/110148109): Allow ordinary method inlining when -if rules work with inlining.
+ options.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE);
+ }
+}
