Avoid merging classes if target has static initialization
Change-Id: I7fb2e1a1c97eb9748244d8a9b087d5a6613229c4
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 c76865e..d9940f3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -448,6 +448,13 @@
}
public boolean classInitializationMayHaveSideEffects(AppInfo appInfo) {
+ return classInitializationMayHaveSideEffects(appInfo, Predicates.alwaysFalse());
+ }
+
+ public boolean classInitializationMayHaveSideEffects(AppInfo appInfo, Predicate<DexType> ignore) {
+ if (ignore.test(type)) {
+ return false;
+ }
if (hasNonTrivialClassInitializer()) {
return true;
}
@@ -455,11 +462,11 @@
return true;
}
for (DexType iface : interfaces.values) {
- if (iface.classInitializationMayHaveSideEffects(appInfo)) {
+ if (iface.classInitializationMayHaveSideEffects(appInfo, ignore)) {
return true;
}
}
- if (superType != null && superType.classInitializationMayHaveSideEffects(appInfo)) {
+ if (superType != null && superType.classInitializationMayHaveSideEffects(appInfo, ignore)) {
return true;
}
return false;
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 31edee9..0c48ea9 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -23,6 +23,7 @@
import java.util.TreeSet;
import java.util.function.Consumer;
import java.util.function.Function;
+import java.util.function.Predicate;
public class DexType extends DexReference implements PresortedComparable<DexType> {
@@ -119,9 +120,9 @@
return implementedInterfaces(appInfo).contains(appInfo.dexItemFactory.serializableType);
}
- public boolean classInitializationMayHaveSideEffects(AppInfo appInfo) {
+ public boolean classInitializationMayHaveSideEffects(AppInfo appInfo, Predicate<DexType> ignore) {
DexClass clazz = appInfo.definitionFor(this);
- return clazz == null || clazz.classInitializationMayHaveSideEffects(appInfo);
+ return clazz == null || clazz.classInitializationMayHaveSideEffects(appInfo, ignore);
}
public boolean isUnknown() {
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index bc22da0..b93c867 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -372,16 +372,16 @@
public DexEncodedMethod mapDexEncodedMethod(
AppInfo appInfo, DexEncodedMethod originalEncodedMethod) {
DexMethod newMethod = getRenamedMethodSignature(originalEncodedMethod.method);
- if (newMethod != originalEncodedMethod.method) {
- // We can't directly use AppInfo#definitionFor(DexMethod) since definitions may not be
- // updated either yet.
- DexClass newHolder = appInfo.definitionFor(newMethod.holder);
- assert newHolder != null;
- DexEncodedMethod newEncodedMethod = newHolder.lookupMethod(newMethod);
- assert newEncodedMethod != null;
- return newEncodedMethod;
- }
- return originalEncodedMethod;
+ // Note that:
+ // * Even if `newMethod` is the same as `originalEncodedMethod.method`, we still need to look it
+ // up, since `originalEncodedMethod` may be obsolete.
+ // * We can't directly use AppInfo#definitionFor(DexMethod) since definitions may not be
+ // updated either yet.
+ DexClass newHolder = appInfo.definitionFor(newMethod.holder);
+ assert newHolder != null;
+ DexEncodedMethod newEncodedMethod = newHolder.lookupMethod(newMethod);
+ assert newEncodedMethod != null;
+ return newEncodedMethod;
}
public abstract DexType lookupType(DexType type);
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
index 9c3bcc3..3ef167c 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeLatticeElement.java
@@ -330,8 +330,7 @@
if (type.isPrimitiveType()) {
return PrimitiveTypeLatticeElement.fromDexType(type, asArrayElementType);
}
- return appInfo.dexItemFactory.createReferenceTypeLatticeElement(
- type, nullability, appInfo);
+ return appInfo.dexItemFactory.createReferenceTypeLatticeElement(type, nullability, appInfo);
}
public boolean isValueTypeCompatible(TypeLatticeElement other) {
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodDesugaringLense.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodDesugaringLense.java
index a8616fb..b708ae9 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodDesugaringLense.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodDesugaringLense.java
@@ -4,8 +4,6 @@
package com.android.tools.r8.ir.desugar;
-import com.android.tools.r8.graph.AppInfo;
-import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.GraphLense;
@@ -13,14 +11,11 @@
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableMap;
-import java.util.Map;
class InterfaceMethodDesugaringLense extends NestedGraphLense {
- private final Map<DexEncodedMethod, DexEncodedMethod> methodsWithMovedCode;
InterfaceMethodDesugaringLense(
BiMap<DexMethod, DexMethod> methodMapping,
- Map<DexEncodedMethod, DexEncodedMethod> methodsWithMovedCode,
GraphLense previous, DexItemFactory factory) {
super(
ImmutableMap.of(),
@@ -30,12 +25,5 @@
methodMapping.inverse(),
previous,
factory);
- this.methodsWithMovedCode = methodsWithMovedCode;
- }
-
- @Override
- public DexEncodedMethod mapDexEncodedMethod(AppInfo appInfo, DexEncodedMethod original) {
- return super.mapDexEncodedMethod(
- appInfo, methodsWithMovedCode.getOrDefault(original, original));
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
index 7a19fb8..10cbabb 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
@@ -428,7 +428,6 @@
converter.appView.setGraphLense(
new InterfaceMethodDesugaringLense(
processor.movedMethods,
- processor.methodsWithMovedCode,
converter.appView.graphLense(),
factory));
}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 956d6bd..15dcafa 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -424,7 +424,8 @@
return false;
}
DexClass targetClass = appInfo.definitionFor(clazz.type.getSingleSubtype());
- if (clazz.hasClassInitializer() && targetClass.hasClassInitializer()) {
+ if ((clazz.hasClassInitializer() && targetClass.hasClassInitializer())
+ || targetClass.classInitializationMayHaveSideEffects(appInfo, type -> type == clazz.type)) {
// TODO(herhut): Handle class initializers.
if (Log.ENABLED) {
AbortReason.STATIC_INITIALIZERS.printLogMessageForClass(clazz);
diff --git a/src/test/java/com/android/tools/r8/classmerging/StaticInitializerTest.java b/src/test/java/com/android/tools/r8/classmerging/StaticInitializerTest.java
new file mode 100644
index 0000000..ce89fe1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/StaticInitializerTest.java
@@ -0,0 +1,73 @@
+// Copyright (c) 2019, 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;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class StaticInitializerTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public StaticInitializerTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput = StringUtils.lines("In A.m()", "In B.<clinit>()", "In B.m()");
+
+ if (backend == Backend.CF) {
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
+ }
+
+ testForR8(backend)
+ .addInnerClasses(StaticInitializerTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ A.m();
+ B.m();
+ }
+ }
+
+ // Cannot be merged into B because that would change the semantics due to <clinit>.
+ static class A {
+
+ @NeverInline
+ public static void m() {
+ System.out.println("In A.m()");
+ }
+ }
+
+ static class B extends A {
+
+ static {
+ System.out.println("In B.<clinit>()");
+ }
+
+ @NeverInline
+ public static void m() {
+ System.out.println("In B.m()");
+ }
+ }
+}
