Merge "Publicize private instance methods to public instance methods, phase 1."
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 13f2404..3d2cc25 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -320,16 +320,17 @@
timing.end();
}
+ GraphLense graphLense = GraphLense.getIdentityLense();
+
if (options.proguardConfiguration.isAccessModificationAllowed()) {
- ClassAndMemberPublicizer.run(application, appInfo.dexItemFactory);
+ graphLense = ClassAndMemberPublicizer.run(
+ executorService, timing, application, appInfo, rootSet, graphLense);
// We can now remove visibility bridges. Note that we do not need to update the
// invoke-targets here, as the existing invokes will simply dispatch to the now
// visible super-method. MemberRebinding, if run, will then dispatch it correctly.
application = new VisibilityBridgeRemover(appInfo, application).run();
}
- GraphLense graphLense = GraphLense.getIdentityLense();
-
if (appInfo.hasLiveness()) {
if (options.proguardConfiguration.hasApplyMappingFile()) {
SeedMapper seedMapper =
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 2c188da..64c85f3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -13,6 +13,7 @@
import com.google.common.collect.Iterators;
import java.util.Arrays;
import java.util.List;
+import java.util.Set;
import java.util.function.Consumer;
public abstract class DexClass extends DexItem {
@@ -158,6 +159,32 @@
return result;
}
+ public void virtualizeMethods(Set<DexEncodedMethod> privateInstanceMethods) {
+ int vLen = virtualMethods.length;
+ int dLen = directMethods.length;
+ int mLen = privateInstanceMethods.size();
+ assert mLen <= dLen;
+
+ DexEncodedMethod[] newDirectMethods = new DexEncodedMethod[dLen - mLen];
+ int index = 0;
+ for (int i = 0; i < dLen; i++) {
+ DexEncodedMethod encodedMethod = directMethods[i];
+ if (!privateInstanceMethods.contains(encodedMethod)) {
+ newDirectMethods[index++] = encodedMethod;
+ }
+ }
+ assert index == dLen - mLen;
+ setDirectMethods(newDirectMethods);
+
+ DexEncodedMethod[] newVirtualMethods = new DexEncodedMethod[vLen + mLen];
+ System.arraycopy(virtualMethods, 0, newVirtualMethods, 0, vLen);
+ index = vLen;
+ for (DexEncodedMethod encodedMethod : privateInstanceMethods) {
+ newVirtualMethods[index++] = encodedMethod;
+ }
+ setVirtualMethods(newVirtualMethods);
+ }
+
/**
* For all annotations on the class and all annotations on its methods and fields apply the
* specified consumer.
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 83cb824..845eecd 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -160,6 +160,10 @@
return isVirtualMethod() && !accessFlags.isAbstract();
}
+ public boolean isPublicized() {
+ return optimizationInfo.isPublicized();
+ }
+
public boolean isPublicMethod() {
return accessFlags.isPublic();
}
@@ -669,6 +673,7 @@
private boolean neverReturnsNormally = false;
private boolean returnsConstant = false;
private long returnedConstant = 0;
+ private boolean publicized = false;
private boolean forceInline = false;
private boolean useIdentifierNameString = false;
private boolean checksNullReceiverBeforeAnySideEffect = false;
@@ -689,6 +694,7 @@
neverReturnsNull = template.neverReturnsNull;
returnsConstant = template.returnsConstant;
returnedConstant = template.returnedConstant;
+ publicized = template.publicized;
forceInline = template.forceInline;
useIdentifierNameString = template.useIdentifierNameString;
checksNullReceiverBeforeAnySideEffect = template.checksNullReceiverBeforeAnySideEffect;
@@ -752,6 +758,10 @@
return returnedConstant;
}
+ public boolean isPublicized() {
+ return publicized;
+ }
+
public boolean forceInline() {
return forceInline;
}
@@ -792,6 +802,10 @@
forceInline = true;
}
+ private void markPublicized() {
+ publicized = true;
+ }
+
private void markUseIdentifierNameString() {
useIdentifierNameString = true;
}
@@ -865,6 +879,10 @@
ensureMutableOI().markForceInline();
}
+ synchronized public void markPublicized() {
+ ensureMutableOI().markPublicized();
+ }
+
synchronized public void markUseIdentifierNameString() {
ensureMutableOI().markUseIdentifierNameString();
}
diff --git a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
index 297dd57..eaeb1ee 100644
--- a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
+++ b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
@@ -3,65 +3,179 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.optimize;
+import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
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.DexType;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.MethodAccessFlags;
+import com.android.tools.r8.optimize.PublicizerLense.PublicizedLenseBuilder;
+import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
+import com.android.tools.r8.utils.MethodSignatureEquivalence;
+import com.android.tools.r8.utils.ThreadUtils;
+import com.android.tools.r8.utils.Timing;
+import com.google.common.base.Equivalence;
+import com.google.common.base.Equivalence.Wrapper;
+import com.google.common.collect.Sets;
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
public final class ClassAndMemberPublicizer {
- private final DexItemFactory factory;
private final DexApplication application;
+ private final AppInfo appInfo;
+ private final RootSet rootSet;
+ private final GraphLense previuosLense;
+ private final PublicizedLenseBuilder lenseBuilder;
- private ClassAndMemberPublicizer(DexApplication application, DexItemFactory factory) {
+ private final Equivalence<DexMethod> equivalence = MethodSignatureEquivalence.get();
+ // TODO(b/72109068): finer-grained naming spaces, e.g., per-tree.
+ private final Set<Wrapper<DexMethod>> methodPool = Sets.newConcurrentHashSet();
+
+ private ClassAndMemberPublicizer(
+ DexApplication application,
+ AppInfo appInfo,
+ RootSet rootSet,
+ GraphLense previousLense) {
this.application = application;
- this.factory = factory;
+ this.appInfo = appInfo;
+ this.rootSet = rootSet;
+ this.previuosLense = previousLense;
+ lenseBuilder = PublicizerLense.createBuilder();
}
/**
- * Marks all package private and protected methods and fields as public. Also makes
- * all private static methods public.
+ * Marks all package private and protected methods and fields as public.
+ * Makes all private static methods public.
+ * Makes private instance methods public final instance methods, if possible.
* <p>
* This will destructively update the DexApplication passed in as argument.
*/
- public static DexApplication run(DexApplication application, DexItemFactory factory) {
- return new ClassAndMemberPublicizer(application, factory).run();
+ public static GraphLense run(
+ ExecutorService executorService,
+ Timing timing,
+ DexApplication application,
+ AppInfo appInfo,
+ RootSet rootSet,
+ GraphLense previousLense) throws ExecutionException {
+ return new ClassAndMemberPublicizer(application, appInfo, rootSet, previousLense)
+ .run(executorService, timing);
}
- private DexApplication run() {
- for (DexClass clazz : application.classes()) {
+ private GraphLense run(ExecutorService executorService, Timing timing)
+ throws ExecutionException {
+ // Phase 1: Collect methods to check if private instance methods don't have conflicts.
+ timing.begin("Phase 1: collectMethods");
+ try {
+ List<Future<?>> futures = new ArrayList<>();
+ // TODO(b/72109068): finer-grained naming spaces will need a different class visiting.
+ application.classes().forEach(clazz ->
+ futures.add(executorService.submit(collectMethodPerClass(clazz))));
+ ThreadUtils.awaitFutures(futures);
+ } finally {
+ timing.end();
+ }
+
+ // Phase 2: Visit classes and promote class/member to public if possible.
+ timing.begin("Phase 2: promoteToPublic");
+ DexType.forAllInterfaces(appInfo.dexItemFactory, this::publicizeType);
+ publicizeType(appInfo.dexItemFactory.objectType);
+ timing.end();
+
+ return lenseBuilder.build(appInfo, previuosLense);
+ }
+
+ private Runnable collectMethodPerClass(DexClass clazz) {
+ return () -> {
+ clazz.forEachMethod(encodedMethod -> {
+ // We will add private instance methods when we promote them.
+ if (!encodedMethod.isPrivateMethod() || encodedMethod.isStaticMethod()) {
+ methodPool.add(equivalence.wrap(encodedMethod.method));
+ }
+ });
+ };
+ }
+
+ private void publicizeType(DexType type) {
+ DexClass clazz = application.definitionFor(type);
+ if (clazz != null && clazz.isProgramClass()) {
clazz.accessFlags.promoteToPublic();
- clazz.forEachMethod(this::publicizeMethod);
clazz.forEachField(field -> field.accessFlags.promoteToPublic());
+ Set<DexEncodedMethod> privateInstanceEncodedMethods = new LinkedHashSet<>();
+ clazz.forEachMethod(encodedMethod -> {
+ if (publicizeMethod(clazz, encodedMethod)) {
+ privateInstanceEncodedMethods.add(encodedMethod);
+ }
+ });
+ if (!privateInstanceEncodedMethods.isEmpty()) {
+ clazz.virtualizeMethods(privateInstanceEncodedMethods);
+ }
}
- return application;
+
+ // TODO(b/72109068): Can process sub types in parallel.
+ type.forAllExtendsSubtypes(this::publicizeType);
}
- private void publicizeMethod(DexEncodedMethod method) {
- MethodAccessFlags accessFlags = method.accessFlags;
+ private boolean publicizeMethod(DexClass holder, DexEncodedMethod encodedMethod) {
+ MethodAccessFlags accessFlags = encodedMethod.accessFlags;
if (accessFlags.isPublic()) {
- return;
+ return false;
}
- if (factory.isClassConstructor(method.method)) {
- return;
+ if (appInfo.dexItemFactory.isClassConstructor(encodedMethod.method)) {
+ return false;
}
if (!accessFlags.isPrivate()) {
accessFlags.unsetProtected();
accessFlags.setPublic();
- return;
+ return false;
}
assert accessFlags.isPrivate();
- if (factory.isConstructor(method.method)) {
- // TODO: b/72211928
- return;
+ if (appInfo.dexItemFactory.isConstructor(encodedMethod.method)) {
+ // TODO(b/72211928)
+ return false;
}
if (!accessFlags.isStatic()) {
- // TODO: b/72109068
- return;
+ // If this method is mentioned in keep rules, do not transform (rule applications changed).
+ if (rootSet.noShrinking.containsKey(encodedMethod)) {
+ return false;
+ }
+
+ // We can't publicize private instance methods in interfaces or methods that are copied from
+ // interfaces to lambda-desugared classes because this will be added as a new default method.
+ // TODO(b/72109068): It might be possible to transform it into static methods, though.
+ if (holder.isInterface() || accessFlags.isSynthetic()) {
+ return false;
+ }
+
+ Wrapper<DexMethod> key = equivalence.wrap(encodedMethod.method);
+ if (methodPool.contains(key)) {
+ // We can't do anything further because even renaming is not allowed due to the keep rule.
+ if (rootSet.noObfuscation.contains(encodedMethod)) {
+ return false;
+ }
+ // TODO(b/72109068): Renaming will enable more private instance methods to be publicized.
+ return false;
+ }
+ methodPool.add(key);
+ lenseBuilder.add(encodedMethod.method);
+ accessFlags.unsetPrivate();
+ accessFlags.setFinal();
+ accessFlags.setPublic();
+ // Although the current method became public, it surely has the single virtual target.
+ encodedMethod.method.setSingleVirtualMethodCache(
+ encodedMethod.method.getHolder(), encodedMethod);
+ encodedMethod.markPublicized();
+ return true;
}
// For private static methods we can just relax the access to private, since
@@ -70,5 +184,6 @@
// does not take into account access of the static methods.
accessFlags.unsetPrivate();
accessFlags.setPublic();
+ return false;
}
}
diff --git a/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java b/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java
new file mode 100644
index 0000000..835135e
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/optimize/PublicizerLense.java
@@ -0,0 +1,68 @@
+// Copyright (c) 2018, 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.optimize;
+
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
+import com.android.tools.r8.ir.code.Invoke.Type;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.Set;
+
+final class PublicizerLense extends NestedGraphLense {
+ private final AppInfo appInfo;
+ private final Set<DexMethod> publicizedMethods;
+
+ PublicizerLense(
+ AppInfo appInfo, GraphLense previousLense, Set<DexMethod> publicizedMethods) {
+ // This lense does not map any DexItem's at all.
+ // It will just tweak invoke type for publicized methods from invoke-direct to invoke-virtual.
+ super(ImmutableMap.of(), ImmutableMap.of(), ImmutableMap.of(),
+ previousLense, appInfo.dexItemFactory);
+ this.appInfo = appInfo;
+ this.publicizedMethods = publicizedMethods;
+ }
+
+ @Override
+ public GraphLenseLookupResult lookupMethod(
+ DexMethod method, DexEncodedMethod context, Type type) {
+ GraphLenseLookupResult previous = previousLense.lookupMethod(method, context, type);
+ method = previous.getMethod();
+ type = previous.getType();
+ if (type == Type.DIRECT && publicizedMethods.contains(method)) {
+ DexClass holderClass = appInfo.definitionFor(method.holder);
+ if (holderClass != null) {
+ DexEncodedMethod actualEncodedTarget = holderClass.lookupVirtualMethod(method);
+ if (actualEncodedTarget != null
+ && actualEncodedTarget.isPublicized()) {
+ return new GraphLenseLookupResult(method, Type.VIRTUAL);
+ }
+ }
+ }
+ return super.lookupMethod(method, context, type);
+ }
+
+ static PublicizedLenseBuilder createBuilder() {
+ return new PublicizedLenseBuilder();
+ }
+
+ static class PublicizedLenseBuilder {
+ private final ImmutableSet.Builder<DexMethod> methodSetBuilder = ImmutableSet.builder();
+
+ private PublicizedLenseBuilder() {
+ }
+
+ public GraphLense build(AppInfo appInfo, GraphLense previousLense) {
+ return new PublicizerLense(appInfo, previousLense, methodSetBuilder.build());
+ }
+
+ public void add(DexMethod publicizedMethod) {
+ methodSetBuilder.add(publicizedMethod);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/AccessRelaxationTest.java b/src/test/java/com/android/tools/r8/accessrelaxation/AccessRelaxationTest.java
index 34b92b0..027b7e8 100644
--- a/src/test/java/com/android/tools/r8/accessrelaxation/AccessRelaxationTest.java
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/AccessRelaxationTest.java
@@ -4,35 +4,89 @@
package com.android.tools.r8.accessrelaxation;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPublic;
+import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.VmTestRunner;
+import com.android.tools.r8.accessrelaxation.privateinstance.Base;
+import com.android.tools.r8.accessrelaxation.privateinstance.Sub1;
+import com.android.tools.r8.accessrelaxation.privateinstance.Sub2;
+import com.android.tools.r8.accessrelaxation.privateinstance.TestMain;
import com.android.tools.r8.accessrelaxation.privatestatic.A;
import com.android.tools.r8.accessrelaxation.privatestatic.B;
import com.android.tools.r8.accessrelaxation.privatestatic.BB;
import com.android.tools.r8.accessrelaxation.privatestatic.C;
+import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
import com.google.common.collect.ImmutableList;
import org.junit.Test;
+import org.junit.runner.RunWith;
+@RunWith(VmTestRunner.class)
public class AccessRelaxationTest extends TestBase {
- @Test
- public void testStaticMethodRelaxation() throws Exception {
+ private static final String STRING = "java.lang.String";
+
+ private static R8Command.Builder loadProgramFiles(Package p, Class... classes) throws Exception {
R8Command.Builder builder = R8Command.builder();
- builder.addProgramFiles(ToolHelper.getClassFilesForTestPackage(A.class.getPackage()));
+ builder.addProgramFiles(ToolHelper.getClassFilesForTestPackage(p));
+ for (Class clazz : classes) {
+ builder.addProgramFiles(ToolHelper.getClassFileForTestClass(clazz));
+ }
builder.setProgramConsumer(DexIndexedConsumer.emptyConsumer());
builder.setMinApiLevel(ToolHelper.getMinApiLevelForDexVm().getLevel());
+ return builder;
+ }
+
+ private void compareJvmAndArt(AndroidApp app, Class mainClass) throws Exception {
+ // Run on Jvm.
+ String jvmOutput = runOnJava(mainClass);
+
+ // Run on Art to check generated code against verifier.
+ String artOutput = runOnArt(app, mainClass);
+
+ String adjustedArtOutput = artOutput.replace(
+ "java.lang.IncompatibleClassChangeError", "java.lang.IllegalAccessError");
+ assertEquals(jvmOutput, adjustedArtOutput);
+ }
+
+ private static void assertPublic(
+ DexInspector dexInspector, Class clazz, MethodSignature signature) {
+ ClassSubject classSubject = dexInspector.clazz(clazz);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(signature);
+ assertThat(methodSubject, isPublic());
+ }
+
+ private static void assertNotPublic(
+ DexInspector dexInspector, Class clazz, MethodSignature signature) {
+ ClassSubject classSubject = dexInspector.clazz(clazz);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(signature);
+ assertThat(methodSubject, not(isPublic()));
+ }
+
+ @Test
+ public void testStaticMethodRelaxation() throws Exception {
+ Class mainClass = C.class;
+ R8Command.Builder builder = loadProgramFiles(mainClass.getPackage());
// Note: we use '-checkdiscard' to indirectly check that the access relaxation is
// done which leads to inlining of all pB*** methods so they are removed. Without
// access relaxation inlining is not performed and method are kept.
builder.addProguardConfiguration(
ImmutableList.of(
- "-keep class " + C.class.getCanonicalName() + "{",
+ "-keep class " + mainClass.getCanonicalName() + "{",
" public static void main(java.lang.String[]);",
"}",
"",
@@ -57,15 +111,71 @@
Origin.unknown());
AndroidApp app = ToolHelper.runR8(builder.build());
+ compareJvmAndArt(app, mainClass);
- // Run on Jvm.
- String jvmOutput = runOnJava(C.class);
+ DexInspector dexInspector = new DexInspector(app);
+ assertPublic(dexInspector, A.class,
+ new MethodSignature("baz", STRING, ImmutableList.of()));
+ assertPublic(dexInspector, A.class,
+ new MethodSignature("bar", STRING, ImmutableList.of()));
+ assertPublic(dexInspector, A.class,
+ new MethodSignature("bar", STRING, ImmutableList.of("int")));
+ assertPublic(dexInspector, A.class,
+ new MethodSignature("blah", STRING, ImmutableList.of("int")));
- // Run on Art to check generated code against verifier.
- String artOutput = runOnArt(app, C.class);
+ assertPublic(dexInspector, B.class,
+ new MethodSignature("blah", STRING, ImmutableList.of("int")));
- String adjustedArtOutput = artOutput.replace(
- "java.lang.IncompatibleClassChangeError", "java.lang.IllegalAccessError");
- assertEquals(jvmOutput, adjustedArtOutput);
+ assertPublic(dexInspector, BB.class,
+ new MethodSignature("blah", STRING, ImmutableList.of("int")));
+ }
+
+ @Test
+ public void testInstanceMethodRelaxation() throws Exception {
+ Class mainClass = TestMain.class;
+ R8Command.Builder builder = loadProgramFiles(mainClass.getPackage());
+
+ builder.addProguardConfiguration(
+ ImmutableList.of(
+ "-keep class " + mainClass.getCanonicalName() + "{",
+ " public static void main(java.lang.String[]);",
+ "}",
+ "",
+ "-checkdiscard class " + Base.class.getCanonicalName() + "{",
+ " *** p*();",
+ "}",
+ "",
+ "-checkdiscard class " + Sub1.class.getCanonicalName() + "{",
+ " *** p*();",
+ "}",
+ "",
+ "-checkdiscard class " + Sub2.class.getCanonicalName() + "{",
+ " *** p*();",
+ "}",
+ "",
+ "-dontobfuscate",
+ "-allowaccessmodification"
+ ),
+ Origin.unknown());
+
+ AndroidApp app = ToolHelper.runR8(builder.build());
+ compareJvmAndArt(app, mainClass);
+
+ DexInspector dexInspector = new DexInspector(app);
+ assertPublic(dexInspector, Base.class,
+ new MethodSignature("foo", STRING, ImmutableList.of()));
+
+ // Base#foo?() can't be publicized due to Itf<1>#foo<1>().
+ assertNotPublic(dexInspector, Base.class,
+ new MethodSignature("foo1", STRING, ImmutableList.of()));
+ assertNotPublic(dexInspector, Base.class,
+ new MethodSignature("foo2", STRING, ImmutableList.of()));
+
+ // Sub1#bar1(int) can't be publicized due to Base#bar1(int).
+ assertNotPublic(dexInspector, Sub1.class,
+ new MethodSignature("bar1", STRING, ImmutableList.of("int")));
+
+ assertPublic(dexInspector, Sub2.class,
+ new MethodSignature("bar2", STRING, ImmutableList.of("int")));
}
}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/InvokeTypeConversionTest.java b/src/test/java/com/android/tools/r8/accessrelaxation/InvokeTypeConversionTest.java
new file mode 100644
index 0000000..fce1f4c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/InvokeTypeConversionTest.java
@@ -0,0 +1,143 @@
+// Copyright (c) 2018, 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.accessrelaxation;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.VmTestRunner;
+import com.android.tools.r8.code.InvokeDirect;
+import com.android.tools.r8.code.InvokeVirtual;
+import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.smali.SmaliBuilder;
+import com.android.tools.r8.smali.SmaliBuilder.MethodSignature;
+import com.android.tools.r8.smali.SmaliTestBase;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import java.util.function.Consumer;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(VmTestRunner.class)
+public class InvokeTypeConversionTest extends SmaliTestBase {
+ private final String CLASS_NAME = "Example";
+ private MethodSignature main;
+
+ private SmaliBuilder buildTestClass(String invokeLine) {
+ SmaliBuilder builder = new SmaliBuilder(CLASS_NAME);
+ builder.addDefaultConstructor();
+ builder.addPrivateInstanceMethod("int", "foo", ImmutableList.of(), 1,
+ " const/4 v0, 0",
+ " return v0");
+ builder.addPrivateStaticMethod("int", "bar", ImmutableList.of(), 1,
+ " const/4 v0, 0",
+ " return v0");
+ main = builder.addMainMethod(2,
+ "new-instance v1, L" + CLASS_NAME + ";",
+ "invoke-direct { v1 }, L" + CLASS_NAME + ";-><init>()V",
+ invokeLine,
+ "move-result v1",
+ "invoke-static { v1 }, Ljava/lang/Integer;->valueOf(I)Ljava/lang/Integer;",
+ "move-result-object v1",
+ "sget-object v0, Ljava/lang/System;->out:Ljava/io/PrintStream;",
+ "invoke-virtual { v0, v1 }, Ljava/io/PrintStream;->print(Ljava/lang/Object;)V",
+ "return-void");
+ return builder;
+ }
+
+ private void run(
+ SmaliBuilder builder,
+ String expectedException,
+ Consumer<DexInspector> inspectorConsumer) throws Exception {
+ AndroidApp app = buildApplication(builder);
+ List<String> pgConfigs = ImmutableList.of(
+ keepMainProguardConfiguration(CLASS_NAME),
+ "-printmapping",
+ "-dontobfuscate",
+ "-allowaccessmodification");
+ R8Command.Builder command = ToolHelper.prepareR8CommandBuilder(app);
+ command.addProguardConfiguration(pgConfigs, Origin.unknown());
+ AndroidApp processedApp = ToolHelper.runR8(command.build(), o -> {
+ o.enableInlining = false;
+ });
+ ProcessResult artResult = runOnArtRaw(processedApp, CLASS_NAME);
+ if (expectedException == null) {
+ assertEquals(0, artResult.exitCode);
+ assertEquals("0", artResult.stdout);
+ } else {
+ assertEquals(1, artResult.exitCode);
+ assertThat(artResult.stderr, containsString(expectedException));
+ }
+ DexInspector inspector = new DexInspector(processedApp);
+ inspectorConsumer.accept(inspector);
+ }
+
+ // The following test checks invoke-direct, which refers to the private static method, is *not*
+ // rewritten by publicizer lense, resulting in IncompatibleClassChangeError, which is expected.
+ //
+ // class Example {
+ // private int foo() { return 0; }
+ // private static int bar() { return 0; }
+ // public static void main(String[] args) {
+ // Example instance = new Example();
+ // "invoke-direct instance, Example->bar()" <- should yield IncompatibleClassChangeError
+ // ...
+ // }
+ // }
+ @Test
+ public void invokeDirectToAlreadyStaticMethod() throws Exception {
+ SmaliBuilder builder = buildTestClass(
+ "invoke-direct { v1 }, L" + CLASS_NAME + ";->bar()I");
+ run(builder, "IncompatibleClassChangeError", dexInspector -> {
+ ClassSubject clazz = dexInspector.clazz(CLASS_NAME);
+ assertThat(clazz, isPresent());
+ DexEncodedMethod method = getMethod(dexInspector, main);
+ assertNotNull(method);
+ DexCode code = method.getCode().asDexCode();
+ // The given invoke line is remained as-is.
+ assertTrue(code.instructions[2] instanceof InvokeDirect);
+ });
+ }
+
+ // The following test checks invoke-direct, which refers to the private instance method, *is*
+ // rewritten by publicizer lense, as the target method will be publicized.
+ //
+ // class Example {
+ // private int foo() { return 0; }
+ // private static int bar() { return 0; }
+ // public static void main(String[] args) {
+ // Example instance = new Example();
+ // instance.foo(); // which was "invoke-direct instance, Example->foo()"
+ // // will be "invoke-virtual instance, Example->foo()"
+ // ...
+ // }
+ // }
+ @Test
+ public void invokeDirectToPublicizedMethod() throws Exception {
+ SmaliBuilder builder = buildTestClass(
+ "invoke-direct { v1 }, L" + CLASS_NAME + ";->foo()I");
+ run(builder, null, dexInspector -> {
+ ClassSubject clazz = dexInspector.clazz(CLASS_NAME);
+ assertThat(clazz, isPresent());
+ DexEncodedMethod method = getMethod(dexInspector, main);
+ assertNotNull(method);
+ DexCode code = method.getCode().asDexCode();
+ // The given invoke line is changed to invoke-virtual
+ assertTrue(code.instructions[2] instanceof InvokeVirtual);
+ });
+ }
+
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Base.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Base.java
new file mode 100644
index 0000000..fd1fd1c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Base.java
@@ -0,0 +1,48 @@
+// Copyright (c) 2018, 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.accessrelaxation.privateinstance;
+
+public class Base {
+
+ // NOTE: here and below 'synchronized' is supposed to disable inlining of this method.
+ private synchronized String foo() {
+ return "Base::foo()";
+ }
+
+ public String pFoo() {
+ return foo();
+ }
+
+ private synchronized String foo1() {
+ return "Base::foo1()";
+ }
+
+ public String pFoo1() {
+ return foo1();
+ }
+
+ private synchronized String foo2() {
+ return "Base::foo2()";
+ }
+
+ public String pFoo2() {
+ return foo2();
+ }
+
+ private synchronized String bar1(int i) {
+ throw new AssertionError("Sub1#bar1(int) will not use this signature.");
+ }
+
+ public void dump() {
+ System.out.println(foo());
+ System.out.println(foo1());
+ System.out.println(foo2());
+ try {
+ bar1(0);
+ } catch (AssertionError e) {
+ // expected
+ }
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf1.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf1.java
new file mode 100644
index 0000000..bc44973
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf1.java
@@ -0,0 +1,12 @@
+// Copyright (c) 2018, 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.accessrelaxation.privateinstance;
+
+public interface Itf1 {
+ String foo1();
+
+ default String foo1(int i) {
+ return "Itf1::foo1(" + i + ") >> " + foo1();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf2.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf2.java
new file mode 100644
index 0000000..233a327
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Itf2.java
@@ -0,0 +1,12 @@
+// Copyright (c) 2018, 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.accessrelaxation.privateinstance;
+
+public interface Itf2 {
+ String foo2();
+
+ default String foo2(int i) {
+ return "Itf2::foo2(" + i + ") >> " + foo2();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java
new file mode 100644
index 0000000..4cbbf11
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java
@@ -0,0 +1,32 @@
+// Copyright (c) 2018, 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.accessrelaxation.privateinstance;
+
+public class Sub1 extends Base implements Itf1 {
+
+ @Override
+ public String foo1() {
+ return "Sub1::foo1()";
+ }
+
+ private synchronized String bar1(int i) {
+ return "Sub1::bar1(" + i + ")";
+ }
+
+ public String pBar1() {
+ return bar1(1);
+ }
+
+ @Override
+ public void dump() {
+ System.out.println(foo1());
+ System.out.println(foo1(0));
+ try {
+ System.out.println(bar1(0));
+ } catch (Throwable e) {
+ System.out.println(e.getClass().getName());
+ }
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java
new file mode 100644
index 0000000..8c4e01e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java
@@ -0,0 +1,28 @@
+// Copyright (c) 2018, 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.accessrelaxation.privateinstance;
+
+public class Sub2 extends Base implements Itf2 {
+
+ @Override
+ public String foo2() {
+ return "Sub2::foo2()";
+ }
+
+ private synchronized String bar2(int i) {
+ return "Sub2::bar2(" + i + ")";
+ }
+
+ public String pBar2() {
+ return bar2(2);
+ }
+
+ @Override
+ public void dump() {
+ System.out.println(foo2());
+ System.out.println(foo2(0));
+ System.out.println(bar2(0));
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/TestMain.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/TestMain.java
new file mode 100644
index 0000000..bfd322b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/TestMain.java
@@ -0,0 +1,13 @@
+// Copyright (c) 2018, 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.accessrelaxation.privateinstance;
+
+public class TestMain {
+
+ public static void main(String[] args) {
+ new Base().dump();
+ new Sub1().dump();
+ new Sub2().dump();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierTest.java b/src/test/java/com/android/tools/r8/naming/MinifierTest.java
index 4b42880..8a5975ea 100644
--- a/src/test/java/com/android/tools/r8/naming/MinifierTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MinifierTest.java
@@ -98,10 +98,10 @@
// method naming001.A.m should be kept, according to the keep rule.
assertEquals("m", naming.lookupName(m).toSourceString());
+ // method naming001.A.privateFunc is transformed to a public method, and then kept.
DexMethod p = dexItemFactory.createMethod(
a, dexItemFactory.createProto(dexItemFactory.voidType), "privateFunc");
- // method naming001.A.privateFunc would be renamed.
- assertNotEquals("privateFunc", naming.lookupName(p).toSourceString());
+ assertEquals("privateFunc", naming.lookupName(p).toSourceString());
}
private static void test001_rule005(DexItemFactory dexItemFactory, NamingLens naming) {
diff --git a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
index f311c15..6d16184 100644
--- a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
+++ b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.optimize.ClassAndMemberPublicizer;
import com.android.tools.r8.shaking.Enqueuer;
import com.android.tools.r8.shaking.ProguardConfiguration;
@@ -57,7 +58,7 @@
}
@Before
- public void readApp() throws IOException, ExecutionException, ProguardRuleParserException {
+ public void readApp() throws IOException, ExecutionException {
program = ToolHelper.buildApplication(ImmutableList.of(appFileName));
dexItemFactory = program.dexItemFactory;
appInfo = new AppInfoWithSubtyping(program);
@@ -70,13 +71,18 @@
InternalOptions options = new InternalOptions(configuration,
new Reporter(new DefaultDiagnosticsHandler()));
- if (options.proguardConfiguration.isAccessModificationAllowed()) {
- ClassAndMemberPublicizer.run(program, dexItemFactory);
- }
-
ExecutorService executor = ThreadUtils.getExecutorService(1);
+
RootSet rootSet = new RootSetBuilder(appInfo, program, configuration.getRules(), options)
.run(executor);
+
+ if (options.proguardConfiguration.isAccessModificationAllowed()) {
+ ClassAndMemberPublicizer.run(
+ executor, timing, program, appInfo, rootSet, GraphLense.getIdentityLense());
+ rootSet =
+ new RootSetBuilder(appInfo, program, configuration.getRules(), options).run(executor);
+ }
+
Enqueuer enqueuer = new Enqueuer(appInfo, options, options.forceProguardCompatibility);
appInfo = enqueuer.traceApplication(rootSet, executor, timing);
return new Minifier(appInfo.withLiveness(), rootSet, options).run(timing);
diff --git a/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java b/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
index 1f9c20f..3e46098 100644
--- a/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
+++ b/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
@@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableList;
import java.util.Iterator;
import java.util.function.BiConsumer;
+import org.junit.Ignore;
import org.junit.Test;
public class NeverReturnsNormallyTest extends TestBase {
@@ -127,6 +128,7 @@
return instructions.next();
}
+ @Ignore("b/110736241")
@Test
public void test() throws Exception {
runTest(this::validate, CompilationMode.DEBUG);
diff --git a/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java b/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
index e818801..1dffcee 100644
--- a/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
+++ b/src/test/java/com/android/tools/r8/smali/SmaliBuilder.java
@@ -257,6 +257,12 @@
return addStaticMethod(returnType, name, parameters, locals, buildCode(instructions));
}
+ public MethodSignature addPrivateStaticMethod(
+ String returnType, String name, List<String> parameters, int locals, String... instructions) {
+ return addMethod(
+ "private static", returnType, name, parameters, locals, buildCode(instructions));
+ }
+
public MethodSignature addStaticMethod(
String returnType, String name, List<String> parameters, int locals, String code) {
return addStaticMethod("", returnType, name, parameters, locals, code);
@@ -301,6 +307,11 @@
"public constructor", "void", "<init>", parameters, locals, buildCode(instructions));
}
+ public MethodSignature addPrivateInstanceMethod(
+ String returnType, String name, List<String> parameters, int locals, String... instructions) {
+ return addMethod("private", returnType, name, parameters, locals, buildCode(instructions));
+ }
+
public MethodSignature addInstanceMethod(
String returnType, String name, int locals, String... instructions) {
return addInstanceMethod(returnType, name, ImmutableList.of(), locals, buildCode(instructions));
diff --git a/src/test/java/com/android/tools/r8/utils/DexInspector.java b/src/test/java/com/android/tools/r8/utils/DexInspector.java
index fd32ec7..0b8de12 100644
--- a/src/test/java/com/android/tools/r8/utils/DexInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/DexInspector.java
@@ -715,6 +715,8 @@
public abstract class MemberSubject extends Subject {
+ public abstract boolean isPublic();
+
public abstract boolean isStatic();
public abstract boolean isFinal();
@@ -773,6 +775,11 @@
}
@Override
+ public boolean isPublic() {
+ return false;
+ }
+
+ @Override
public boolean isStatic() {
return false;
}
@@ -849,6 +856,11 @@
}
@Override
+ public boolean isPublic() {
+ return dexMethod.accessFlags.isPublic();
+ }
+
+ @Override
public boolean isStatic() {
return dexMethod.accessFlags.isStatic();
}
@@ -966,6 +978,11 @@
public class AbsentFieldSubject extends FieldSubject {
@Override
+ public boolean isPublic() {
+ return false;
+ }
+
+ @Override
public boolean isStatic() {
return false;
}
@@ -1032,6 +1049,11 @@
}
@Override
+ public boolean isPublic() {
+ return dexField.accessFlags.isPublic();
+ }
+
+ @Override
public boolean isStatic() {
return dexField.accessFlags.isStatic();
}
diff --git a/src/test/java/com/android/tools/r8/utils/DexInspectorMatchers.java b/src/test/java/com/android/tools/r8/utils/DexInspectorMatchers.java
index 8e21bea..1986bbc 100644
--- a/src/test/java/com/android/tools/r8/utils/DexInspectorMatchers.java
+++ b/src/test/java/com/android/tools/r8/utils/DexInspectorMatchers.java
@@ -138,4 +138,24 @@
}
};
}
+
+ public static Matcher<MethodSubject> isPublic() {
+ return new TypeSafeMatcher<MethodSubject>() {
+ @Override
+ public boolean matchesSafely(final MethodSubject method) {
+ return method.isPresent() && method.isPublic();
+ }
+
+ @Override
+ public void describeTo(final Description description) {
+ description.appendText("method public");
+ }
+
+ @Override
+ public void describeMismatchSafely(final MethodSubject method, Description description) {
+ description
+ .appendText("method ").appendValue(method.getOriginalName()).appendText(" was not");
+ }
+ };
+ }
}
