Support merging of classes with static fields
Bug: 163311975
Change-Id: I58d4641525289b315d2d98dd73f1a7716db35f55
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 dc680a8..13c4ac4 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -1845,6 +1845,23 @@
holder);
return method;
}
+ /**
+ * Tries to find a method name for insertion into the class {@code target} of the form
+ * baseName$holder$n, where {@code baseName} and {@code holder} are supplied by the user, and
+ * {@code n} is picked to be the first number so that {@code isFresh.apply(method)} returns {@code
+ * true}.
+ *
+ * @param holder indicates where the method originates from.
+ */
+ public DexField createFreshFieldName(
+ DexField template, DexType holder, Predicate<DexField> isFresh) {
+ DexField field =
+ createFreshMember(
+ name -> Optional.of(template.withName(name, this)).filter(isFresh),
+ template.name.toSourceString(),
+ holder);
+ return field;
+ }
public DexMethod createInstanceInitializerWithFreshProto(
DexMethod method, DexType extraType, Predicate<DexMethod> isFresh) {
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
index f99999a..d441d2c 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -49,6 +49,7 @@
private final FieldAccessInfoCollectionModifier.Builder fieldAccessChangesBuilder;
private final Reference2IntMap<DexType> classIdentifiers = new Reference2IntOpenHashMap<>();
+ private final ClassStaticFieldsMerger classStaticFieldsMerger;
private final Collection<VirtualMethodMerger> virtualMethodMergers;
private final Collection<ConstructorMerger> constructorMergers;
private final DexField classIdField;
@@ -73,6 +74,7 @@
this.constructorMergers = constructorMergers;
this.dexItemFactory = appView.dexItemFactory();
+ this.classStaticFieldsMerger = new ClassStaticFieldsMerger(appView, lensBuilder, target);
buildClassIdentifierMap();
}
@@ -115,6 +117,8 @@
}
});
+ classStaticFieldsMerger.addFields(toMerge);
+
// Clear the members of the class to be merged since they have now been moved to the target.
toMerge.setVirtualMethods(null);
toMerge.setDirectMethods(null);
@@ -161,6 +165,10 @@
target.appendInstanceField(encodedField);
}
+ void mergeStaticFields() {
+ classStaticFieldsMerger.merge(target);
+ }
+
public void mergeGroup(SyntheticArgumentClass syntheticArgumentClass) {
appendClassIdField();
@@ -171,6 +179,7 @@
mergeConstructors(syntheticArgumentClass);
mergeVirtualMethods();
+ mergeStaticFields();
}
public static class Builder {
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassStaticFieldsMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassStaticFieldsMerger.java
new file mode 100644
index 0000000..ce4b93e
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassStaticFieldsMerger.java
@@ -0,0 +1,60 @@
+// 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.horizontalclassmerging;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.horizontalclassmerging.HorizontalClassMergerGraphLens.Builder;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class ClassStaticFieldsMerger {
+ private final Builder lensBuilder;
+ private final DexProgramClass target;
+ private final Map<DexField, DexEncodedField> targetFields = new LinkedHashMap<>();
+ private final DexItemFactory dexItemFactory;
+ private final AppView<?> appView;
+
+ public ClassStaticFieldsMerger(
+ AppView<?> appView,
+ HorizontalClassMergerGraphLens.Builder lensBuilder,
+ DexProgramClass target) {
+ this.appView = appView;
+ this.lensBuilder = lensBuilder;
+
+ this.target = target;
+ // Add mappings for all target fields.
+ target.staticFields().forEach(field -> targetFields.put(field.getReference(), field));
+
+ this.dexItemFactory = appView.dexItemFactory();
+ }
+
+ private final boolean isFresh(DexField fieldReference) {
+ return !targetFields.containsKey(fieldReference);
+ }
+
+ private void addField(DexEncodedField field) {
+ DexField oldFieldReference = field.getReference();
+ DexField templateReference = field.getReference().withHolder(target.type, dexItemFactory);
+ DexField newFieldReference =
+ dexItemFactory.createFreshFieldName(templateReference, field.holder(), this::isFresh);
+
+ field = field.toTypeSubstitutedField(newFieldReference);
+ targetFields.put(newFieldReference, field);
+
+ lensBuilder.mapField(oldFieldReference, newFieldReference);
+ }
+
+ public void addFields(DexProgramClass toMerge) {
+ toMerge.staticFields().forEach(this::addField);
+ }
+
+ public void merge(DexProgramClass clazz) {
+ clazz.setStaticFields(targetFields.values().toArray(DexEncodedField.EMPTY_ARRAY));
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
index 5dc81b1..225f8fe 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -17,8 +17,8 @@
import com.android.tools.r8.horizontalclassmerging.policies.NoClassesOrMembersWithAnnotations;
import com.android.tools.r8.horizontalclassmerging.policies.NoClassesWithInterfaces;
import com.android.tools.r8.horizontalclassmerging.policies.NoEnums;
-import com.android.tools.r8.horizontalclassmerging.policies.NoFields;
import com.android.tools.r8.horizontalclassmerging.policies.NoInnerClasses;
+import com.android.tools.r8.horizontalclassmerging.policies.NoInstanceFields;
import com.android.tools.r8.horizontalclassmerging.policies.NoInterfaces;
import com.android.tools.r8.horizontalclassmerging.policies.NoKeepRules;
import com.android.tools.r8.horizontalclassmerging.policies.NoNativeMethods;
@@ -59,7 +59,7 @@
List<Policy> policies =
ImmutableList.of(
new NotMatchedByNoHorizontalClassMerging(appView),
- new NoFields(),
+ new NoInstanceFields(),
// TODO(b/166071504): Allow merging of classes that implement interfaces.
new NoInterfaces(),
new NoClassesWithInterfaces(),
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
index 867725b..8dadc71 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMergerGraphLens.java
@@ -123,6 +123,10 @@
}
public Builder mapField(DexField from, DexField to) {
+ DexField previousFrom = fieldMap.inverse().remove(from);
+ if (previousFrom != null) {
+ from = previousFrom;
+ }
fieldMap.put(from, to);
return this;
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoFields.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoInstanceFields.java
similarity index 78%
rename from src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoFields.java
rename to src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoInstanceFields.java
index 9ba49a4..326e94a 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoFields.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoInstanceFields.java
@@ -7,10 +7,9 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.horizontalclassmerging.SingleClassPolicy;
-public class NoFields extends SingleClassPolicy {
+public class NoInstanceFields extends SingleClassPolicy {
@Override
public boolean canMerge(DexProgramClass program) {
- // TODO(b/165498187): remove this policy
- return !program.hasFields();
+ return !program.hasInstanceFields();
}
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithStaticFields.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithStaticFields.java
new file mode 100644
index 0000000..f00e07e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithStaticFields.java
@@ -0,0 +1,67 @@
+// 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.classmerging.horizontal;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.notIf;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.TestParameters;
+import org.junit.Test;
+
+public class ClassesWithStaticFields extends HorizontalClassMergingTestBase {
+ public ClassesWithStaticFields(TestParameters parameters, boolean enableHorizontalClassMerging) {
+ super(parameters, enableHorizontalClassMerging);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addOptionsModification(
+ options -> options.enableHorizontalClassMerging = enableHorizontalClassMerging)
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .addHorizontallyMergedClassesInspectorIf(
+ enableHorizontalClassMerging, inspector -> inspector.assertMergedInto(B.class, A.class))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("a: 0", "b: 0", "a: 1", "b: 1")
+ .inspect(
+ codeInspector -> {
+ assertThat(codeInspector.clazz(A.class), isPresent());
+ assertThat(
+ codeInspector.clazz(B.class), notIf(isPresent(), enableHorizontalClassMerging));
+ });
+ }
+
+ @NeverClassInline
+ public static class A {
+ public static int counter;
+
+ public A() {
+ System.out.println("a: " + counter++);
+ }
+ }
+
+ @NeverClassInline
+ public static class B {
+ public static int counter;
+
+ public B() {
+ System.out.println("b: " + counter++);
+ }
+ }
+
+ public static class Main {
+ public static void main(String[] args) {
+ new A();
+ new B();
+ new A();
+ new B();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/GenericStaticFieldTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/GenericStaticFieldTest.java
new file mode 100644
index 0000000..9b86b43
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/GenericStaticFieldTest.java
@@ -0,0 +1,73 @@
+// 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.classmerging.horizontal;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.TestParameters;
+import org.junit.Test;
+
+public class GenericStaticFieldTest extends HorizontalClassMergingTestBase {
+ public GenericStaticFieldTest(TestParameters parameters, boolean enableHorizontalClassMerging) {
+ super(parameters, enableHorizontalClassMerging);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addKeepRules("-keepattributes Signatures")
+ .addOptionsModification(
+ options -> options.enableHorizontalClassMerging = enableHorizontalClassMerging)
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ // .addHorizontallyMergedClassesInspectorIf(
+ // enableHorizontalClassMerging, inspector ->
+ // inspector.assertMergedInto(EmptyClassTest.B.class, EmptyClassTest.A.class))
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("false", "false")
+ .inspect(codeInspector -> {});
+ }
+
+ @NeverClassInline
+ public static class A<Z extends Object> {
+ public A(Z z) {
+ if (System.currentTimeMillis() < 0) {
+ z.toString();
+ }
+ }
+ }
+
+ @NeverClassInline
+ public static class B {}
+
+ @NeverClassInline
+ public static class Foo {
+ public static A<Object> field;
+ public static A<B> field2;
+
+ public Foo() {
+ field = new A<Object>(new Object());
+ }
+ }
+
+ @NeverClassInline
+ public static class Foo2 {
+ public static A<Object> field;
+
+ public Foo2() {
+ field = new A<Object>(new Object());
+ }
+ }
+
+ public static class Main {
+ public static void main(String[] args) {
+ Foo foo = new Foo();
+ Foo2 foo2 = new Foo2();
+ System.out.println(foo.field.equals(System.out));
+ System.out.println(foo2.field.equals(System.out));
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/CandidateOkSideEffects.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/CandidateOkSideEffects.java
index a02bed3..36e56f2 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/CandidateOkSideEffects.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/CandidateOkSideEffects.java
@@ -5,7 +5,9 @@
package com.android.tools.r8.ir.optimize.staticizer.movetohost;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoHorizontalClassMerging;
+@NoHorizontalClassMerging
public class CandidateOkSideEffects {
@NeverInline
public String foo() {
diff --git a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSuperClassTest.java b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSuperClassTest.java
index e7f32e3..3c264ae 100644
--- a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSuperClassTest.java
+++ b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSuperClassTest.java
@@ -11,6 +11,7 @@
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assume.assumeFalse;
+import com.android.tools.r8.NoHorizontalClassMerging;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.StringUtils;
@@ -50,6 +51,7 @@
reserveName
? "-keepclassmembernames class " + A.class.getTypeName() + "{ <fields>; }"
: "")
+ .enableNoHorizontalClassMergingAnnotations()
.run(TestClass.class)
.assertSuccessWithOutput(expectedOutput)
.inspector();
@@ -78,6 +80,7 @@
.addLibraryClasses(A.class)
.addLibraryFiles(runtimeJar(Backend.DEX))
.addKeepMainRule(TestClass.class)
+ .enableNoHorizontalClassMergingAnnotations()
.compile()
.addRunClasspathFiles(testForD8().addProgramClasses(A.class).compile().writeToZip())
.run(TestClass.class)
@@ -128,6 +131,7 @@
}
}
+ @NoHorizontalClassMerging
static class ASub2 extends A {
static String bar;
