Allow for minification of classes with references to missing classes
Bug: 150589374
Bug: 194427614
Change-Id: I898603ecefe6d94ee3171fdb8943e6e18989479a
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 62b62b9..03627d7 100644
--- a/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/FieldResolutionResult.java
@@ -66,6 +66,10 @@
return false;
}
+ public boolean isFailedResolution() {
+ return false;
+ }
+
public DexClass getInitialResolutionHolder() {
return null;
}
@@ -163,6 +167,11 @@
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 27247d4..e81bd8b 100644
--- a/src/main/java/com/android/tools/r8/repackaging/RepackagingUseRegistry.java
+++ b/src/main/java/com/android/tools/r8/repackaging/RepackagingUseRegistry.java
@@ -116,11 +116,23 @@
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 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
+ // 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
// IllegalAccessError.
- if (isInvoke) {
- // TODO(b/150589374): Only add this if we are in the minification mode of repackaging.
+
+ // 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) {
node.addNeighbor(missingTypeNode);
}
return;
@@ -161,9 +173,6 @@
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 aef58b7..be31b16 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -986,11 +986,6 @@
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 23e2e5c..90d4f75 100644
--- a/src/test/java/com/android/tools/r8/TestRunResult.java
+++ b/src/test/java/com/android/tools/r8/TestRunResult.java
@@ -145,6 +145,14 @@
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 bdcfd8a..59853ee 100644
--- a/src/test/java/com/android/tools/r8/desugar/LambdaMissingInterfaceTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/LambdaMissingInterfaceTest.java
@@ -4,6 +4,8 @@
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;
@@ -38,7 +40,12 @@
.compile()
.addRunClasspathClasses(MissingInterface.class)
.run(parameters.getRuntime(), Main.class)
- .assertFailureWithErrorThatThrows(AbstractMethodError.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);
}
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 df0bfa6..2265ba0 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.containsString;
+import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
@@ -52,10 +52,6 @@
.inspect(
inspector -> {
// Find the generated lambda class
- if (!parameters.isDexRuntime()) {
- assertThat(ClassWithLambda.class, isNotRepackaged(inspector));
- return;
- }
if (repackage) {
assertThat(ClassWithLambda.class, isRepackaged(inspector));
} else {
@@ -66,7 +62,8 @@
clazz -> {
if (clazz.isSynthesizedJavaLambdaClass()) {
assertThat(
- clazz.getFinalName(), containsString(Main.class.getPackage().getName()));
+ clazz.getFinalName(),
+ startsWith(repackage ? getRepackagePackage() : "a."));
}
});
})
@@ -74,7 +71,7 @@
.run(parameters.getRuntime(), Main.class);
}
- interface MissingInterface {
+ public 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 9dc8746..cfa6e9a 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageMissingMemberReferenceTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageMissingMemberReferenceTest.java
@@ -7,7 +7,6 @@
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;
@@ -25,16 +24,16 @@
@Test
public void testR8WithoutRepackaging() throws Exception {
- runTest(false).assertSuccessWithOutputLines(EXPECTED);
+ runTest(false);
}
@Test
public void testR8() throws Exception {
- runTest(true).assertSuccessWithOutputLines(EXPECTED);
+ runTest(true);
}
- private R8TestRunResult runTest(boolean repackage) throws Exception {
- return testForR8(parameters.getBackend())
+ private void runTest(boolean repackage) throws Exception {
+ testForR8(parameters.getBackend())
.addProgramClasses(ClassWithMissingReferenceInCode.class, Main.class)
.addKeepMainRule(Main.class)
.applyIf(repackage, this::configureRepackaging)
@@ -44,12 +43,14 @@
.compile()
.inspect(
inspector ->
- assertThat(ClassWithMissingReferenceInCode.class, isNotRepackaged(inspector)))
+ assertThat(
+ ClassWithMissingReferenceInCode.class, isRepackagedIf(inspector, repackage)))
.addRunClasspathClasses(MissingReference.class)
- .run(parameters.getRuntime(), Main.class);
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
}
- static class MissingReference {
+ public 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 c853a99..e5ffadc 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperInterfaceTestTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperInterfaceTestTest.java
@@ -4,12 +4,13 @@
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;
@@ -26,16 +27,16 @@
@Test
public void testR8WithoutRepackaging() throws Exception {
- runTest(false).assertSuccessWithOutputLines(EXPECTED);
+ runTest(false);
}
@Test
public void testR8() throws Exception {
- runTest(true).assertSuccessWithOutputLines(EXPECTED);
+ runTest(true);
}
- private R8TestRunResult runTest(boolean repackage) throws Exception {
- return testForR8(parameters.getBackend())
+ private void runTest(boolean repackage) throws Exception {
+ testForR8(parameters.getBackend())
.addProgramClasses(ClassImplementingMissingInterface.class, Main.class)
.addKeepMainRule(Main.class)
.applyIf(repackage, this::configureRepackaging)
@@ -46,13 +47,20 @@
.compile()
.inspect(
inspector -> {
- assertThat(ClassImplementingMissingInterface.class, isNotRepackaged(inspector));
+ if (repackage) {
+ assertThat(ClassImplementingMissingInterface.class, isRepackaged(inspector));
+ } else {
+ // The class is minified.
+ ClassSubject clazz = inspector.clazz(ClassImplementingMissingInterface.class);
+ assertThat(clazz, isPresentAndRenamed());
+ }
})
.addRunClasspathClasses(MissingInterface.class)
- .run(parameters.getRuntime(), Main.class);
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
}
- private interface MissingInterface {
+ public 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 5880f86..b903e37 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperTypeTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageMissingSuperTypeTest.java
@@ -8,7 +8,6 @@
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;
@@ -32,16 +31,16 @@
@Test
public void testR8WithoutRepackaging() throws Exception {
- runTest(false).assertSuccessWithOutputLines(EXPECTED);
+ runTest(false);
}
@Test
public void testR8() throws Exception {
- runTest(true).assertSuccessWithOutputLines(EXPECTED);
+ runTest(true);
}
- private R8TestRunResult runTest(boolean repackage) throws Exception {
- return testForR8(parameters.getBackend())
+ private void runTest(boolean repackage) throws Exception {
+ testForR8(parameters.getBackend())
.addProgramClasses(
ClassWithSuperCall.class,
ClassWithoutSuperCall.class,
@@ -56,14 +55,15 @@
.compile()
.inspect(
inspector -> {
- assertThat(ClassWithSuperCall.class, isNotRepackaged(inspector));
- assertThat(ClassWithoutSuperCall.class, isNotRepackaged(inspector));
+ assertThat(ClassWithSuperCall.class, isRepackagedIf(inspector, repackage));
+ assertThat(ClassWithoutSuperCall.class, isRepackagedIf(inspector, repackage));
})
.addRunClasspathClasses(MissingSuperType.class)
- .run(parameters.getRuntime(), Main.class);
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
}
- static class MissingSuperType {
+ public static class MissingSuperType {
public void foo() {
System.out.println("MissingSuperType::foo");
