Use original static modifier when evaluating -if rules
Bug: 117403482, 122867080, 122875545, 122867087, 120266482
Change-Id: Ieff6fd207cdf187b4787530ee66af7d3a97ec94c
diff --git a/src/main/java/com/android/tools/r8/graph/AccessFlags.java b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
index 09efa690..63c1cde 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
@@ -194,6 +194,10 @@
promote(Constants.ACC_PUBLIC);
}
+ public void promoteToStatic() {
+ promote(Constants.ACC_STATIC);
+ }
+
private boolean wasSet(int flag) {
return (originalFlags & flag) != 0;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 1081eff..44294db 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -685,7 +685,8 @@
public DexEncodedMethod toStaticMethodWithoutThis() {
checkIfObsolete();
assert !accessFlags.isStatic();
- Builder builder = builder(this).setStatic().unsetOptimizationInfo().withoutThisParameter();
+ Builder builder =
+ builder(this).promoteToStatic().unsetOptimizationInfo().withoutThisParameter();
setObsolete();
return builder.build();
}
@@ -1255,8 +1256,8 @@
this.method = method;
}
- public Builder setStatic() {
- this.accessFlags.setStatic();
+ public Builder promoteToStatic() {
+ this.accessFlags.promoteToStatic();
return this;
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
index 6ba9b6a..4f335a6 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
@@ -85,7 +85,7 @@
MethodAccessFlags newFlags = virtual.accessFlags.copy();
newFlags.unsetBridge();
- newFlags.setStatic();
+ newFlags.promoteToStatic();
DexCode dexCode = code.asDexCode();
// We cannot name the parameter "this" because the debugger may omit it due to the method
// actually being static. Instead we prepend it with a special character.
@@ -124,8 +124,7 @@
MethodAccessFlags originalFlags = direct.accessFlags;
MethodAccessFlags newFlags = originalFlags.copy();
if (originalFlags.isPrivate()) {
- newFlags.unsetPrivate();
- newFlags.setPublic();
+ newFlags.promoteToPublic();
}
DexMethod oldMethod = direct.method;
@@ -143,7 +142,7 @@
assert !rewriter.factory.isClassConstructor(oldMethod)
: "Unexpected private constructor " + direct.toSourceString()
+ " in " + iface.origin;
- newFlags.setStatic();
+ newFlags.promoteToStatic();
DexMethod companionMethod = rewriter.privateAsMethodOfCompanionClass(oldMethod);
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/classstaticizer/IfRuleWithClassStaticizerTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/classstaticizer/IfRuleWithClassStaticizerTest.java
new file mode 100644
index 0000000..a12620d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/classstaticizer/IfRuleWithClassStaticizerTest.java
@@ -0,0 +1,113 @@
+// 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.shaking.ifrule.classstaticizer;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+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.FoundMethodSubject;
+import java.util.List;
+import java.util.stream.Collectors;
+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 IfRuleWithClassStaticizerTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public IfRuleWithClassStaticizerTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput = StringUtils.lines("In method()");
+
+ if (backend == Backend.CF) {
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
+ }
+
+ CodeInspector inspector =
+ testForR8(backend)
+ .addInnerClasses(IfRuleWithClassStaticizerTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepRules(
+ "-if class " + StaticizerCandidate.Companion.class.getTypeName() + " {",
+ " public !static void method();",
+ "}",
+ "-keep class " + Unused.class.getTypeName())
+ .enableInliningAnnotations()
+ .enableClassInliningAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput)
+ .inspector();
+
+ ClassSubject classSubject = inspector.clazz(StaticizerCandidate.class);
+ assertThat(classSubject, isPresent());
+
+ if (backend == Backend.CF) {
+ // The class staticizer is not enabled for CF.
+ assertThat(inspector.clazz(Unused.class), isPresent());
+ } else {
+ assert backend == Backend.DEX;
+
+ // There should be a static method on StaticizerCandidate after staticizing.
+ List<FoundMethodSubject> staticMethods =
+ classSubject.allMethods().stream()
+ .filter(method -> method.isStatic() && !method.isClassInitializer())
+ .collect(Collectors.toList());
+ assertEquals(1, staticMethods.size());
+ assertEquals(
+ "void " + StaticizerCandidate.Companion.class.getTypeName() + ".method()",
+ staticMethods.get(0).getOriginalSignature().toString());
+
+ // The Companion class should not be present after staticizing.
+ assertThat(inspector.clazz(StaticizerCandidate.Companion.class), not(isPresent()));
+
+ // TODO(b/122867080): The Unused class should be present due to the -if rule.
+ assertThat(inspector.clazz(Unused.class), not(isPresent()));
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ StaticizerCandidate.companion.method();
+ }
+ }
+
+ @NeverClassInline
+ static class StaticizerCandidate {
+
+ static final Companion companion = new Companion();
+
+ @NeverClassInline
+ static class Companion {
+
+ @NeverInline
+ public void method() {
+ System.out.println("In method()");
+ }
+ }
+ }
+
+ static class Unused {}
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/interfacemethoddesugaring/IfRuleWithInterfaceMethodDesugaringTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/interfacemethoddesugaring/IfRuleWithInterfaceMethodDesugaringTest.java
new file mode 100644
index 0000000..d86bc68
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/interfacemethoddesugaring/IfRuleWithInterfaceMethodDesugaringTest.java
@@ -0,0 +1,105 @@
+// 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.shaking.ifrule.interfacemethoddesugaring;
+
+import static com.android.tools.r8.ir.desugar.InterfaceMethodRewriter.COMPANION_CLASS_NAME_SUFFIX;
+import static com.android.tools.r8.ir.desugar.InterfaceMethodRewriter.DEFAULT_METHOD_PREFIX;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPublic;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isStatic;
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.AndroidApiLevel;
+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.MethodSubject;
+import org.junit.Test;
+
+public class IfRuleWithInterfaceMethodDesugaringTest extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput =
+ StringUtils.lines("In Interface.staticMethod()", "In Interface.virtualMethod()");
+
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
+
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(IfRuleWithInterfaceMethodDesugaringTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepRules(
+ "-if class " + Interface.class.getTypeName() + " {",
+ " !public static void staticMethod();",
+ "}",
+ "-keep class " + Unused1.class.getTypeName(),
+ "-if class " + Interface.class.getTypeName() + " {",
+ " !public !static void virtualMethod();",
+ "}",
+ "-keep class " + Unused2.class.getTypeName())
+ .enableInliningAnnotations()
+ .enableClassInliningAnnotations()
+ .enableMergeAnnotations()
+ .setMinApi(AndroidApiLevel.M)
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput)
+ .inspector();
+
+ ClassSubject classSubject =
+ inspector.clazz(Interface.class.getTypeName() + COMPANION_CLASS_NAME_SUFFIX);
+ assertThat(classSubject, isPresent());
+ assertEquals(2, classSubject.allMethods().size());
+
+ MethodSubject staticMethodSubject = classSubject.uniqueMethodWithName("staticMethod");
+ assertThat(staticMethodSubject, allOf(isPresent(), isPublic(), isStatic()));
+
+ // TODO(b/122867087): Should not be necessary to use `DEFAULT_METHOD_PREFIX`.
+ MethodSubject virtualMethodSubject =
+ classSubject.uniqueMethodWithName(DEFAULT_METHOD_PREFIX + "virtualMethod");
+ assertThat(virtualMethodSubject, allOf(isPresent(), isPublic(), isStatic()));
+
+ // TODO(b/122875545): The Unused class should be present due to the -if rule.
+ assertThat(inspector.clazz(Unused1.class), not(isPresent()));
+ assertThat(inspector.clazz(Unused2.class), not(isPresent()));
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ Interface.staticMethod();
+ new InterfaceImpl().virtualMethod();
+ }
+ }
+
+ @NeverClassInline
+ @NeverMerge
+ interface Interface {
+
+ @NeverInline
+ static void staticMethod() {
+ System.out.println("In Interface.staticMethod()");
+ }
+
+ @NeverInline
+ default void virtualMethod() {
+ System.out.println("In Interface.virtualMethod()");
+ }
+ }
+
+ @NeverClassInline
+ static class InterfaceImpl implements Interface {}
+
+ static class Unused1 {}
+
+ static class Unused2 {}
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
index 8cd5cc7..11ee3af 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
@@ -102,7 +102,7 @@
public MethodSubject uniqueMethodWithName(String name) {
MethodSubject methodSubject = null;
for (FoundMethodSubject candidate : allMethods()) {
- if (candidate.getOriginalName().equals(name)) {
+ if (candidate.getOriginalName(false).equals(name)) {
assert methodSubject == null;
methodSubject = candidate;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
index 4ea2d93..103ffb9 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
@@ -146,6 +146,28 @@
};
}
+ public static Matcher<MemberSubject> isStatic() {
+ return new TypeSafeMatcher<MemberSubject>() {
+ @Override
+ public boolean matchesSafely(final MemberSubject subject) {
+ return subject.isPresent() && subject.isStatic();
+ }
+
+ @Override
+ public void describeTo(final Description description) {
+ description.appendText(" present");
+ }
+
+ @Override
+ public void describeMismatchSafely(final MemberSubject subject, Description description) {
+ description
+ .appendText(type(subject) + " ")
+ .appendValue(name(subject))
+ .appendText(" was not");
+ }
+ };
+ }
+
public static Matcher<Subject> isSynthetic() {
return new TypeSafeMatcher<Subject>() {
@Override
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
index a95b6e6..af1fe14 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
@@ -25,8 +25,22 @@
public abstract Signature getFinalSignature();
public String getOriginalName() {
+ return getOriginalName(true);
+ }
+
+ public String getOriginalName(boolean qualified) {
Signature originalSignature = getOriginalSignature();
- return originalSignature == null ? null : originalSignature.name;
+ if (originalSignature != null) {
+ String name = originalSignature.name;
+ if (!qualified) {
+ int index = name.lastIndexOf(".");
+ if (index >= 0) {
+ return name.substring(index + 1);
+ }
+ }
+ return name;
+ }
+ return null;
}
public String getFinalName() {
