Revert "Allow for minification of classes with references to missing classes"
This reverts commit 6db8c087faef79361fe76871f8afdf06bc5aeba0.
Reason for revert: Failing bots
Change-Id: Iacac4159fc2bd7edad647db14f3d71f4381fa2ea
diff --git a/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java b/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java
index 03627d7..62b62b9 100644
--- a/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java
@@ -66,10 +66,6 @@
return false;
}
- public boolean isFailedResolution() {
- return false;
- }
-
public DexClass getInitialResolutionHolder() {
return null;
}
@@ -167,11 +163,6 @@
public boolean isFailedOrUnknownResolution() {
return true;
}
-
- @Override
- public boolean isFailedResolution() {
- return true;
- }
}
/**
diff --git a/src/main/java/com/android/tools/r8/repackaging/RepackagingUseRegistry.java b/src/main/java/com/android/tools/r8/repackaging/RepackagingUseRegistry.java
index e81bd8b..27247d4 100644
--- a/src/main/java/com/android/tools/r8/repackaging/RepackagingUseRegistry.java
+++ b/src/main/java/com/android/tools/r8/repackaging/RepackagingUseRegistry.java
@@ -116,23 +116,11 @@
MemberResolutionResult<?, ?> resolutionResult, boolean isInvoke) {
if (!resolutionResult.isSuccessfulMemberResolutionResult()) {
// To preserve errors in the original program, we need to look at the failure dependencies.
- // For example, if this member accesses a package-private method in another package, and we
- // move the two members to the same package, then the invoke would no longer fail with an
+ // For example, if this method accesses in a package-private method in another package, and we
+ // move the two methods to the same package, then the invoke would no longer fail with an
// IllegalAccessError.
-
- // For fields and methods that cannot be found the chance of recovering by repackaging is
- // pretty slim thus we allow for repackaging the callers.
- boolean isMissing =
- (resolutionResult.isMethodResolutionResult()
- && resolutionResult
- .asMethodResolutionResult()
- .isNoSuchMethodErrorResult(context.getContextClass(), appInfo))
- || (resolutionResult.isFieldResolutionResult()
- && resolutionResult.asFieldResolutionResult().isFailedOrUnknownResolution());
- assert !isMissing
- || resolutionResult.isMethodResolutionResult()
- || !resolutionResult.asFieldResolutionResult().isFailedResolution();
- if (!isMissing) {
+ if (isInvoke) {
+ // TODO(b/150589374): Only add this if we are in the minification mode of repackaging.
node.addNeighbor(missingTypeNode);
}
return;
@@ -173,6 +161,9 @@
DexClass clazz = appInfo.definitionFor(type);
if (clazz != null) {
consumer.accept(clazz);
+ } else {
+ // The missing type reference can be package private and we cannot repackage.
+ node.addNeighbor(missingTypeNode);
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index be31b16..aef58b7 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -986,6 +986,11 @@
if (!keepInfo.getInfo(clazz).isRepackagingAllowed(clazz, options())) {
return false;
}
+ for (DexType superType : clazz.allImmediateSupertypes()) {
+ if (definitionFor(superType) == null) {
+ return false;
+ }
+ }
return clazz
.traverseProgramMembers(
member -> {
diff --git a/src/test/java/com/android/tools/r8/TestRunResult.java b/src/test/java/com/android/tools/r8/TestRunResult.java
index 90d4f75..23e2e5c 100644
--- a/src/test/java/com/android/tools/r8/TestRunResult.java
+++ b/src/test/java/com/android/tools/r8/TestRunResult.java
@@ -145,14 +145,6 @@
return assertFailure();
}
- public RR assertFailureWithErrorThatMatchesIf(boolean condition, Matcher<String> matcher) {
- if (condition) {
- assertStderrMatches(matcher);
- return assertFailure();
- }
- return self();
- }
-
public RR assertFailureWithOutput(String expected) {
assertStdoutMatches(is(expected));
return assertFailure();
diff --git a/src/test/java/com/android/tools/r8/desugar/LambdaMissingInterfaceTest.java b/src/test/java/com/android/tools/r8/desugar/LambdaMissingInterfaceTest.java
index 59853ee..bdcfd8a 100644
--- a/src/test/java/com/android/tools/r8/desugar/LambdaMissingInterfaceTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/LambdaMissingInterfaceTest.java
@@ -4,8 +4,6 @@
package com.android.tools.r8.desugar;
-import static org.hamcrest.CoreMatchers.containsString;
-
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -40,12 +38,7 @@
.compile()
.addRunClasspathClasses(MissingInterface.class)
.run(parameters.getRuntime(), Main.class)
- // We allow for renaming if the class is missing
- .assertFailureWithErrorThatMatchesIf(
- parameters.getDexRuntimeVersion().isDalvik(),
- containsString(descriptor(MissingInterface.class) + "' is not accessible"))
- .assertFailureWithErrorThatThrowsIf(
- !parameters.getDexRuntimeVersion().isDalvik(), IllegalAccessError.class);
+ .assertFailureWithErrorThatThrows(AbstractMethodError.class);
}
interface MissingInterface {
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageLambdaMissingInterfaceTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageLambdaMissingInterfaceTest.java
index 2265ba0..df0bfa6 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageLambdaMissingInterfaceTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageLambdaMissingInterfaceTest.java
@@ -5,7 +5,7 @@
package com.android.tools.r8.repackage;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
-import static org.hamcrest.CoreMatchers.startsWith;
+import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
@@ -52,6 +52,10 @@
.inspect(
inspector -> {
// Find the generated lambda class
+ if (!parameters.isDexRuntime()) {
+ assertThat(ClassWithLambda.class, isNotRepackaged(inspector));
+ return;
+ }
if (repackage) {
assertThat(ClassWithLambda.class, isRepackaged(inspector));
} else {
@@ -62,8 +66,7 @@
clazz -> {
if (clazz.isSynthesizedJavaLambdaClass()) {
assertThat(
- clazz.getFinalName(),
- startsWith(repackage ? getRepackagePackage() : "a."));
+ clazz.getFinalName(), containsString(Main.class.getPackage().getName()));
}
});
})
@@ -71,7 +74,7 @@
.run(parameters.getRuntime(), Main.class);
}
- public interface MissingInterface {
+ interface MissingInterface {
void bar(int x);
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageMissingMemberReferenceTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageMissingMemberReferenceTest.java
index cfa6e9a..9dc8746 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageMissingMemberReferenceTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageMissingMemberReferenceTest.java
@@ -7,6 +7,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.TestParameters;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -24,16 +25,16 @@
@Test
public void testR8WithoutRepackaging() throws Exception {
- runTest(false);
+ runTest(false).assertSuccessWithOutputLines(EXPECTED);
}
@Test
public void testR8() throws Exception {
- runTest(true);
+ runTest(true).assertSuccessWithOutputLines(EXPECTED);
}
- private void runTest(boolean repackage) throws Exception {
- testForR8(parameters.getBackend())
+ private R8TestRunResult runTest(boolean repackage) throws Exception {
+ return testForR8(parameters.getBackend())
.addProgramClasses(ClassWithMissingReferenceInCode.class, Main.class)
.addKeepMainRule(Main.class)
.applyIf(repackage, this::configureRepackaging)
@@ -43,14 +44,12 @@
.compile()
.inspect(
inspector ->
- assertThat(
- ClassWithMissingReferenceInCode.class, isRepackagedIf(inspector, repackage)))
+ assertThat(ClassWithMissingReferenceInCode.class, isNotRepackaged(inspector)))
.addRunClasspathClasses(MissingReference.class)
- .run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(EXPECTED);
+ .run(parameters.getRuntime(), Main.class);
}
- public static class MissingReference {
+ static class MissingReference {
public static void doSomething() {
System.out.println("MissingReference::doSomething");
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperInterfaceTestTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperInterfaceTestTest.java
index e5ffadc..c853a99 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperInterfaceTestTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperInterfaceTestTest.java
@@ -4,13 +4,12 @@
package com.android.tools.r8.repackage;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.utils.codeinspector.ClassSubject;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -27,16 +26,16 @@
@Test
public void testR8WithoutRepackaging() throws Exception {
- runTest(false);
+ runTest(false).assertSuccessWithOutputLines(EXPECTED);
}
@Test
public void testR8() throws Exception {
- runTest(true);
+ runTest(true).assertSuccessWithOutputLines(EXPECTED);
}
- private void runTest(boolean repackage) throws Exception {
- testForR8(parameters.getBackend())
+ private R8TestRunResult runTest(boolean repackage) throws Exception {
+ return testForR8(parameters.getBackend())
.addProgramClasses(ClassImplementingMissingInterface.class, Main.class)
.addKeepMainRule(Main.class)
.applyIf(repackage, this::configureRepackaging)
@@ -47,20 +46,13 @@
.compile()
.inspect(
inspector -> {
- if (repackage) {
- assertThat(ClassImplementingMissingInterface.class, isRepackaged(inspector));
- } else {
- // The class is minified.
- ClassSubject clazz = inspector.clazz(ClassImplementingMissingInterface.class);
- assertThat(clazz, isPresentAndRenamed());
- }
+ assertThat(ClassImplementingMissingInterface.class, isNotRepackaged(inspector));
})
.addRunClasspathClasses(MissingInterface.class)
- .run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(EXPECTED);
+ .run(parameters.getRuntime(), Main.class);
}
- public interface MissingInterface {
+ private interface MissingInterface {
void bar();
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperTypeTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperTypeTest.java
index b903e37..5880f86 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperTypeTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperTypeTest.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.TestParameters;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -31,16 +32,16 @@
@Test
public void testR8WithoutRepackaging() throws Exception {
- runTest(false);
+ runTest(false).assertSuccessWithOutputLines(EXPECTED);
}
@Test
public void testR8() throws Exception {
- runTest(true);
+ runTest(true).assertSuccessWithOutputLines(EXPECTED);
}
- private void runTest(boolean repackage) throws Exception {
- testForR8(parameters.getBackend())
+ private R8TestRunResult runTest(boolean repackage) throws Exception {
+ return testForR8(parameters.getBackend())
.addProgramClasses(
ClassWithSuperCall.class,
ClassWithoutSuperCall.class,
@@ -55,15 +56,14 @@
.compile()
.inspect(
inspector -> {
- assertThat(ClassWithSuperCall.class, isRepackagedIf(inspector, repackage));
- assertThat(ClassWithoutSuperCall.class, isRepackagedIf(inspector, repackage));
+ assertThat(ClassWithSuperCall.class, isNotRepackaged(inspector));
+ assertThat(ClassWithoutSuperCall.class, isNotRepackaged(inspector));
})
.addRunClasspathClasses(MissingSuperType.class)
- .run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines(EXPECTED);
+ .run(parameters.getRuntime(), Main.class);
}
- public static class MissingSuperType {
+ static class MissingSuperType {
public void foo() {
System.out.println("MissingSuperType::foo");
