Rewrite Kotlin @Metadata on renamed classes.
Bug: 70169921, 147447503, 143687784
Change-Id: I616f1c1adef5408c84854dbfd8b7d3451509e700
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java
index 0edb554..9ac3a82 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java
@@ -48,6 +48,16 @@
@Override
void rewrite(AppView<AppInfoWithLiveness> appView, NamingLens lens) {
+ if (appView.options().enableKotlinMetadataRewritingForRenamedClasses
+ && lens.lookupType(clazz.type, appView.dexItemFactory()) != clazz.type) {
+ String renamedClassifier = toRenamedClassifier(clazz.type, appView, lens);
+ if (renamedClassifier != null) {
+ assert !kmClass.getName().equals(renamedClassifier);
+ kmClass.setName(renamedClassifier);
+ }
+ }
+
+ // Rewriting upward hierarchy.
List<KmType> superTypes = kmClass.getSupertypes();
superTypes.clear();
for (DexType itfType : clazz.interfaces.values) {
@@ -66,7 +76,8 @@
superTypes.add(toKmType(addKotlinPrefix("Any;")));
}
- if (!appView.options().enableKotlinMetadataRewriting) {
+ // TODO(b/70169921): downward hierarchy is harmless. Move above member-rewriting flag.
+ if (!appView.options().enableKotlinMetadataRewritingForMembers) {
return;
}
List<KmConstructor> constructors = kmClass.getConstructors();
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassFacade.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassFacade.java
index f8c1c12..07ec533 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassFacade.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassFacade.java
@@ -36,6 +36,7 @@
void rewrite(AppView<AppInfoWithLiveness> appView, NamingLens lens) {
// TODO(b/70169921): no idea yet!
assert lens.lookupType(clazz.type, appView.dexItemFactory()) == clazz.type
+ || appView.options().enableKotlinMetadataRewritingForRenamedClasses
: toString();
}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java
index 94a993b..58c5aff 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java
@@ -37,7 +37,7 @@
@Override
void rewrite(AppView<AppInfoWithLiveness> appView, NamingLens lens) {
- if (!appView.options().enableKotlinMetadataRewriting) {
+ if (!appView.options().enableKotlinMetadataRewritingForMembers) {
return;
}
rewriteDeclarationContainer(kmPackage, appView, lens);
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java b/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java
index 9eb70f8..5eac335 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java
@@ -37,7 +37,7 @@
@Override
void rewrite(AppView<AppInfoWithLiveness> appView, NamingLens lens) {
- if (!appView.options().enableKotlinMetadataRewriting) {
+ if (!appView.options().enableKotlinMetadataRewritingForMembers) {
return;
}
rewriteDeclarationContainer(kmPackage, appView, lens);
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 5658b0d..b295da3 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataRewriter.java
@@ -68,7 +68,10 @@
if (kotlinInfo != null) {
// If @Metadata is still associated, this class should not be renamed
// (by {@link ClassNameMinifier} of course).
+ // Or, we start maintaining @Metadata for renamed classes.
+ // TODO(b/70169921): if this option is settled down, this assertion is meaningless.
assert lens.lookupType(clazz.type, appView.dexItemFactory()) == clazz.type
+ || appView.options().enableKotlinMetadataRewritingForRenamedClasses
: clazz.toSourceString() + " != "
+ lens.lookupType(clazz.type, appView.dexItemFactory());
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java
index 8a8773a..c46b94a 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java
@@ -128,7 +128,7 @@
}
DexMethod renamedMethod = lens.lookupMethod(method.method, appView.dexItemFactory());
// For a library method override, we should not have renamed it.
- assert !method.isLibraryMethodOverride().isTrue() || renamedMethod == method.method
+ assert !method.isLibraryMethodOverride().isTrue() || renamedMethod.name == method.method.name
: method.toSourceString() + " -> " + renamedMethod.toSourceString();
// TODO(b/70169921): Should we keep kotlin-specific flags only while synthesizing the base
// value from general JVM flags?
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClass.java b/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClass.java
index 59f7dce..28a6eb1 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClass.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinSyntheticClass.java
@@ -53,6 +53,7 @@
void rewrite(AppView<AppInfoWithLiveness> appView, NamingLens lens) {
// TODO(b/70169921): no idea yet!
assert lens.lookupType(clazz.type, appView.dexItemFactory()) == clazz.type
+ || appView.options().enableKotlinMetadataRewritingForRenamedClasses
: toString();
}
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index d13cc3b..1c75720 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -115,7 +115,9 @@
if (!renaming.containsKey(clazz.type)) {
DexString renamed = computeName(clazz.type);
renaming.put(clazz.type, renamed);
- KotlinMetadataRewriter.removeKotlinMetadataFromRenamedClass(appView, clazz);
+ if (!appView.options().enableKotlinMetadataRewritingForRenamedClasses) {
+ KotlinMetadataRewriter.removeKotlinMetadataFromRenamedClass(appView, clazz);
+ }
// If the class is a member class and it has used $ separator, its renamed name should have
// the same separator (as long as inner-class attribute is honored).
assert !keepInnerClassStructure
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 5b0400f..259d74a 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -224,7 +224,11 @@
public boolean enablePropagationOfDynamicTypesAtCallSites = true;
// TODO(b/69963623): enable if everything is ready, including signature rewriting at call sites.
public boolean enablePropagationOfConstantsAtCallSites = false;
- public boolean enableKotlinMetadataRewriting = true;
+ // At 2.0, part of @Metadata up to this flag is rewritten, which is super-type hierarchy.
+ public boolean enableKotlinMetadataRewritingForMembers = true;
+ // Up to 2.0, Kotlin @Metadata is removed if the associated class is renamed.
+ // Under this flag, Kotlin @Metadata is generally kept and modified for all program classes.
+ public boolean enableKotlinMetadataRewritingForRenamedClasses = true;
public boolean encodeChecksums = false;
public BiPredicate<String, Long> dexClassChecksumFilter = (name, checksum) -> true;
public boolean enableCfInterfaceMethodDesugaring = false;
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 cd74f9b..6bfafc4 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
@@ -4,11 +4,13 @@
package com.android.tools.r8.kotlin.metadata;
import static com.android.tools.r8.KotlinCompilerTool.KOTLINC;
+import static com.android.tools.r8.utils.DescriptorUtils.getDescriptorFromKotlinClassifier;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.ToolHelper;
@@ -116,7 +118,7 @@
// API entry is kept, hence @Metadata exists.
KmClassSubject kmClass = kept.getKmClass();
assertThat(kmClass, isPresent());
- // TODO(b/70169921): check if `name` in @Metadata is equal to the (kept) class name.
+ assertEquals(kept.getFinalDescriptor(), getDescriptorFromKotlinClassifier(kmClass.getName()));
// @Anno is kept.
String annoName = pkg + ".anno.Anno";
AnnotationSubject anno = kept.annotation(annoName);
@@ -127,9 +129,10 @@
// @Anno is kept.
anno = renamed.annotation(annoName);
assertThat(anno, isPresent());
- // TODO(b/147447503): But, R8 treats @Metadata specially.
+ // @Metadata is kept even though the class is renamed.
kmClass = renamed.getKmClass();
- assertThat(kmClass, not(isPresent()));
- // TODO(b/70169921): check if `name` in @Metadata is equal to the (renamed) class name.
+ assertThat(kmClass, isPresent());
+ assertEquals(
+ renamed.getFinalDescriptor(), getDescriptorFromKotlinClassifier(kmClass.getName()));
}
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataStripTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataStripTest.java
index f93179f..31cd07d 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataStripTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataStripTest.java
@@ -52,6 +52,8 @@
.addKeepMainRule(mainClassName)
.addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
.addKeepRules("-keep class kotlin.Metadata")
+ // TODO(b/70169921): if this option is settled down, this test is meaningless.
+ .addOptionsModification(o -> o.enableKotlinMetadataRewritingForRenamedClasses = false)
.allowDiagnosticWarningMessages()
.setMinApi(parameters.getApiLevel())
.compile()
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentKmClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentKmClassSubject.java
index 1a640ed..af619ea 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentKmClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentKmClassSubject.java
@@ -9,6 +9,11 @@
public class AbsentKmClassSubject extends KmClassSubject {
@Override
+ public String getName() {
+ return null;
+ }
+
+ @Override
public DexClass getDexClass() {
return null;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundKmClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundKmClassSubject.java
index 6c65c2d..39a3ce7 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundKmClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundKmClassSubject.java
@@ -24,6 +24,11 @@
}
@Override
+ public String getName() {
+ return kmClass.getName();
+ }
+
+ @Override
public DexClass getDexClass() {
return clazz;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/KmClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/KmClassSubject.java
index 97b233d..191956c 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/KmClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/KmClassSubject.java
@@ -7,6 +7,8 @@
import java.util.List;
public abstract class KmClassSubject extends Subject implements KmDeclarationContainerSubject {
+ public abstract String getName();
+
public abstract DexClass getDexClass();
public abstract List<String> getSuperTypeDescriptors();
