Add a test for not staticizing synchronized methods
Change-Id: I6530b6dd615d47fda74de4a523637db232e2dfa9
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
index 0372a33..0231d43 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
@@ -15,7 +15,6 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.ResolutionResult;
@@ -38,7 +37,6 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.ListIterator;
@@ -132,23 +130,28 @@
DexType type = field.field.type;
if (singletonFields.put(type, field) != null) {
// There is already candidate singleton field found.
- notEligible.add(type);
+ markNotEligible(type, notEligible);
}
}
// Don't allow fields with this candidate types.
for (DexEncodedField field : cls.instanceFields()) {
- notEligible.add(field.field.type);
+ markNotEligible(field.field.type, notEligible);
}
// Don't allow methods that take a value of this type.
for (DexEncodedMethod method : cls.methods()) {
- DexProto proto = method.method.proto;
- notEligible.addAll(Arrays.asList(proto.parameters.values));
+ for (DexType parameter : method.getProto().parameters.values) {
+ markNotEligible(parameter, notEligible);
+ }
+ if (method.isSynchronized()) {
+ markNotEligible(cls.type, notEligible);
+ }
}
// High-level limitations on what classes we consider eligible.
- if (cls.isInterface() // Must not be an interface or an abstract class.
+ if (cls.isInterface()
+ // Must not be an interface or an abstract class.
|| cls.accessFlags.isAbstract()
// Don't support candidates with instance fields
|| cls.instanceFields().size() > 0
@@ -159,7 +162,7 @@
// Staticizing classes implementing interfaces is more
// difficult, so don't support it until we really need it.
|| !cls.interfaces.isEmpty()) {
- notEligible.add(cls.type);
+ markNotEligible(cls.type, notEligible);
}
});
@@ -180,6 +183,12 @@
});
}
+ private void markNotEligible(DexType type, Set<DexType> notEligible) {
+ if (type.isClassType()) {
+ notEligible.add(type);
+ }
+ }
+
private boolean isPinned(DexClass clazz, DexEncodedField singletonField) {
AppInfoWithLiveness appInfo = appView.appInfo();
if (appInfo.isPinned(clazz.type) || appInfo.isPinned(singletonField.field)) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/SynchronizedCompanionMethodTest.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/SynchronizedCompanionMethodTest.java
new file mode 100644
index 0000000..8df91f6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/SynchronizedCompanionMethodTest.java
@@ -0,0 +1,85 @@
+// 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.ir.optimize.staticizer;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class SynchronizedCompanionMethodTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public SynchronizedCompanionMethodTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ private void inspect(CodeInspector inspector) {
+ ClassSubject testClassSubject = inspector.clazz(TestClass.class);
+ assertThat(testClassSubject, isPresent());
+ assertEquals(
+ 1,
+ testClassSubject
+ .allMethods(method -> method.isStatic() && !method.isClassInitializer())
+ .size());
+
+ ClassSubject companionClassSubject = inspector.clazz(Companion.class);
+ assertThat(companionClassSubject, isPresent());
+ assertEquals(
+ 1,
+ companionClassSubject
+ .allMethods(method -> !method.isStatic() && method.isSynchronized())
+ .size());
+ }
+
+ static class TestClass {
+
+ static final Companion companion = new Companion();
+
+ public static void main(String[] args) {
+ companion.greet();
+ }
+ }
+
+ @NeverClassInline
+ static class Companion {
+
+ @NeverInline
+ public synchronized void greet() {
+ System.out.println("Hello world!");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
index c62f945..f81dffd 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
@@ -74,6 +74,11 @@
}
@Override
+ public boolean isSynchronized() {
+ throw new Unreachable("Cannot determine if an absent method is synchronized");
+ }
+
+ @Override
public boolean isInstanceInitializer() {
throw new Unreachable("Cannot determine if an absent method is an instance initializer");
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
index 299a6c0..536aa5e3 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
@@ -122,6 +122,11 @@
}
@Override
+ public boolean isSynchronized() {
+ return dexMethod.accessFlags.isSynchronized();
+ }
+
+ @Override
public boolean isInstanceInitializer() {
return dexMethod.isInstanceInitializer();
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
index 690838d..3c86361 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
@@ -25,6 +25,8 @@
public abstract boolean isBridge();
+ public abstract boolean isSynchronized();
+
public abstract boolean isInstanceInitializer();
public abstract boolean isClassInitializer();
