Keep default ctor of a class that implements Externalizable.
Bug: 116735204
Change-Id: I25750f25c5462e9e4c54611fb4f866e8bdbd7c08
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 3df31fd..22790b4 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -412,6 +412,10 @@
return null;
}
+ public boolean isExternalizable(AppInfo appInfo) {
+ return type.implementedInterfaces(appInfo).contains(appInfo.dexItemFactory.externalizableType);
+ }
+
public boolean defaultValuesForStaticFieldsMayTriggerAllocation() {
return Arrays.stream(staticFields())
.anyMatch(field -> !field.getStaticValue().mayTriggerAllocation());
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 7254577..90a098a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -256,6 +256,7 @@
public final DexType callSiteType = createType("Ljava/lang/invoke/CallSite;");
public final DexType lookupType = createType("Ljava/lang/invoke/MethodHandles$Lookup;");
public final DexType serializableType = createType("Ljava/io/Serializable;");
+ public final DexType externalizableType = createType("Ljava/io/Externalizable;");
public final DexType comparableType = createType("Ljava/lang/Comparable;");
public final DexMethod metafactoryMethod =
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 fe63f46..138e07e 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -37,6 +37,9 @@
*/
private Set<DexType> directSubtypes = NO_DIRECT_SUBTYPE;
+ // Caching what interfaces this type is implementing. This includes super-interface hierarchy.
+ private Set<DexType> implementedInterfaces;
+
DexType(DexString descriptor) {
assert !descriptor.toString().contains(".");
this.descriptor = descriptor;
@@ -234,9 +237,12 @@
* @return a set of interfaces of {@link DexType}.
*/
public Set<DexType> implementedInterfaces(AppInfo appInfo) {
- Set<DexType> interfaces = Sets.newIdentityHashSet();
- implementedInterfaces(appInfo, interfaces);
- return interfaces;
+ if (implementedInterfaces == null) {
+ Set<DexType> interfaces = Sets.newIdentityHashSet();
+ implementedInterfaces(appInfo, interfaces);
+ implementedInterfaces = interfaces;
+ }
+ return implementedInterfaces;
}
private void implementedInterfaces(AppInfo appInfo, Set<DexType> interfaces) {
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index bfb2b95..de42c18 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -265,13 +265,18 @@
if (item.isDexClass()) {
DexClass clazz = item.asDexClass();
workList.add(Action.markInstantiated(clazz, reason));
- if (forceProguardCompatibility && clazz.hasDefaultInitializer()) {
- ProguardKeepRule compatRule =
+ if (clazz.hasDefaultInitializer()) {
+ if (forceProguardCompatibility) {
+ ProguardKeepRule compatRule =
ProguardConfigurationUtils.buildDefaultInitializerKeepRule(clazz);
- proguardCompatibilityWorkList.add(
- Action.markMethodLive(
- clazz.getDefaultInitializer(),
- KeepReason.dueToProguardCompatibilityKeepRule(compatRule)));
+ proguardCompatibilityWorkList.add(
+ Action.markMethodLive(
+ clazz.getDefaultInitializer(),
+ KeepReason.dueToProguardCompatibilityKeepRule(compatRule)));
+ }
+ if (clazz.isExternalizable(appInfo)) {
+ workList.add(Action.markMethodLive(clazz.getDefaultInitializer(), reason));
+ }
}
} else if (item.isDexEncodedField()) {
workList.add(Action.markFieldKept(item.asDexEncodedField(), reason));
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/type/TypeLatticeTest.java b/src/test/java/com/android/tools/r8/ir/analysis/type/TypeLatticeTest.java
index 0898891..8d956a8 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/type/TypeLatticeTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/type/TypeLatticeTest.java
@@ -456,7 +456,7 @@
DexType collection = factory.createType("Ljava/util/Collection;");
DexType set = factory.createType("Ljava/util/Set;");
DexType list = factory.createType("Ljava/util/List;");
- DexType serializable = factory.createType("Ljava/io/Serializable;");
+ DexType serializable = factory.serializableType;
Set<DexType> lub = computeLeastUpperBoundOfInterfaces(appInfo,
ImmutableSet.of(set), ImmutableSet.of(list));
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatibilityTestBase.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatibilityTestBase.java
index 952bae6..cf14d8b 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatibilityTestBase.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatibilityTestBase.java
@@ -88,13 +88,13 @@
case PROGUARD6_THEN_D8:
return runProguard6AndD8(programClasses, proguardConfig, proguardMap);
case R8_COMPAT:
- return runR8Compat(programClasses, proguardConfig, Backend.DEX);
+ return runR8Compat(programClasses, proguardConfig, proguardMap, Backend.DEX);
case R8_COMPAT_CF:
- return runR8Compat(programClasses, proguardConfig, Backend.CF);
+ return runR8Compat(programClasses, proguardConfig, proguardMap, Backend.CF);
case R8:
- return runR8(programClasses, proguardConfig, Backend.DEX);
+ return runR8(programClasses, proguardConfig, proguardMap, Backend.DEX);
case R8_CF:
- return runR8(programClasses, proguardConfig, Backend.CF);
+ return runR8(programClasses, proguardConfig, proguardMap, Backend.CF);
}
throw new IllegalArgumentException("Unknown shrinker: " + mode);
}
@@ -126,34 +126,45 @@
throw new IllegalArgumentException("Unknown shrinker: " + mode);
}
- protected AndroidApp runR8(List<Class> programClasses, String proguardConfig, Backend backend)
+ protected AndroidApp runR8(
+ List<Class> programClasses,
+ String proguardConfig,
+ Path proguardMap,
+ Backend backend)
throws Exception {
- return runR8(programClasses, proguardConfig, null, backend);
+ return runR8(programClasses, proguardConfig, proguardMap, null, backend);
}
protected AndroidApp runR8(
List<Class> programClasses,
String proguardConfig,
+ Path proguardMap,
Consumer<InternalOptions> configure,
Backend backend)
throws Exception {
AndroidApp app = readClassesAndRuntimeJar(programClasses, backend);
R8Command.Builder builder = ToolHelper.prepareR8CommandBuilder(app, emptyConsumer(backend));
ToolHelper.allowTestProguardOptions(builder);
- builder.addProguardConfiguration(ImmutableList.of(proguardConfig), Origin.unknown());
+ builder.addProguardConfiguration(
+ ImmutableList.of(proguardConfig, toPrintMappingRule(proguardMap)), Origin.unknown());
return ToolHelper.runR8(builder.build(), configure);
}
protected CodeInspector inspectR8Result(
List<Class> programClasses, String proguardConfig, Backend backend) throws Exception {
- return new CodeInspector(runR8(programClasses, proguardConfig, backend));
+ return new CodeInspector(runR8(programClasses, proguardConfig, null, backend));
}
protected AndroidApp runR8Compat(
- List<Class> programClasses, String proguardConfig, Backend backend) throws Exception {
+ List<Class> programClasses,
+ String proguardConfig,
+ Path proguardMap,
+ Backend backend)
+ throws Exception {
CompatProguardCommandBuilder builder = new CompatProguardCommandBuilder(true);
ToolHelper.allowTestProguardOptions(builder);
- builder.addProguardConfiguration(ImmutableList.of(proguardConfig), Origin.unknown());
+ builder.addProguardConfiguration(
+ ImmutableList.of(proguardConfig, toPrintMappingRule(proguardMap)), Origin.unknown());
programClasses.forEach(
clazz -> builder.addProgramFiles(ToolHelper.getClassFileForTestClass(clazz)));
if (backend == Backend.DEX) {
@@ -169,7 +180,7 @@
protected CodeInspector inspectR8CompatResult(
List<Class> programClasses, String proguardConfig, Backend backend) throws Exception {
- return new CodeInspector(runR8Compat(programClasses, proguardConfig, backend));
+ return new CodeInspector(runR8Compat(programClasses, proguardConfig, null, backend));
}
protected AndroidApp runProguard5(
@@ -306,4 +317,8 @@
assertThat(c, not(isPresent()));
}
}
+
+ private String toPrintMappingRule(Path proguardMap) {
+ return proguardMap == null ? "" : "-printmapping " + proguardMap.toAbsolutePath();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ExternalizableTest.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ExternalizableTest.java
index dcb1970c..e20b80a 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ExternalizableTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/defaultctor/ExternalizableTest.java
@@ -264,7 +264,9 @@
@Parameterized.Parameters(name = "Shrinker: {0}")
public static Collection<Object> data() {
return ImmutableList.of(
- Shrinker.PROGUARD6_THEN_D8, Shrinker.PROGUARD6, Shrinker.R8, Shrinker.R8_CF);
+ Shrinker.PROGUARD6_THEN_D8, Shrinker.PROGUARD6,
+ Shrinker.R8_COMPAT, Shrinker.R8_COMPAT_CF,
+ Shrinker.R8, Shrinker.R8_CF);
}
public ExternalizableTest(Shrinker shrinker) {
@@ -273,11 +275,6 @@
@Test
public void testExternalizable() throws Exception {
- // TODO(b/116735204): R8 should keep default ctor() of classes that implement Externalizable
- if (shrinker.isR8()) {
- return;
- }
-
String javaOutput = runOnJava(ExternalizableTestMain.class);
List<String> config = ImmutableList.of(
@@ -300,6 +297,12 @@
assertEquals(javaOutput.trim(), output.trim());
}
+ // https://docs.oracle.com/javase/8/docs/platform/serialization/spec/serial-arch.html
+ // 1.11 The Externalizable Interface
+ // ...
+ // The class of an Externalizable object must do the following:
+ // ...
+ // * Have a public no-arg constructor
CodeInspector codeInspector = new CodeInspector(processedApp, proguardMap);
ClassSubject classSubject = codeInspector.clazz(ExternalizableDataClass.class);
assertThat(classSubject, isPresent());
@@ -336,6 +339,12 @@
assertEquals(javaOutput.trim(), output.trim());
}
+ // https://docs.oracle.com/javase/8/docs/platform/serialization/spec/serial-arch.html
+ // 1.10 The Serializable Interface
+ // ...
+ // A Serializable class must do the following:
+ // ...
+ // * Have access to the no-arg constructor of its first non-serializable superclass
CodeInspector codeInspector = new CodeInspector(processedApp, proguardMap);
ClassSubject classSubject = codeInspector.clazz(NonSerializableSuperClass.class);
assertThat(classSubject, isPresent());
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
index 5ea7769..89d5079 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
import org.junit.Test;
@@ -45,10 +46,12 @@
}
@Override
- protected AndroidApp runR8(List<Class> programClasses, String proguardConfig, Backend backend)
+ protected AndroidApp runR8(
+ List<Class> programClasses, String proguardConfig, Path proguardMap, Backend backend)
throws Exception {
// Disable inlining, otherwise classes can be pruned away if all their methods are inlined.
- return runR8(programClasses, proguardConfig, o -> o.enableInlining = false, backend);
+ return runR8(
+ programClasses, proguardConfig, proguardMap, o -> o.enableInlining = false, backend);
}
@Test
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 b003b7b..165a68f 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
@@ -89,9 +89,10 @@
}
@Override
- protected AndroidApp runR8(List<Class> programClasses, String proguardConfig, Backend backend)
+ protected AndroidApp runR8(
+ List<Class> programClasses, String proguardConfig, Path proguardMap, Backend backend)
throws Exception {
- return super.runR8(programClasses, proguardConfig, this::configure, backend);
+ return super.runR8(programClasses, proguardConfig, proguardMap, this::configure, backend);
}
private void check(AndroidApp app) throws Exception {
