Disable open interfaces analysis
Bug: 214496607
Change-Id: I26e1b740dd4bf7f59baa18d06782aa7c73defff1
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index 810874b..2e84b7a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -157,7 +157,8 @@
if (clazz == null) {
return false;
}
- if (clazz.isInterface() && appView.getOpenClosedInterfacesCollection().isMaybeOpen(clazz)) {
+ // TODO(b/214496607): Allow uninstantiated reasoning for closed interfaces.
+ if (clazz.isInterface()) {
return false;
}
return !appView.appInfo().isInstantiatedDirectlyOrIndirectly(clazz);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index acadf11..c91dd28 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -83,7 +83,6 @@
import com.android.tools.r8.optimize.argumentpropagation.ArgumentPropagator;
import com.android.tools.r8.optimize.argumentpropagation.ArgumentPropagatorIROptimizer;
import com.android.tools.r8.optimize.interfaces.analysis.OpenClosedInterfacesAnalysis;
-import com.android.tools.r8.optimize.interfaces.analysis.OpenClosedInterfacesAnalysisImpl;
import com.android.tools.r8.position.MethodPosition;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.KeepMethodInfo;
@@ -259,7 +258,8 @@
this.memberValuePropagation = new MemberValuePropagation(appViewWithLiveness);
this.methodOptimizationInfoCollector =
new MethodOptimizationInfoCollector(appViewWithLiveness, this);
- this.openClosedInterfacesAnalysis = new OpenClosedInterfacesAnalysisImpl(appViewWithLiveness);
+ // TODO(b/214496607): Enable open/closed interfaces analysis.
+ this.openClosedInterfacesAnalysis = OpenClosedInterfacesAnalysis.empty();
if (options.isMinifying()) {
this.identifierNameStringMarker = new IdentifierNameStringMarker(appViewWithLiveness);
} else {
diff --git a/src/main/java/com/android/tools/r8/optimize/interfaces/collection/DefaultOpenClosedInterfacesCollection.java b/src/main/java/com/android/tools/r8/optimize/interfaces/collection/DefaultOpenClosedInterfacesCollection.java
index df264e4..f8aeb81 100644
--- a/src/main/java/com/android/tools/r8/optimize/interfaces/collection/DefaultOpenClosedInterfacesCollection.java
+++ b/src/main/java/com/android/tools/r8/optimize/interfaces/collection/DefaultOpenClosedInterfacesCollection.java
@@ -22,7 +22,8 @@
@Override
public boolean isDefinitelyClosed(DexClass clazz) {
- return false;
+ // TODO(b/214496607): Should return false.
+ return true;
}
@Override
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/interfaces/CollisionWithDefaultMethodOutsideMergeGroupLambdaTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/interfaces/CollisionWithDefaultMethodOutsideMergeGroupLambdaTest.java
index 7081080..9fa9260 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/interfaces/CollisionWithDefaultMethodOutsideMergeGroupLambdaTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/interfaces/CollisionWithDefaultMethodOutsideMergeGroupLambdaTest.java
@@ -18,8 +18,8 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRuntime.CfVm;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
-import com.android.tools.r8.utils.codeinspector.HorizontallyMergedClassesInspector;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -46,7 +46,13 @@
// I and J are not eligible for merging, since the lambda that implements I & J inherits a
// default m() method from K, which is also on J.
.addHorizontallyMergedClassesInspector(
- HorizontallyMergedClassesInspector::assertNoClassesMerged)
+ inspector -> {
+ if (parameters.isCfRuntime()) {
+ inspector.assertIsCompleteMergeGroup(I.class, J.class);
+ } else {
+ inspector.assertNoClassesMerged();
+ }
+ })
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
.enableNoHorizontalClassMergingAnnotations()
@@ -60,13 +66,25 @@
ClassSubject aClassSubject = inspector.clazz(A.class);
if (parameters.canUseDefaultAndStaticInterfaceMethods()) {
assertThat(aClassSubject, isPresent());
- assertThat(aClassSubject, isImplementing(inspector.clazz(J.class)));
+ if (parameters.isCfRuntime()) {
+ assertThat(aClassSubject, isImplementing(inspector.clazz(I.class)));
+ } else {
+ assertThat(aClassSubject, isImplementing(inspector.clazz(J.class)));
+ }
} else {
assertThat(aClassSubject, isAbsent());
}
})
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("K", "J");
+ // TODO(b/173990042): Should succeed with "K", "J".
+ .applyIf(
+ parameters.isCfRuntime(),
+ builder ->
+ builder.assertFailureWithErrorThatThrows(
+ parameters.isCfRuntime(CfVm.JDK11)
+ ? AbstractMethodError.class
+ : IncompatibleClassChangeError.class),
+ builder -> builder.assertSuccessWithOutputLines("K", "J"));
}
static class Main {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceCheckCastTest.java b/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceCheckCastTest.java
index c756f8c..e8d4e5f 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceCheckCastTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceCheckCastTest.java
@@ -41,7 +41,7 @@
.addProgramClasses(getProgramClasses())
.addProgramClassFileData(getTransformedMainClass())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(false));
}
@Test
@@ -52,7 +52,7 @@
.addProgramClassFileData(getTransformedMainClass())
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(false));
}
@Test
@@ -68,7 +68,7 @@
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(true));
}
private List<Class<?>> getProgramClasses() {
@@ -102,7 +102,11 @@
.transform();
}
- private List<String> getExpectedOutputLines() {
+ private List<String> getExpectedOutputLines(boolean isR8) {
+ if (isR8) {
+ // TODO(b/214496607): R8 should not optimize the check-cast instruction since I is open.
+ return ImmutableList.of("OK", "OK");
+ }
if (parameters.isDexRuntime()
&& parameters
.getDexRuntimeVersion()
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceInliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceInliningTest.java
index 4eaa6a4..e2b4a29 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceInliningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceInliningTest.java
@@ -71,7 +71,8 @@
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ // TODO(b/214496607): Should succeed with the expected output.
+ .assertFailureWithErrorThatThrows(ClassCastException.class);
}
private List<Class<?>> getProgramClasses() {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceInstanceofTest.java b/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceInstanceofTest.java
index dac3848..5a57dc4 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceInstanceofTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenInterfaceInstanceofTest.java
@@ -41,7 +41,7 @@
.addProgramClasses(getProgramClasses())
.addProgramClassFileData(getTransformedMainClass())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(false));
}
@Test
@@ -52,7 +52,7 @@
.addProgramClassFileData(getTransformedMainClass())
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(false));
}
@Test
@@ -68,7 +68,7 @@
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(true));
}
private List<Class<?>> getProgramClasses() {
@@ -95,7 +95,11 @@
.transform();
}
- private List<String> getExpectedOutputLines() {
+ private List<String> getExpectedOutputLines(boolean isR8) {
+ if (isR8) {
+ // TODO(b/214496607): R8 should not optimize the instanceof instruction since I is open.
+ return ImmutableList.of("true", "true");
+ }
if (parameters.isDexRuntime() && parameters.getDexRuntimeVersion().isEqualTo(Version.V7_0_0)) {
return ImmutableList.of("true", "true");
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenUninstantiatedInterfaceInstanceofTest.java b/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenUninstantiatedInterfaceInstanceofTest.java
index 606871f..a729899 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenUninstantiatedInterfaceInstanceofTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/interfaces/OpenUninstantiatedInterfaceInstanceofTest.java
@@ -40,7 +40,7 @@
.addProgramClasses(getProgramClasses())
.addProgramClassFileData(getTransformedMainClass())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(false));
}
@Test
@@ -51,7 +51,7 @@
.addProgramClassFileData(getTransformedMainClass())
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(false));
}
@Test
@@ -65,7 +65,7 @@
.enableInliningAnnotations()
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(getExpectedOutputLines());
+ .assertSuccessWithOutputLines(getExpectedOutputLines(true));
}
private List<Class<?>> getProgramClasses() {
@@ -92,7 +92,11 @@
.transform();
}
- private List<String> getExpectedOutputLines() {
+ private List<String> getExpectedOutputLines(boolean isR8) {
+ if (isR8) {
+ // TODO(b/214496607): R8 should not optimize the instanceof instruction since I is open.
+ return ImmutableList.of("true");
+ }
if (parameters.isDexRuntime()) {
if (parameters.getDexRuntimeVersion().isEqualTo(Version.V7_0_0)
|| parameters.getDexRuntimeVersion().isEqualTo(Version.V13_MASTER)) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/lambda/LambdaMethodInliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/lambda/LambdaMethodInliningTest.java
index a3d1e6b..a63addb 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/lambda/LambdaMethodInliningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/lambda/LambdaMethodInliningTest.java
@@ -65,7 +65,7 @@
assertTrue(
testClassMethodSubject
.streamInstructions()
- .noneMatch(
+ .anyMatch(
instruction ->
instruction.isInvokeVirtual()
&& instruction.getMethod().toSourceString().contains("println")));
diff --git a/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java b/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
index c14b357..488271b 100644
--- a/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
@@ -73,9 +73,8 @@
.count();
long paramNullCheckCount =
countCall(testMethod, "Intrinsics", "checkParameterIsNotNull");
- // TODO(b/214496607): Should be one after Iterator#hasNext, and another in the filter
- // predicate: sinceYear != null.
- assertEquals(testParameters.isCfRuntime() ? 5 : 2, ifzCount);
+ // One after Iterator#hasNext, and another in the filter predicate: sinceYear != null.
+ assertEquals(2, ifzCount);
assertEquals(0, paramNullCheckCount);
});
}
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleTargetExecutionTest.java b/src/test/java/com/android/tools/r8/resolution/SingleTargetExecutionTest.java
index 1446e76..4726fe4 100644
--- a/src/test/java/com/android/tools/r8/resolution/SingleTargetExecutionTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/SingleTargetExecutionTest.java
@@ -84,6 +84,10 @@
}
private String getExpectedOutput() {
+ String icceOrNot =
+ enableInliningAnnotations || !parameters.canUseDefaultAndStaticInterfaceMethods()
+ ? "ICCE"
+ : "InterfaceWithDefault";
return StringUtils.lines(
"SubSubClassOne",
"SubSubClassOne",
@@ -93,7 +97,7 @@
"com.android.tools.r8.resolution.singletarget.one.AbstractSubClass",
"InterfaceWithDefault",
"InterfaceWithDefault",
- "ICCE",
+ icceOrNot,
"com.android.tools.r8.resolution.singletarget.one.SubSubClassTwo",
"com.android.tools.r8.resolution.singletarget.one.SubSubClassTwo",
"AbstractTopClass",
@@ -105,7 +109,7 @@
"InterfaceWithDefault",
"InterfaceWithDefault",
"InterfaceWithDefault",
- "ICCE",
+ icceOrNot,
"InterfaceWithDefault",
"com.android.tools.r8.resolution.singletarget.one.SubSubClassTwo",
"InterfaceWithDefault",
diff --git a/src/test/java/com/android/tools/r8/shaking/proxy/MockitoTest.java b/src/test/java/com/android/tools/r8/shaking/proxy/MockitoTest.java
index 1464e82..d31da79 100644
--- a/src/test/java/com/android/tools/r8/shaking/proxy/MockitoTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/proxy/MockitoTest.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking.proxy;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
import static org.hamcrest.MatcherAssert.assertThat;
@@ -57,11 +58,8 @@
.inspector();
ClassSubject itf = inspector.clazz(M_I);
assertThat(itf, isPresent());
- // TODO(b/214496607): This could be removed as a result of devirtualization, but this only
- // happens if the call site is reprocessed, since we don't have knowledge of open/closed
- // interfaces until the second optimization pass.
MethodSubject mtd = itf.uniqueMethodWithName("onEnterForeground");
- assertThat(mtd, isPresent());
+ assertThat(mtd, isAbsent());
}
@Test
diff --git a/src/test/java/com/android/tools/r8/shaking/proxy/ProxiesTest.java b/src/test/java/com/android/tools/r8/shaking/proxy/ProxiesTest.java
index c850ae8..047a109 100644
--- a/src/test/java/com/android/tools/r8/shaking/proxy/ProxiesTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/proxy/ProxiesTest.java
@@ -139,8 +139,8 @@
private void noInterfaceKept(CodeInspector inspector) {
// Indirectly assert that method is inlined into x, y and z and that redundant field loads
// remove invokes.
- assertEquals(3, countInstructionInX(inspector, InstructionSubject::isInvokeInterface));
- assertEquals(3, countInstructionInY(inspector, InstructionSubject::isInvokeInterface));
+ assertEquals(0, countInstructionInX(inspector, InstructionSubject::isInvokeInterface));
+ assertEquals(0, countInstructionInY(inspector, InstructionSubject::isInvokeInterface));
assertEquals(0, countInstructionInZ(inspector, InstructionSubject::isInvokeVirtual));
}
@@ -149,10 +149,7 @@
runTest(
ImmutableList.of(),
this::noInterfaceKept,
- "TestClass 1\nTestClass 1\nTestClass 1\nProxy\nProxy\nProxy\n"
- + "TestClass 2\nTestClass 2\nTestClass 2\nProxy\nProxy\nProxy\n"
- + "TestClass 3\nTestClass 3\nTestClass 3\n"
- + "TestClass 4\nTestClass 4\nTestClass 4\nSUCCESS\n");
+ "TestClass 1\nTestClass 1\nTestClass 1\nEXCEPTION\n");
}
private void baseInterfaceKept(CodeInspector inspector) {
@@ -160,7 +157,7 @@
assertEquals(3, countInstructionInX(inspector, InstructionSubject::isInvokeInterface));
// Indirectly assert that method is inlined into y and z and that redundant field loads
// remove invokes.
- assertEquals(3, countInstructionInY(inspector, InstructionSubject::isInvokeInterface));
+ assertEquals(0, countInstructionInY(inspector, InstructionSubject::isInvokeInterface));
assertEquals(0, countInstructionInZ(inspector, InstructionSubject::isInvokeVirtual));
assertEquals(0, countInstructionInZSubClass(inspector, InstructionSubject::isInvokeVirtual));
}
@@ -174,9 +171,7 @@
"}"),
this::baseInterfaceKept,
"TestClass 1\nTestClass 1\nTestClass 1\nProxy\nProxy\nProxy\n"
- + "TestClass 2\nTestClass 2\nTestClass 2\nProxy\nProxy\nProxy\n"
- + "TestClass 3\nTestClass 3\nTestClass 3\n"
- + "TestClass 4\nTestClass 4\nTestClass 4\nSUCCESS\n");
+ + "TestClass 2\nTestClass 2\nTestClass 2\nEXCEPTION\n");
}
private void subInterfaceKept(CodeInspector inspector) {
