Move testNoIllegalClassAccessWithAccessModificationsTest() to NoIllegalClassAccessWithAccessModificationsTest
Change-Id: Ifb04d4bf4ceda0e5e81e87700ade8ffcf9a1a304
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/NoIllegalClassAccessWithAccessModificationsTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/NoIllegalClassAccessWithAccessModificationsTest.java
new file mode 100644
index 0000000..a0e15f7
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/NoIllegalClassAccessWithAccessModificationsTest.java
@@ -0,0 +1,116 @@
+// 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.classmerging.vertical;
+
+import static com.android.tools.r8.classmerging.vertical.testclasses.NoIllegalClassAccessWithAccessModificationsTestClasses.getSimpleInterfaceImplClass;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.classmerging.vertical.testclasses.NoIllegalClassAccessWithAccessModificationsTestClasses;
+import com.android.tools.r8.classmerging.vertical.testclasses.NoIllegalClassAccessWithAccessModificationsTestClasses.SimpleInterfaceFactory;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.classmerging.VerticallyMergedClasses;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+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 NoIllegalClassAccessWithAccessModificationsTest extends VerticalClassMergerTestBase {
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return TestBase.getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public NoIllegalClassAccessWithAccessModificationsTest(TestParameters parameters) {
+ super(parameters);
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(NoIllegalClassAccessWithAccessModificationsTest.class)
+ .addInnerClasses(NoIllegalClassAccessWithAccessModificationsTestClasses.class)
+ .addKeepMainRule(TestClass.class)
+ .addVerticallyMergedClassesInspector(this::inspectVerticallyMergedClasses)
+ .allowAccessModification()
+ .enableNoHorizontalClassMergingAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccess();
+ }
+
+ private void inspectVerticallyMergedClasses(
+ DexItemFactory dexItemFactory, VerticallyMergedClasses verticallyMergedClasses) {
+ assertMergedIntoSubtype(SimpleInterface.class, dexItemFactory, verticallyMergedClasses);
+ assertMergedIntoSubtype(OtherSimpleInterface.class, dexItemFactory, verticallyMergedClasses);
+ }
+
+ private void inspect(CodeInspector inspector) {
+ assertThat(inspector.clazz(TestClass.class), isPresent());
+ assertThat(inspector.clazz(getSimpleInterfaceImplClass()), isPresent());
+ assertThat(inspector.clazz(OtherSimpleInterfaceImpl.class), isPresent());
+ assertThat(inspector.clazz(SimpleInterface.class), not(isPresent()));
+ assertThat(inspector.clazz(OtherSimpleInterface.class), not(isPresent()));
+ }
+
+ public static class TestClass {
+
+ public static void main(String[] args) {
+ // Without access modifications, it is not possible to merge the interface SimpleInterface
+ // into
+ // SimpleInterfaceImpl, since this would lead to an illegal class access here.
+ SimpleInterface x = SimpleInterfaceFactory.create();
+ x.foo();
+
+ // Without access modifications, it is not possible to merge the interface
+ // OtherSimpleInterface
+ // into OtherSimpleInterfaceImpl, since this could lead to an illegal class access if another
+ // package references OtherSimpleInterface.
+ OtherSimpleInterface y = new OtherSimpleInterfaceImpl();
+ y.bar();
+
+ // Ensure that the instantiations are not dead code eliminated.
+ escape(x);
+ escape(y);
+ }
+
+ @NeverInline
+ static void escape(Object o) {
+ if (System.currentTimeMillis() < 0) {
+ System.out.println(o);
+ }
+ }
+ }
+
+ // Should only be merged into OtherSimpleInterfaceImpl if access modifications are allowed.
+ public interface SimpleInterface {
+
+ void foo();
+ }
+
+ // Should only be merged into OtherSimpleInterfaceImpl if access modifications are allowed.
+ public interface OtherSimpleInterface {
+
+ void bar();
+ }
+
+ private static class OtherSimpleInterfaceImpl implements OtherSimpleInterface {
+
+ @Override
+ public void bar() {
+ System.out.println("In bar on OtherSimpleInterfaceImpl");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
index cbda594..4a3cc91 100644
--- a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
@@ -42,7 +42,6 @@
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Streams;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
@@ -52,7 +51,6 @@
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Predicate;
-import java.util.stream.IntStream;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -1112,42 +1110,6 @@
preservedClassNames::contains);
}
- @Test
- public void testNoIllegalClassAccessWithAccessModifications() throws Throwable {
- expectThrowsWithHorizontalClassMerging();
- // If access modifications are allowed then SimpleInterface should be merged into
- // SimpleInterfaceImpl.
- String main = "classmerging.SimpleInterfaceAccessTest";
- Path[] programFiles =
- new Path[] {
- CF_DIR.resolve("SimpleInterfaceAccessTest.class"),
- CF_DIR.resolve("SimpleInterfaceAccessTest$SimpleInterface.class"),
- CF_DIR.resolve("SimpleInterfaceAccessTest$OtherSimpleInterface.class"),
- CF_DIR.resolve("SimpleInterfaceAccessTest$OtherSimpleInterfaceImpl.class"),
- CF_DIR.resolve("pkg/SimpleInterfaceImplRetriever.class"),
- CF_DIR.resolve("pkg/SimpleInterfaceImplRetriever$SimpleInterfaceImpl.class")
- };
- ImmutableSet<String> preservedClassNames =
- ImmutableSet.of(
- "classmerging.SimpleInterfaceAccessTest",
- "classmerging.SimpleInterfaceAccessTest$OtherSimpleInterfaceImpl",
- "classmerging.pkg.SimpleInterfaceImplRetriever",
- "classmerging.pkg.SimpleInterfaceImplRetriever$SimpleInterfaceImpl");
- // Allow access modifications (and prevent SimpleInterfaceImplRetriever from being removed as
- // a result of inlining).
- runTest(
- testForR8(parameters.getBackend())
- .addKeepRules(
- getProguardConfig(
- EXAMPLE_KEEP,
- "-allowaccessmodification",
- "-keep public class classmerging.pkg.SimpleInterfaceImplRetriever"))
- .allowUnusedProguardConfigurationRules(),
- main,
- programFiles,
- preservedClassNames::contains);
- }
-
// TODO(christofferqa): This test checks that the invoke-super instruction in B is not rewritten
// into an invoke-direct instruction after it gets merged into class C. We should add a test that
// checks that this works with and without inlining of the method B.m().
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/testclasses/NoIllegalClassAccessWithAccessModificationsTestClasses.java b/src/test/java/com/android/tools/r8/classmerging/vertical/testclasses/NoIllegalClassAccessWithAccessModificationsTestClasses.java
new file mode 100644
index 0000000..b16d11b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/testclasses/NoIllegalClassAccessWithAccessModificationsTestClasses.java
@@ -0,0 +1,34 @@
+// 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.classmerging.vertical.testclasses;
+
+import com.android.tools.r8.NoHorizontalClassMerging;
+import com.android.tools.r8.classmerging.vertical.NoIllegalClassAccessWithAccessModificationsTest.SimpleInterface;
+
+public class NoIllegalClassAccessWithAccessModificationsTestClasses {
+
+ public static Class<?> getSimpleInterfaceImplClass() {
+ return SimpleInterfaceImpl.class;
+ }
+
+ public static class SimpleInterfaceFactory {
+
+ public static SimpleInterface create() {
+ return new SimpleInterfaceImpl();
+ }
+ }
+
+ // This class is intentionally marked private. It is not possible to merge the interface
+ // SimpleInterface into SimpleInterfaceImpl, since this would lead to an illegal class access
+ // in SimpleInterfaceAccessTest.
+ @NoHorizontalClassMerging
+ private static class SimpleInterfaceImpl implements SimpleInterface {
+
+ @Override
+ public void foo() {
+ System.out.println("In foo on SimpleInterfaceImpl");
+ }
+ }
+}
