Only emit kotlin metadata values for non-pruned annotation members
Bug: 161230424
Bug: 162900580
Change-Id: Ib74e75e89577626b0b1e511011f4e018bb9c9795
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java
index 2171a00..281950c 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassMetadataReader.java
@@ -188,8 +188,8 @@
return v.asDexValueString().getValue().toString();
}
- private static class MetadataError extends RuntimeException {
- MetadataError(String cause) {
+ public static class MetadataError extends RuntimeException {
+ private MetadataError(String cause) {
super(cause);
}
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java
index d709ea4..5e11d43 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java
@@ -9,8 +9,10 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexAnnotationElement;
+import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedAnnotation;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexValue;
import com.android.tools.r8.graph.DexValue.DexValueArray;
import com.android.tools.r8.graph.DexValue.DexValueInt;
@@ -25,6 +27,36 @@
public class KotlinMetadataRewriter {
+ private static final class WriteMetadataFieldInfo {
+ final boolean writeKind;
+ final boolean writeMetadataVersion;
+ final boolean writeByteCodeVersion;
+ final boolean writeData1;
+ final boolean writeData2;
+ final boolean writeExtraString;
+ final boolean writePackageName;
+ final boolean writeExtraInt;
+
+ public WriteMetadataFieldInfo(
+ boolean writeKind,
+ boolean writeMetadataVersion,
+ boolean writeByteCodeVersion,
+ boolean writeData1,
+ boolean writeData2,
+ boolean writeExtraString,
+ boolean writePackageName,
+ boolean writeExtraInt) {
+ this.writeKind = writeKind;
+ this.writeMetadataVersion = writeMetadataVersion;
+ this.writeByteCodeVersion = writeByteCodeVersion;
+ this.writeData1 = writeData1;
+ this.writeData2 = writeData2;
+ this.writeExtraString = writeExtraString;
+ this.writePackageName = writePackageName;
+ this.writeExtraInt = writeExtraInt;
+ }
+ }
+
private final AppView<?> appView;
private final NamingLens lens;
private final DexItemFactory factory;
@@ -42,6 +74,18 @@
}
public void run(ExecutorService executorService) throws ExecutionException {
+ final DexClass kotlinMetadata =
+ appView.definitionFor(appView.dexItemFactory().kotlinMetadataType);
+ final WriteMetadataFieldInfo writeMetadataFieldInfo =
+ new WriteMetadataFieldInfo(
+ kotlinMetadataFieldExists(kotlinMetadata, appView, kotlin.metadata.kind),
+ kotlinMetadataFieldExists(kotlinMetadata, appView, kotlin.metadata.metadataVersion),
+ kotlinMetadataFieldExists(kotlinMetadata, appView, kotlin.metadata.bytecodeVersion),
+ kotlinMetadataFieldExists(kotlinMetadata, appView, kotlin.metadata.data1),
+ kotlinMetadataFieldExists(kotlinMetadata, appView, kotlin.metadata.data2),
+ kotlinMetadataFieldExists(kotlinMetadata, appView, kotlin.metadata.extraString),
+ kotlinMetadataFieldExists(kotlinMetadata, appView, kotlin.metadata.packageName),
+ kotlinMetadataFieldExists(kotlinMetadata, appView, kotlin.metadata.extraInt));
ThreadUtils.processItems(
appView.appInfo().classes(),
clazz -> {
@@ -66,7 +110,8 @@
try {
KotlinClassHeader kotlinClassHeader = kotlinInfo.rewrite(clazz, appView, lens);
DexAnnotation newMeta =
- createKotlinMetadataAnnotation(kotlinClassHeader, kotlinInfo.getPackageName());
+ createKotlinMetadataAnnotation(
+ kotlinClassHeader, kotlinInfo.getPackageName(), writeMetadataFieldInfo);
clazz.setAnnotations(
clazz.annotations().rewrite(anno -> anno == oldMeta ? newMeta : anno));
} catch (Throwable t) {
@@ -79,33 +124,54 @@
executorService);
}
+ private boolean kotlinMetadataFieldExists(
+ DexClass kotlinMetadata, AppView<?> appView, DexString fieldName) {
+ if (!appView.appInfo().hasLiveness()) {
+ return true;
+ }
+ if (kotlinMetadata == null || kotlinMetadata.isNotProgramClass()) {
+ return true;
+ }
+ return kotlinMetadata.methods(method -> method.method.name == fieldName).iterator().hasNext();
+ }
+
private DexAnnotation createKotlinMetadataAnnotation(
- KotlinClassHeader header, String packageName) {
+ KotlinClassHeader header, String packageName, WriteMetadataFieldInfo writeMetadataFieldInfo) {
List<DexAnnotationElement> elements = new ArrayList<>();
- elements.add(
- new DexAnnotationElement(
- kotlin.metadata.metadataVersion, createIntArray(header.getMetadataVersion())));
- elements.add(
- new DexAnnotationElement(
- kotlin.metadata.bytecodeVersion, createIntArray(header.getBytecodeVersion())));
- elements.add(
- new DexAnnotationElement(kotlin.metadata.kind, DexValueInt.create(header.getKind())));
- elements.add(
- new DexAnnotationElement(kotlin.metadata.data1, createStringArray(header.getData1())));
- elements.add(
- new DexAnnotationElement(kotlin.metadata.data2, createStringArray(header.getData2())));
- if (packageName != null && !packageName.isEmpty()) {
+ if (writeMetadataFieldInfo.writeMetadataVersion) {
+ elements.add(
+ new DexAnnotationElement(
+ kotlin.metadata.metadataVersion, createIntArray(header.getMetadataVersion())));
+ }
+ if (writeMetadataFieldInfo.writeByteCodeVersion) {
+ elements.add(
+ new DexAnnotationElement(
+ kotlin.metadata.bytecodeVersion, createIntArray(header.getBytecodeVersion())));
+ }
+ if (writeMetadataFieldInfo.writeKind) {
+ elements.add(
+ new DexAnnotationElement(kotlin.metadata.kind, DexValueInt.create(header.getKind())));
+ }
+ if (writeMetadataFieldInfo.writeData1) {
+ elements.add(
+ new DexAnnotationElement(kotlin.metadata.data1, createStringArray(header.getData1())));
+ }
+ if (writeMetadataFieldInfo.writeData2) {
+ elements.add(
+ new DexAnnotationElement(kotlin.metadata.data2, createStringArray(header.getData2())));
+ }
+ if (writeMetadataFieldInfo.writePackageName && packageName != null && !packageName.isEmpty()) {
elements.add(
new DexAnnotationElement(
kotlin.metadata.packageName, new DexValueString(factory.createString(packageName))));
}
- if (!header.getExtraString().isEmpty()) {
+ if (writeMetadataFieldInfo.writeExtraString && !header.getExtraString().isEmpty()) {
elements.add(
new DexAnnotationElement(
kotlin.metadata.extraString,
new DexValueString(factory.createString(header.getExtraString()))));
}
- if (header.getExtraInt() != 0) {
+ if (writeMetadataFieldInfo.writeExtraInt && header.getExtraInt() != 0) {
elements.add(
new DexAnnotationElement(
kotlin.metadata.extraInt, DexValueInt.create(header.getExtraInt())));
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataPrunedFieldsTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataPrunedFieldsTest.java
index 41a60a4..77134d8 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataPrunedFieldsTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataPrunedFieldsTest.java
@@ -6,15 +6,12 @@
import static com.android.tools.r8.KotlinCompilerTool.KOTLINC;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
-import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ToolHelper.DexVm;
import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
import com.android.tools.r8.kotlin.metadata.metadata_pruned_fields.Main;
import com.android.tools.r8.shaking.ProguardKeepAttributes;
@@ -61,43 +58,28 @@
@Test
public void testR8() throws Exception {
- final R8TestRunResult runResult =
- testForR8(parameters.getBackend())
- .addProgramFiles(ToolHelper.getKotlinStdlibJar())
- .addProgramFiles(libJars.get(targetVersion))
- .addProgramClassFileData(Main.dump())
- .addKeepRules("-keep class " + PKG + ".metadata_pruned_fields.MethodsKt { *; }")
- .addKeepRules("-keep class kotlin.Metadata { *** pn(); }")
- .addKeepMainRule(Main.class)
- .allowDiagnosticWarningMessages()
- .setMinApi(parameters.getApiLevel())
- .addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
- .compile()
- .inspect(
- codeInspector -> {
- final ClassSubject clazz = codeInspector.clazz("kotlin.Metadata");
- assertThat(clazz, isPresent());
- assertThat(clazz.uniqueMethodWithName("pn"), isPresent());
- assertThat(clazz.uniqueMethodWithName("d1"), not(isPresent()));
- assertThat(clazz.uniqueMethodWithName("d2"), not(isPresent()));
- assertThat(clazz.uniqueMethodWithName("bv"), not(isPresent()));
- })
- .assertAllWarningMessagesMatch(
- equalTo("Resource 'META-INF/MANIFEST.MF' already exists."))
- .run(parameters.getRuntime(), Main.class);
- if (parameters.getRuntime().isCf()) {
- runResult.assertSuccessWithOutputLines("", "Hello World!");
- return;
- }
- final DexVm vm = parameters.getRuntime().asDex().getVm();
- if (vm.isOlderThanOrEqual(DexVm.ART_4_4_4_HOST)) {
- runResult.assertFailureWithErrorThatMatches(
- containsString("java.lang.RuntimeException: failure in processEncodedAnnotation"));
- } else if (vm.isOlderThanOrEqual(DexVm.ART_6_0_1_HOST)) {
- runResult.assertFailureWithErrorThatMatches(
- containsString("java.lang.IncompatibleClassChangeError: Couldn't find kotlin.Metadata."));
- } else {
- runResult.assertFailureWithErrorThatMatches(containsString("java.lang.NullPointerException"));
- }
+ testForR8(parameters.getBackend())
+ .addProgramFiles(ToolHelper.getKotlinStdlibJar())
+ .addProgramFiles(libJars.get(targetVersion))
+ .addProgramClassFileData(Main.dump())
+ .addKeepRules("-keep class " + PKG + ".metadata_pruned_fields.MethodsKt { *; }")
+ .addKeepRules("-keep class kotlin.Metadata { *** pn(); }")
+ .addKeepMainRule(Main.class)
+ .allowDiagnosticWarningMessages()
+ .setMinApi(parameters.getApiLevel())
+ .addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
+ .compile()
+ .inspect(
+ codeInspector -> {
+ final ClassSubject clazz = codeInspector.clazz("kotlin.Metadata");
+ assertThat(clazz, isPresent());
+ assertThat(clazz.uniqueMethodWithName("pn"), isPresent());
+ assertThat(clazz.uniqueMethodWithName("d1"), not(isPresent()));
+ assertThat(clazz.uniqueMethodWithName("d2"), not(isPresent()));
+ assertThat(clazz.uniqueMethodWithName("bv"), not(isPresent()));
+ })
+ .assertAllWarningMessagesMatch(equalTo("Resource 'META-INF/MANIFEST.MF' already exists."))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("", "Hello World!");
}
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInRenamedTypeTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInRenamedTypeTest.java
index bb77f38..8be744d 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInRenamedTypeTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInRenamedTypeTest.java
@@ -101,7 +101,7 @@
.addProgramFiles(inputJarMap.get(targetVersion))
.addKeepRules(OBFUSCATE_RENAMED, KEEP_KEPT)
.addKeepRules("-keep class **.Anno")
- .addKeepRules("-keep class kotlin.Metadata")
+ .addKeepKotlinMetadata()
.addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
.allowDiagnosticWarningMessages()
.compile()
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteKeepTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteKeepTest.java
index 295f26a..3986b38 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteKeepTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteKeepTest.java
@@ -4,17 +4,15 @@
package com.android.tools.r8.kotlin.metadata;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThrows;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
+import com.android.tools.r8.kotlin.KotlinClassMetadataReader.MetadataError;
import com.android.tools.r8.shaking.ProguardKeepAttributes;
-import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
import java.util.Collection;
@@ -51,55 +49,19 @@
}
@Test
- public void testR8KeepPartial() throws Exception {
- // This test is a bit weird, since it shows that we can remove params from the kotlin.Metadata
- // class, but still be able to fully read the kotlin.Metadata.
- testForR8(parameters.getBackend())
- .addProgramFiles(ToolHelper.getKotlinStdlibJar())
- .setMinApi(parameters.getApiLevel())
- .addKeepRules("-keep class kotlin.Metadata { *** d1(); }")
- .addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
- .compile()
- .inspect(
- inspector -> {
- inspect(inspector);
- ClassSubject kotlinMetadataClass = inspector.clazz("kotlin.Metadata");
- assertThat(kotlinMetadataClass, isPresent());
- assertEquals(1, kotlinMetadataClass.allMethods().size());
- assertNotNull(kotlinMetadataClass.getKmClass().getName());
- });
- }
-
- @Test
- public void testR8KeepPartialCooking() throws Exception {
- // This test is a bit weird, since it shows that we can remove params from the kotlin.Metadata
- // class, but still be able to fully read the kotlin.Metadata externally.
- testForR8(parameters.getBackend())
- .addProgramFiles(ToolHelper.getKotlinStdlibJar())
- .setMinApi(parameters.getApiLevel())
- .addKeepRules("-keep class kotlin.Metadata { *** d1(); }")
- .addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
- .compile()
- .inspect(
- inspector -> {
- inspect(inspector);
- ClassSubject kotlinMetadataClass = inspector.clazz("kotlin.Metadata");
- assertThat(kotlinMetadataClass, isPresent());
- assertEquals(1, kotlinMetadataClass.allMethods().size());
- assertNotNull(kotlinMetadataClass.getKmClass().getName());
- });
- }
-
- @Test
public void testR8KeepIf() throws Exception {
testForR8(parameters.getBackend())
.addProgramFiles(ToolHelper.getKotlinStdlibJar())
.setMinApi(parameters.getApiLevel())
.addKeepRules("-keep class kotlin.io.** { *; }")
- .addKeepRules("-if class * { *** $VALUES; }", "-keep class kotlin.Metadata { *; }")
+ .addKeepRules("-if class *", "-keep class kotlin.Metadata { *; }")
.addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
.compile()
- .inspect(this::inspect);
+ .inspect(
+ codeInspector -> {
+ // TODO(b/162900580): This should be kept or the test refined.
+ assertThrows(MetadataError.class, () -> inspect(codeInspector));
+ });
}
private void inspect(CodeInspector inspector) {
