Account for host-companion relations when synthesizing property @Metadata.
Backing fields for properties in companion classes are declared in host
classes. @Metadata synthesizer currently searched for live members of
Kotlin properties _within_ a class, resulting in removals of properties
whose backing fields are not in the companion, but in the host class.
To address that, this CL
1) stores the relations between host and companion classes;
2) restores/rewrites companion object and class in the host class; and
3) revises property builder logic to explore proper fields with proper
tester: for companion, iterates fields in the host class with a tester
that checks if the field is a backing field for companion. Otherwise,
i.e., if we don't distinguish backing field for companion v.s. for the
current class, such field will be grouped to host class's property.
Also, this CL addresses undiscovered issues regarding nested class and
property flags. In particular, property has three different flags---
itself, getter, and setter---and they should be handled differently.
Currently, if a backing field is not needed, getter flags were used
as property flags, resulting in visibility issues.
Bug: 70169921, 143687784
Change-Id: I927dee3ea5dfd4d6e85ecab6b0b509f1f8bde273
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 8817968..d2a7616 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -46,9 +46,7 @@
import com.android.tools.r8.ir.optimize.enums.EnumValueInfoMapCollector;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackSimple;
import com.android.tools.r8.jar.CfApplicationWriter;
-import com.android.tools.r8.kotlin.Kotlin;
-import com.android.tools.r8.kotlin.KotlinInfo;
-import com.android.tools.r8.kotlin.KotlinMemberInfo;
+import com.android.tools.r8.kotlin.KotlinInfoCollector;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.naming.ClassNameMapper;
import com.android.tools.r8.naming.Minifier;
@@ -91,7 +89,6 @@
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.LineNumberOptimizer;
-import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.SelfRetraceTest;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.ThreadUtils;
@@ -309,7 +306,8 @@
// Compute kotlin info before setting the roots and before
// kotlin metadata annotation is removed.
- computeKotlinInfoForProgramClasses(application, appView, executorService);
+ KotlinInfoCollector.computeKotlinInfoForProgramClasses(
+ application, appView, executorService);
// Add synthesized -assumenosideeffects from min api if relevant.
if (options.isGeneratingDex()) {
@@ -913,24 +911,6 @@
}
}
- private void computeKotlinInfoForProgramClasses(
- DexApplication application, AppView<?> appView, ExecutorService executorService)
- throws ExecutionException {
- if (appView.options().kotlinOptimizationOptions().disableKotlinSpecificOptimizations) {
- return;
- }
- Kotlin kotlin = appView.dexItemFactory().kotlin;
- Reporter reporter = options.reporter;
- ThreadUtils.processItems(
- application.classes(),
- programClass -> {
- KotlinInfo kotlinInfo = kotlin.getKotlinInfo(programClass, reporter);
- programClass.setKotlinInfo(kotlinInfo);
- KotlinMemberInfo.markKotlinMemberInfo(programClass, kotlinInfo, reporter);
- },
- executorService);
- }
-
private static boolean verifyNoJarApplicationReaders(List<DexProgramClass> classes) {
for (DexProgramClass clazz : classes) {
for (DexEncodedMethod method : clazz.methods()) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
index a22091c..a6c8186 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -80,6 +80,10 @@
return kotlinMemberInfo.memberKind.isBackingField();
}
+ public boolean isKotlinBackingFieldForCompanionObject() {
+ return kotlinMemberInfo.memberKind.isBackingFieldForCompanionObject();
+ }
+
@Override
public void collectIndexedItems(
IndexedItemCollection indexedItems, DexMethod method, int instructionOffset) {
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 4a65b7c..76fae8e 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClass.java
@@ -13,7 +13,10 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.InnerClassAttribute;
@@ -30,6 +33,9 @@
KmClass kmClass;
+ DexField companionObject = null;
+ DexProgramClass hostClass = null;
+
static KotlinClass fromKotlinClassMetadata(
KotlinClassMetadata kotlinClassMetadata, DexClass clazz) {
assert kotlinClassMetadata instanceof KotlinClassMetadata.Class;
@@ -41,6 +47,28 @@
super(metadata, clazz);
}
+ void foundCompanionObject(DexEncodedField companionObject) {
+ // Companion cannot be nested. If this class is a host (and about to store a field that holds
+ // a companion object), it should not have a host class.
+ assert hostClass == null;
+ this.companionObject = companionObject.field;
+ }
+
+ boolean hasCompanionObject() {
+ return companionObject != null;
+ }
+
+ DexType getCompanionObjectType() {
+ return hasCompanionObject() ? companionObject.type : null;
+ }
+
+ void linkHostClass(DexProgramClass hostClass) {
+ // Companion cannot be nested. If this class is a companion object (and about to link to its
+ // host class), it should not have a companion object.
+ assert companionObject == null;
+ this.hostClass = hostClass;
+ }
+
@Override
void processMetadata(KotlinClassMetadata.Class metadata) {
kmClass = metadata.toKmClass();
@@ -78,10 +106,19 @@
superTypes.add(toKmType(addKotlinPrefix("Any;")));
}
- // Rewriting downward hierarchies: nested.
+ // Rewriting downward hierarchies: nested, including companion class.
+ // Note that `kotlinc` uses these nested classes to determine which classes to look up when
+ // resolving declarations in the companion object, e.g., Host.Companion.prop and Host.prop.
+ // Thus, users (in particular, library developers) should keep InnerClasses and EnclosingMethod
+ // attributes if declarations in the companion need to be exposed.
List<String> nestedClasses = kmClass.getNestedClasses();
nestedClasses.clear();
for (InnerClassAttribute innerClassAttribute : clazz.getInnerClasses()) {
+ // Skip InnerClass attribute for itself.
+ // Otherwise, an inner class would have itself as a nested class.
+ if (clazz.getInnerClassAttributeForThisClass() == innerClassAttribute) {
+ continue;
+ }
DexString renamedInnerName = lens.lookupInnerName(innerClassAttribute, appView.options());
if (renamedInnerName != null) {
nestedClasses.add(renamedInnerName.toString());
@@ -103,6 +140,8 @@
if (!appView.options().enableKotlinMetadataRewritingForMembers) {
return;
}
+
+ // Rewriting constructors.
List<KmConstructor> constructors = kmClass.getConstructors();
constructors.clear();
for (DexEncodedMethod method : clazz.directMethods()) {
@@ -115,9 +154,14 @@
}
}
+ // Rewriting companion object if any.
+ if (kmClass.getCompanionObject() != null && hasCompanionObject()) {
+ kmClass.setCompanionObject(lens.lookupName(companionObject).toString());
+ }
+
// TODO(b/70169921): enum entries
- rewriteDeclarationContainer(kmClass, appView, lens);
+ rewriteDeclarationContainer(appView, lens);
}
@Override
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 f108252..d51f712 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassPart.java
@@ -48,7 +48,7 @@
if (!appView.options().enableKotlinMetadataRewritingForMembers) {
return;
}
- rewriteDeclarationContainer(kmPackage, appView, lens);
+ rewriteDeclarationContainer(appView, lens);
}
@Override
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 bb4dc47..27c9230 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinFile.java
@@ -38,7 +38,7 @@
if (!appView.options().enableKotlinMetadataRewritingForMembers) {
return;
}
- rewriteDeclarationContainer(kmPackage, appView, lens);
+ rewriteDeclarationContainer(appView, lens);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinInfo.java b/src/main/java/com/android/tools/r8/kotlin/KotlinInfo.java
index e9c1eb6..7e7f602 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinInfo.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinInfo.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.kotlin;
import static com.android.tools.r8.kotlin.KotlinMetadataSynthesizer.toRenamedKmFunction;
+import static kotlinx.metadata.Flag.Class.IS_COMPANION_OBJECT;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
@@ -17,6 +18,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.function.Predicate;
import kotlinx.metadata.KmDeclarationContainer;
import kotlinx.metadata.KmFunction;
import kotlinx.metadata.KmProperty;
@@ -93,15 +95,49 @@
return isClass() || isFile() || isClassPart();
}
+ KmDeclarationContainer getDeclarations() {
+ if (isClass()) {
+ return asClass().kmClass;
+ } else if (isFile()) {
+ return asFile().kmPackage;
+ } else if (isClassPart()) {
+ return asClassPart().kmPackage;
+ } else {
+ throw new Unreachable("Unexpected KotlinInfo: " + this);
+ }
+ }
+
// {@link KmClass} and {@link KmPackage} are inherited from {@link KmDeclarationContainer} that
// abstract functions and properties. Rewriting of those portions can be unified here.
- void rewriteDeclarationContainer(
- KmDeclarationContainer kmDeclarationContainer,
- AppView<AppInfoWithLiveness> appView,
- NamingLens lens) {
+ void rewriteDeclarationContainer(AppView<AppInfoWithLiveness> appView, NamingLens lens) {
assert clazz != null;
+ KmDeclarationContainer kmDeclarationContainer = getDeclarations();
Map<String, KmPropertyGroup.Builder> propertyGroupBuilderMap = new HashMap<>();
+
+ // Backing fields for a companion object are declared in its host class.
+ Iterable<DexEncodedField> fields = clazz.fields();
+ Predicate<DexEncodedField> backingFieldTester = DexEncodedField::isKotlinBackingField;
+ if (isClass()) {
+ KotlinClass ktClass = asClass();
+ if (IS_COMPANION_OBJECT.invoke(ktClass.kmClass.getFlags()) && ktClass.hostClass != null) {
+ fields = ktClass.hostClass.fields();
+ backingFieldTester = DexEncodedField::isKotlinBackingFieldForCompanionObject;
+ }
+ }
+
+ for (DexEncodedField field : fields) {
+ if (backingFieldTester.test(field)) {
+ String name = field.getKotlinMemberInfo().propertyName;
+ assert name != null;
+ KmPropertyGroup.Builder builder =
+ propertyGroupBuilderMap.computeIfAbsent(
+ name,
+ k -> KmPropertyGroup.builder(field.getKotlinMemberInfo().propertyFlags, name));
+ builder.foundBackingField(field);
+ }
+ }
+
List<KmFunction> functions = kmDeclarationContainer.getFunctions();
functions.clear();
for (DexEncodedMethod method : clazz.methods()) {
@@ -121,19 +157,22 @@
assert name != null;
KmPropertyGroup.Builder builder =
propertyGroupBuilderMap.computeIfAbsent(
- name, k -> KmPropertyGroup.builder(method.getKotlinMemberInfo().flag, name));
+ name,
+ // Hitting here (creating a property builder) after visiting all fields means that
+ // this property doesn't have a backing field. Don't use members' flags.
+ k -> KmPropertyGroup.builder(method.getKotlinMemberInfo().propertyFlags, name));
switch (method.getKotlinMemberInfo().memberKind) {
case EXTENSION_PROPERTY_GETTER:
builder.isExtensionGetter();
// fallthrough;
case PROPERTY_GETTER:
- builder.foundGetter(method);
+ builder.foundGetter(method, method.getKotlinMemberInfo().flags);
break;
case EXTENSION_PROPERTY_SETTER:
builder.isExtensionSetter();
// fallthrough;
case PROPERTY_SETTER:
- builder.foundSetter(method);
+ builder.foundSetter(method, method.getKotlinMemberInfo().flags);
break;
case EXTENSION_PROPERTY_ANNOTATIONS:
builder.isExtensionAnnotations();
@@ -150,17 +189,6 @@
// TODO(b/70169921): What should we do for methods that fall into this category---no mark?
}
- for (DexEncodedField field : clazz.fields()) {
- if (field.isKotlinBackingField()) {
- String name = field.getKotlinMemberInfo().propertyName;
- assert name != null;
- KmPropertyGroup.Builder builder =
- propertyGroupBuilderMap.computeIfAbsent(
- name, k -> KmPropertyGroup.builder(field.getKotlinMemberInfo().flag, name));
- builder.foundBackingField(field);
- }
- }
-
List<KmProperty> properties = kmDeclarationContainer.getProperties();
properties.clear();
for (KmPropertyGroup.Builder builder : propertyGroupBuilderMap.values()) {
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinInfoCollector.java b/src/main/java/com/android/tools/r8/kotlin/KotlinInfoCollector.java
new file mode 100644
index 0000000..f921c06
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinInfoCollector.java
@@ -0,0 +1,62 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.kotlin;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexApplication;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.utils.Reporter;
+import com.android.tools.r8.utils.ThreadUtils;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+
+public class KotlinInfoCollector {
+ public static void computeKotlinInfoForProgramClasses(
+ DexApplication application, AppView<?> appView, ExecutorService executorService)
+ throws ExecutionException {
+ if (appView.options().kotlinOptimizationOptions().disableKotlinSpecificOptimizations) {
+ return;
+ }
+ Kotlin kotlin = appView.dexItemFactory().kotlin;
+ Reporter reporter = appView.options().reporter;
+ Map<DexProgramClass, DexProgramClass> companionToHostMap = new ConcurrentHashMap<>();
+ ThreadUtils.processItems(
+ application.classes(),
+ programClass -> {
+ KotlinInfo kotlinInfo = kotlin.getKotlinInfo(programClass, reporter);
+ programClass.setKotlinInfo(kotlinInfo);
+ KotlinMemberInfo.markKotlinMemberInfo(programClass, kotlinInfo, reporter);
+ // Store a companion type to revisit.
+ if (kotlinInfo != null
+ && kotlinInfo.isClass()
+ && kotlinInfo.asClass().hasCompanionObject()) {
+ DexType companionType = kotlinInfo.asClass().getCompanionObjectType();
+ DexProgramClass companionClass = appView.definitionForProgramType(companionType);
+ if (companionClass != null) {
+ companionToHostMap.put(companionClass, programClass);
+ }
+ }
+ },
+ executorService);
+ // TODO(b/70169921): if we can guarantee that Companion classes are visited ahead and their
+ // KotlinInfo is created before processing host classes, below could be hoisted to 1st pass.
+ // Maybe name-based filtering? E.g., classes whose name ends with "$Companion" v.s. not?
+ ThreadUtils.processItems(
+ companionToHostMap.keySet(),
+ companionClass -> {
+ KotlinInfo kotlinInfo = companionClass.getKotlinInfo();
+ if (kotlinInfo != null && kotlinInfo.isClass()) {
+ DexProgramClass hostClass = companionToHostMap.get(companionClass);
+ assert hostClass != null;
+ kotlinInfo.asClass().linkHostClass(hostClass);
+ // Revisit host class's members with declarations in the companion object.
+ KotlinMemberInfo.markKotlinMemberInfo(hostClass, kotlinInfo, reporter);
+ }
+ },
+ executorService);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinMemberInfo.java b/src/main/java/com/android/tools/r8/kotlin/KotlinMemberInfo.java
index 50058ec..b50f12d 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMemberInfo.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMemberInfo.java
@@ -7,7 +7,6 @@
import static com.android.tools.r8.kotlin.KotlinMetadataJvmExtensionUtils.toJvmMethodSignature;
import static com.android.tools.r8.kotlin.KotlinMetadataSynthesizer.isExtension;
-import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -37,8 +36,12 @@
}
public final MemberKind memberKind;
- // Original member flag. May be necessary to keep Kotlin-specific flag, e.g., suspend function.
- final int flag;
+ // Original member flags. May be necessary to keep Kotlin-specific flag, e.g., suspend function.
+ final int flags;
+ // TODO(b/70169921): better to split into FunctionInfo v.s. PropertyInfo ?
+ // Original property flags. E.g., for property getter, getter flags are stored to `flags`, while
+ // the property's flags are stored here, in case of properties without a backing field.
+ final int propertyFlags;
// Original property name for (extension) property. Otherwise, null.
final String propertyName;
// Information from original KmValueParameter(s) if available. Otherwise, null.
@@ -46,32 +49,40 @@
// Constructor for KmFunction
private KotlinMemberInfo(
- MemberKind memberKind, int flag, List<KmValueParameter> kmValueParameters) {
- this(memberKind, flag, null, kmValueParameters);
+ MemberKind memberKind, int flags, List<KmValueParameter> kmValueParameters) {
+ this(memberKind, flags, 0, null, kmValueParameters);
}
// Constructor for a backing field and a getter in KmProperty
- private KotlinMemberInfo(MemberKind memberKind, int flag, String propertyName) {
- this(memberKind, flag, propertyName, EMPTY_PARAM);
+ private KotlinMemberInfo(
+ MemberKind memberKind, int flags, int propertyFlags, String propertyName) {
+ this(memberKind, flags, propertyFlags, propertyName, EMPTY_PARAM);
}
// Constructor for a setter in KmProperty
private KotlinMemberInfo(
MemberKind memberKind,
- int flag,
+ int flags,
+ int propertyFlags,
String propertyName,
KmValueParameter kmValueParameter) {
- this(memberKind, flag, propertyName,
+ this(
+ memberKind,
+ flags,
+ propertyFlags,
+ propertyName,
kmValueParameter != null ? ImmutableList.of(kmValueParameter) : EMPTY_PARAM);
}
private KotlinMemberInfo(
MemberKind memberKind,
- int flag,
+ int flags,
+ int propertyFlags,
String propertyName,
List<KmValueParameter> kmValueParameters) {
this.memberKind = memberKind;
- this.flag = flag;
+ this.flags = flags;
+ this.propertyFlags = propertyFlags;
this.propertyName = propertyName;
assert kmValueParameters != null;
if (kmValueParameters.isEmpty()) {
@@ -100,6 +111,7 @@
FUNCTION,
EXTENSION_FUNCTION,
+ COMPANION_OBJECT_BACKING_FIELD,
PROPERTY_BACKING_FIELD,
PROPERTY_GETTER,
PROPERTY_SETTER,
@@ -110,8 +122,6 @@
EXTENSION_PROPERTY_SETTER,
EXTENSION_PROPERTY_ANNOTATIONS;
- // TODO(b/70169921): companion
-
public boolean isFunction() {
return this == FUNCTION || isExtensionFunction();
}
@@ -124,8 +134,13 @@
return this == PROPERTY_BACKING_FIELD;
}
+ public boolean isBackingFieldForCompanionObject() {
+ return this == COMPANION_OBJECT_BACKING_FIELD;
+ }
+
public boolean isProperty() {
return isBackingField()
+ || isBackingFieldForCompanionObject()
|| this == PROPERTY_GETTER
|| this == PROPERTY_SETTER
|| this == PROPERTY_ANNOTATIONS
@@ -139,24 +154,17 @@
}
}
- public static void markKotlinMemberInfo(
- DexClass clazz, KotlinInfo kotlinInfo, Reporter reporter) {
+ static void markKotlinMemberInfo(DexClass clazz, KotlinInfo kotlinInfo, Reporter reporter) {
if (kotlinInfo == null || !kotlinInfo.hasDeclarations()) {
return;
}
- if (kotlinInfo.isClass()) {
- markKotlinMemberInfo(clazz, kotlinInfo.asClass().kmClass, reporter);
- } else if (kotlinInfo.isFile()) {
- markKotlinMemberInfo(clazz, kotlinInfo.asFile().kmPackage, reporter);
- } else if (kotlinInfo.isClassPart()) {
- markKotlinMemberInfo(clazz, kotlinInfo.asClassPart().kmPackage, reporter);
- } else {
- throw new Unreachable("Unexpected KotlinInfo: " + kotlinInfo);
- }
- }
- private static void markKotlinMemberInfo(
- DexClass clazz, KmDeclarationContainer kmDeclarationContainer, Reporter reporter) {
+ KmDeclarationContainer kmDeclarationContainer = kotlinInfo.getDeclarations();
+ String companionObject = null;
+ if (kotlinInfo.isClass()) {
+ companionObject = kotlinInfo.asClass().kmClass.getCompanionObject();
+ }
+
Map<String, KmFunction> kmFunctionMap = new HashMap<>();
Map<String, KmProperty> kmPropertyFieldMap = new HashMap<>();
Map<String, KmProperty> kmPropertyGetterMap = new HashMap<>();
@@ -183,12 +191,22 @@
});
for (DexEncodedField field : clazz.fields()) {
+ if (companionObject != null && companionObject.equals(field.field.name.toString())) {
+ assert kotlinInfo.isClass();
+ kotlinInfo.asClass().foundCompanionObject(field);
+ continue;
+ }
String key = toJvmFieldSignature(field.field).asString();
if (kmPropertyFieldMap.containsKey(key)) {
KmProperty kmProperty = kmPropertyFieldMap.get(key);
field.setKotlinMemberInfo(
new KotlinMemberInfo(
- MemberKind.PROPERTY_BACKING_FIELD, kmProperty.getFlags(), kmProperty.getName()));
+ clazz == kotlinInfo.clazz
+ ? MemberKind.PROPERTY_BACKING_FIELD
+ : MemberKind.COMPANION_OBJECT_BACKING_FIELD,
+ kmProperty.getFlags(),
+ kmProperty.getFlags(),
+ kmProperty.getName()));
}
}
@@ -218,12 +236,16 @@
method.setKotlinMemberInfo(
new KotlinMemberInfo(
MemberKind.EXTENSION_PROPERTY_GETTER,
+ kmProperty.getGetterFlags(),
kmProperty.getFlags(),
kmProperty.getName()));
} else {
method.setKotlinMemberInfo(
new KotlinMemberInfo(
- MemberKind.PROPERTY_GETTER, kmProperty.getFlags(), kmProperty.getName()));
+ MemberKind.PROPERTY_GETTER,
+ kmProperty.getGetterFlags(),
+ kmProperty.getFlags(),
+ kmProperty.getName()));
}
} else if (kmPropertySetterMap.containsKey(key)) {
KmProperty kmProperty = kmPropertySetterMap.get(key);
@@ -231,6 +253,7 @@
method.setKotlinMemberInfo(
new KotlinMemberInfo(
MemberKind.EXTENSION_PROPERTY_SETTER,
+ kmProperty.getSetterFlags(),
kmProperty.getFlags(),
kmProperty.getName(),
kmProperty.getSetterParameter()));
@@ -238,6 +261,7 @@
method.setKotlinMemberInfo(
new KotlinMemberInfo(
MemberKind.PROPERTY_SETTER,
+ kmProperty.getSetterFlags(),
kmProperty.getFlags(),
kmProperty.getName(),
kmProperty.getSetterParameter()));
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 ce685b2..c3dd291 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java
@@ -9,6 +9,7 @@
import static com.android.tools.r8.utils.DescriptorUtils.descriptorToKotlinClassifier;
import static com.android.tools.r8.utils.DescriptorUtils.getBinaryNameFromDescriptor;
import static com.android.tools.r8.utils.DescriptorUtils.getDescriptorFromKmType;
+import static kotlinx.metadata.Flag.Property.IS_VAR;
import static kotlinx.metadata.FlagsKt.flagsOf;
import com.android.tools.r8.graph.AppView;
@@ -238,7 +239,7 @@
// value from general JVM flags?
int flag =
appView.appInfo().isPinned(method.method) && method.getKotlinMemberInfo() != null
- ? method.getKotlinMemberInfo().flag
+ ? method.getKotlinMemberInfo().flags
: method.accessFlags.getAsKotlinFlags();
KmFunction kmFunction = new KmFunction(flag, renamedMethod.name.toString());
JvmExtensionsKt.setSignature(kmFunction, toJvmMethodSignature(renamedMethod));
@@ -369,49 +370,57 @@
* getter, and so on.
*/
static class KmPropertyGroup {
- final int flag;
+ final int flags;
final String name;
final DexEncodedField field;
final DexEncodedMethod getter;
+ final int getterFlags;
final DexEncodedMethod setter;
+ final int setterFlags;
final DexEncodedMethod annotations;
final boolean isExtension;
private KmPropertyGroup(
- int flag,
+ int flags,
String name,
DexEncodedField field,
DexEncodedMethod getter,
+ int getterFlags,
DexEncodedMethod setter,
+ int setterFlags,
DexEncodedMethod annotations,
boolean isExtension) {
- this.flag = flag;
+ this.flags = flags;
this.name = name;
this.field = field;
this.getter = getter;
+ this.getterFlags = getterFlags;
this.setter = setter;
+ this.setterFlags = setterFlags;
this.annotations = annotations;
this.isExtension = isExtension;
}
- static Builder builder(int flag, String name) {
- return new Builder(flag, name);
+ static Builder builder(int flags, String name) {
+ return new Builder(flags, name);
}
static class Builder {
- private final int flag;
+ private final int flags;
private final String name;
private DexEncodedField field;
private DexEncodedMethod getter;
+ private int getterFlags;
private DexEncodedMethod setter;
+ private int setterFlags;
private DexEncodedMethod annotations;
private boolean isExtensionGetter;
private boolean isExtensionSetter;
private boolean isExtensionAnnotations;
- private Builder(int flag, String name) {
- this.flag = flag;
+ private Builder(int flags, String name) {
+ this.flags = flags;
this.name = name;
}
@@ -420,13 +429,15 @@
return this;
}
- Builder foundGetter(DexEncodedMethod getter) {
+ Builder foundGetter(DexEncodedMethod getter, int flags) {
this.getter = getter;
+ this.getterFlags = flags;
return this;
}
- Builder foundSetter(DexEncodedMethod setter) {
+ Builder foundSetter(DexEncodedMethod setter, int flags) {
this.setter = setter;
+ this.setterFlags = flags;
return this;
}
@@ -464,12 +475,13 @@
return null;
}
}
- return new KmPropertyGroup(flag, name, field, getter, setter, annotations, isExtension);
+ return new KmPropertyGroup(
+ flags, name, field, getter, getterFlags, setter, setterFlags, annotations, isExtension);
}
}
KmProperty toRenamedKmProperty(AppView<AppInfoWithLiveness> appView, NamingLens lens) {
- KmProperty kmProperty = new KmProperty(flag, name, flagsOf(), flagsOf());
+ KmProperty kmProperty = new KmProperty(flags, name, flagsOf(), flagsOf());
KmType kmPropertyType = null;
KmType kmReceiverType = null;
@@ -540,8 +552,13 @@
&& renamedPropertyName.equals(name)) {
renamedPropertyName = renamedGetter.name.toString();
}
- kmProperty.setGetterFlags(getter.accessFlags.getAsKotlinFlags());
+ kmProperty.setGetterFlags(getterFlags);
JvmExtensionsKt.setGetterSignature(kmProperty, toJvmMethodSignature(renamedGetter));
+ } else if (field != null) {
+ // Property without getter.
+ // Even though a getter does not exist, `kotlinc` still set getter flags and use them to
+ // determine when to direct field access v.s. getter calls for property resolution.
+ kmProperty.setGetterFlags(field.accessFlags.getAsKotlinFlags());
}
criteria = checkSetterCriteria();
@@ -610,8 +627,13 @@
&& renamedPropertyName.equals(name)) {
renamedPropertyName = renamedSetter.name.toString();
}
- kmProperty.setSetterFlags(setter.accessFlags.getAsKotlinFlags());
+ kmProperty.setSetterFlags(setterFlags);
JvmExtensionsKt.setSetterSignature(kmProperty, toJvmMethodSignature(renamedSetter));
+ } else if (field != null && IS_VAR.invoke(flags)) {
+ // Editable property without setter.
+ // Even though a setter does not exist, `kotlinc` still set setter flags and use them to
+ // determine when to direct field access v.s. setter calls for property resolution.
+ kmProperty.setGetterFlags(field.accessFlags.getAsKotlinFlags());
}
// If the property type remains null at the end, bail out to synthesize this property.
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInCompanionTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInCompanionTest.java
index f77cdb9..79ea63b 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInCompanionTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInCompanionTest.java
@@ -6,18 +6,19 @@
import static com.android.tools.r8.KotlinCompilerTool.KOTLINC;
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.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.KotlinTargetVersion;
-import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.shaking.ProguardKeepAttributes;
+import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.FieldSubject;
import com.android.tools.r8.utils.codeinspector.KmClassSubject;
import com.android.tools.r8.utils.codeinspector.KmPropertySubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -33,6 +34,9 @@
@RunWith(Parameterized.class)
public class MetadataRewriteInCompanionTest extends KotlinMetadataTestBase {
+ private static final String EXPECTED =
+ StringUtils.lines(
+ "B.Companion::foo", "B.Companion::foo", "B.Companion::foo", "B.Companion::foo");
private final TestParameters parameters;
@@ -70,21 +74,27 @@
// Keep everything
.addKeepRules("-keep class **.companion_lib.** { *; }")
.addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
+ // To keep @JvmField annotation
+ .addKeepAttributes(ProguardKeepAttributes.RUNTIME_INVISIBLE_ANNOTATIONS)
+ // To keep ...$Companion structure
+ .addKeepAttributes(ProguardKeepAttributes.INNER_CLASSES)
+ .addKeepAttributes(ProguardKeepAttributes.ENCLOSING_METHOD)
.compile()
.inspect(codeInspector -> inspect(codeInspector, true))
.writeToZip();
- ProcessResult kotlinTestCompileResult =
+ Path output =
kotlinc(parameters.getRuntime().asCf(), KOTLINC, targetVersion)
.addClasspathFiles(libJar)
.addSourceFiles(getKotlinFileInTest(PKG_PREFIX + "/companion_app", "main"))
.setOutputPath(temp.newFolder().toPath())
- // TODO(b/70169921): update to just .compile() once fixed.
- .compileRaw();
+ .compile();
- // TODO(b/70169921): should be able to compile!
- assertNotEquals(0, kotlinTestCompileResult.exitCode);
- assertThat(kotlinTestCompileResult.stderr, containsString("unresolved reference: elt2"));
+ testForJvm()
+ .addRunClasspathFiles(ToolHelper.getKotlinStdlibJar(), libJar)
+ .addClasspath(output)
+ .run(parameters.getRuntime(), PKG + ".companion_app.MainKt")
+ .assertSuccessWithOutput(EXPECTED);
}
@Test
@@ -99,31 +109,39 @@
.addKeepRules("-keep class **.I { <methods>; }")
// Keep getters for B$Companion.(eltN|foo) which will be referenced at the app.
.addKeepRules("-keepclassmembers class **.B$* { *** get*(...); }")
+ // Keep the companion instance in the B class
+ .addKeepRules("-keepclassmembers class **.B { *** Companion; }")
+ // Keep the name of companion class
+ .addKeepRules("-keepnames class **.*$Companion")
// No rule for Super, but will be kept and renamed.
.addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
+ // To keep @JvmField annotation
+ .addKeepAttributes(ProguardKeepAttributes.RUNTIME_INVISIBLE_ANNOTATIONS)
+ // To keep ...$Companion structure
+ .addKeepAttributes(ProguardKeepAttributes.INNER_CLASSES)
+ .addKeepAttributes(ProguardKeepAttributes.ENCLOSING_METHOD)
.compile()
.inspect(codeInspector -> inspect(codeInspector, false))
.writeToZip();
- ProcessResult kotlinTestCompileResult =
+ Path output =
kotlinc(parameters.getRuntime().asCf(), KOTLINC, targetVersion)
.addClasspathFiles(libJar)
.addSourceFiles(getKotlinFileInTest(PKG_PREFIX + "/companion_app", "main"))
.setOutputPath(temp.newFolder().toPath())
- // TODO(b/70169921): update to just .compile() once fixed.
- .compileRaw();
+ .compile();
- // TODO(b/70169921): should be able to compile!
- assertNotEquals(0, kotlinTestCompileResult.exitCode);
- assertThat(kotlinTestCompileResult.stderr, containsString("unresolved reference: elt1"));
- assertThat(kotlinTestCompileResult.stderr, containsString("unresolved reference: elt2"));
- assertThat(kotlinTestCompileResult.stderr, containsString("unresolved reference: foo"));
+ testForJvm()
+ .addRunClasspathFiles(ToolHelper.getKotlinStdlibJar(), libJar)
+ .addClasspath(output)
+ .run(parameters.getRuntime(), PKG + ".companion_app.MainKt")
+ .assertSuccessWithOutput(EXPECTED);
}
private void inspect(CodeInspector inspector, boolean keptAll) {
final String superClassName = PKG + ".companion_lib.Super";
final String bClassName = PKG + ".companion_lib.B";
- final String companionClassName = PKG + ".companion_lib.B$Companion";
+ final String companionClassName = bClassName + "$Companion";
ClassSubject sup = inspector.clazz(superClassName);
if (keptAll) {
@@ -147,6 +165,16 @@
supertype -> supertype.getFinalDescriptor().equals(sup.getFinalDescriptor())));
}
+ // The backing field for the property in the companion, with @JvmField
+ FieldSubject elt2 = impl.uniqueFieldWithName("elt2");
+ assertThat(elt2, isPresent());
+ assertThat(elt2, not(isRenamed()));
+
+ FieldSubject companionObject = impl.uniqueFieldWithName("Companion");
+ assertThat(companionObject, isPresent());
+ assertThat(companionObject, not(isRenamed()));
+ assertEquals(companionObject.getFinalName(), kmClass.getCompanionObject());
+
// Bridge for the property in the companion that needs a backing field.
MethodSubject elt1Bridge = impl.uniqueMethodWithName("access$getElt1$cp");
if (keptAll) {
@@ -155,6 +183,7 @@
} else {
assertThat(elt1Bridge, isRenamed());
}
+
// With @JvmField, no bridge is added.
MethodSubject elt2Bridge = impl.uniqueMethodWithName("access$getElt2$cp");
assertThat(elt2Bridge, not(isPresent()));
@@ -164,23 +193,20 @@
assertThat(fooBridge, not(isPresent()));
ClassSubject companion = inspector.clazz(companionClassName);
- if (keptAll) {
- assertThat(companion, isPresent());
- assertThat(companion, not(isRenamed()));
- } else {
- assertThat(companion, isRenamed());
- }
+ assertThat(companion, isPresent());
+ assertThat(companion, not(isRenamed()));
- // TODO(b/70169921): Assert impl's KmClass points to the correct companion object and class.
+ List<String> nestedClassDescriptors = kmClass.getNestedClassDescriptors();
+ assertEquals(1, nestedClassDescriptors.size());
+ assertEquals(companion.getFinalDescriptor(), nestedClassDescriptors.get(0));
kmClass = companion.getKmClass();
assertThat(kmClass, isPresent());
KmPropertySubject kmProperty = kmClass.kmPropertyWithUniqueName("elt1");
assertThat(kmProperty, isPresent());
- // TODO(b/70169921): property in companion with @JvmField is missing.
kmProperty = kmClass.kmPropertyWithUniqueName("elt2");
- assertThat(kmProperty, not(isPresent()));
+ assertThat(kmProperty, isPresent());
kmProperty = kmClass.kmPropertyWithUniqueName("foo");
assertThat(kmProperty, isPresent());
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/companion_lib/lib.kt b/src/test/java/com/android/tools/r8/kotlin/metadata/companion_lib/lib.kt
index e3da633..8e69219 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/companion_lib/lib.kt
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/companion_lib/lib.kt
@@ -23,6 +23,6 @@
@JvmField
val elt2: Super = B()
val foo: String
- get() = "B.Companion:foo"
+ get() = "B.Companion::foo"
}
}
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 c467da4..a009244 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
@@ -122,4 +122,9 @@
public List<ClassSubject> getSealedSubclasses() {
return null;
}
+
+ @Override
+ public String getCompanionObject() {
+ 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 7b57397..47d43e0 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
@@ -110,4 +110,9 @@
.map(this::getClassSubjectFromDescriptor)
.collect(Collectors.toList());
}
+
+ @Override
+ public String getCompanionObject() {
+ return kmClass.getCompanionObject();
+ }
}
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 7fb1f9f..3f5be34 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
@@ -22,4 +22,6 @@
public abstract List<String> getSealedSubclassDescriptors();
public abstract List<ClassSubject> getSealedSubclasses();
+
+ public abstract String getCompanionObject();
}
