Version to 1.2.27
Handle non-standard invoke-super in class merging
CL: https://r8-review.googlesource.com/c/r8/+/22461
Avoid vertical class merging across package boundaries
CL: https://r8-review.googlesource.com/c/r8/+/22441
Add a test for -if rules combined with inlining
CL: https://r8-review.googlesource.com/c/r8/+/22401
Add a test for -if rules combined with vertical class merging
CL: https://r8-review.googlesource.com/c/r8/+/22400
Add test for invoke-special instructions
CL: https://r8-review.googlesource.com/c/r8/+/22440
Restrict vertical class merging
CL: https://r8-review.googlesource.com/c/r8/+/22383
Update holder of fields during class merging
CL: https://r8-review.googlesource.com/c/r8/+/22382
Rewrite super calls in vertical class merging
CL: https://r8-review.googlesource.com/c/r8/+/22342
Add tests for Proguard's behavior regarding -allowaccessmodification.
CL: https://r8-review.googlesource.com/c/r8/+/22200
Test class merging in presence of invoke-super
CL: https://r8-review.googlesource.com/c/r8/+/22341
Change-Id: Icede0aaa16d35553bb776bb97f6e43e84d36efc1
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 98a6d86..02c5f8e 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.2.26";
+ public static final String LABEL = "1.2.27";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/DefaultUseRegistry.java b/src/main/java/com/android/tools/r8/graph/DefaultUseRegistry.java
new file mode 100644
index 0000000..7f31098
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/DefaultUseRegistry.java
@@ -0,0 +1,63 @@
+// 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.graph;
+
+public class DefaultUseRegistry extends UseRegistry {
+
+ @Override
+ public boolean registerInvokeVirtual(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeDirect(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeStatic(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeInterface(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeSuper(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInstanceFieldWrite(DexField field) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInstanceFieldRead(DexField field) {
+ return true;
+ }
+
+ @Override
+ public boolean registerNewInstance(DexType type) {
+ return true;
+ }
+
+ @Override
+ public boolean registerStaticFieldRead(DexField field) {
+ return true;
+ }
+
+ @Override
+ public boolean registerStaticFieldWrite(DexField field) {
+ return true;
+ }
+
+ @Override
+ public boolean registerTypeReference(DexType type) {
+ return true;
+ }
+}
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 23139c5..2c188da 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.ThrowingConsumer;
import com.google.common.base.MoreObjects;
+import com.google.common.collect.Iterators;
import java.util.Arrays;
import java.util.List;
import java.util.function.Consumer;
@@ -96,6 +97,16 @@
}
}
+ public Iterable<DexEncodedField> fields() {
+ return () ->
+ Iterators.concat(Iterators.forArray(instanceFields), Iterators.forArray(staticFields));
+ }
+
+ public Iterable<DexEncodedMethod> methods() {
+ return () ->
+ Iterators.concat(Iterators.forArray(directMethods), Iterators.forArray(virtualMethods));
+ }
+
@Override
void collectMixedSectionItems(MixedSectionCollection mixedItems) {
throw new Unreachable();
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index a52637f..50fc1c9 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -140,7 +140,7 @@
this.memberValuePropagation =
options.enableValuePropagation ?
new MemberValuePropagation(appInfo.withLiveness()) : null;
- this.lensCodeRewriter = new LensCodeRewriter(graphLense, appInfo.withSubtyping());
+ this.lensCodeRewriter = new LensCodeRewriter(graphLense, appInfo.withSubtyping(), options);
if (appInfo.hasLiveness()) {
// When disabling the pruner here, also disable the ProtoLiteExtension in R8.java.
this.protoLiteRewriter =
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 1926b14..0e4be8b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -34,6 +34,7 @@
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.StaticPut;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.utils.InternalOptions;
import java.util.List;
import java.util.ListIterator;
import java.util.stream.Collectors;
@@ -42,10 +43,13 @@
private final GraphLense graphLense;
private final AppInfoWithSubtyping appInfo;
+ private final InternalOptions options;
- public LensCodeRewriter(GraphLense graphLense, AppInfoWithSubtyping appInfo) {
+ public LensCodeRewriter(
+ GraphLense graphLense, AppInfoWithSubtyping appInfo, InternalOptions options) {
this.graphLense = graphLense;
this.appInfo = appInfo;
+ this.options = options;
}
private Value makeOutValue(Instruction insn, IRCode code) {
@@ -97,7 +101,7 @@
continue;
}
DexMethod actualTarget = graphLense.lookupMethod(invokedMethod, method, invoke.getType());
- Invoke.Type invokeType = getInvokeType(invoke, actualTarget, invokedMethod);
+ Invoke.Type invokeType = getInvokeType(invoke, actualTarget, invokedMethod, method);
if (actualTarget != invokedMethod || invoke.getType() != invokeType) {
Invoke newInvoke = Invoke.create(invokeType, actualTarget, null,
invoke.outValue(), invoke.inValues());
@@ -242,7 +246,8 @@
private Type getInvokeType(
InvokeMethod invoke,
DexMethod actualTarget,
- DexMethod originalTarget) {
+ DexMethod originalTarget,
+ DexEncodedMethod invocationContext) {
// We might move methods from interfaces to classes and vice versa. So we have to support
// fixing the invoke kind, yet only if it was correct to start with.
if (invoke.isInvokeVirtual() || invoke.isInvokeInterface()) {
@@ -250,23 +255,42 @@
DexClass newTargetClass = appInfo.definitionFor(actualTarget.holder);
if (newTargetClass == null) {
return invoke.getType();
- } else {
- DexClass originalTargetClass = appInfo.definitionFor(originalTarget.holder);
- if (originalTargetClass != null
- && (originalTargetClass.isInterface() ^ (invoke.getType() == Type.INTERFACE))) {
- // The invoke was wrong to start with, so we keep it wrong. This is to ensure we get
- // the IncompatibleClassChangeError the original invoke would have triggered.
- return newTargetClass.accessFlags.isInterface()
- ? Type.VIRTUAL
- : Type.INTERFACE;
- } else {
- return newTargetClass.accessFlags.isInterface()
- ? Type.INTERFACE
- : Type.VIRTUAL;
+ }
+ DexClass originalTargetClass = appInfo.definitionFor(originalTarget.holder);
+ if (originalTargetClass != null
+ && (originalTargetClass.isInterface() ^ (invoke.getType() == Type.INTERFACE))) {
+ // The invoke was wrong to start with, so we keep it wrong. This is to ensure we get
+ // the IncompatibleClassChangeError the original invoke would have triggered.
+ return newTargetClass.accessFlags.isInterface() ? Type.VIRTUAL : Type.INTERFACE;
+ }
+ return newTargetClass.accessFlags.isInterface() ? Type.INTERFACE : Type.VIRTUAL;
+ }
+ if (options.enableClassMerging && invoke.isInvokeSuper()) {
+ if (actualTarget.getHolder() == invocationContext.method.getHolder()) {
+ DexClass targetClass = appInfo.definitionFor(actualTarget.holder);
+ if (targetClass == null) {
+ return invoke.getType();
+ }
+
+ // If the super class A of the enclosing class B (i.e., invocationContext.method.holder)
+ // has been merged into B during vertical class merging, and this invoke-super instruction
+ // was resolving to a method in A, then the target method has been changed to a direct
+ // method and moved into B, so that we need to use an invoke-direct instruction instead of
+ // invoke-super.
+ //
+ // At this point, we have an invoke-super instruction where the static target is the
+ // enclosing class. However, such an instruction could occur even if a subclass has never
+ // been merged into the enclosing class. Therefore, to determine if vertical class merging
+ // has been applied, we look if there is a direct method with the right signature, and only
+ // return Type.DIRECT in that case.
+ DexEncodedMethod method = targetClass.lookupDirectMethod(actualTarget);
+ if (method != null) {
+ // The target method has been moved from the super class into the sub class during class
+ // merging such that we now need to use an invoke-direct instruction.
+ return Type.DIRECT;
}
}
- } else {
- return invoke.getType();
}
+ return invoke.getType();
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 25cd969..029b169 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -271,7 +271,7 @@
Origin origin = appInfo.originFor(target.method.holder);
IRCode code = target.buildInliningIR(appInfo, options, generator, callerPosition, origin);
if (!target.isProcessed()) {
- new LensCodeRewriter(graphLense, appInfo).rewrite(code, target);
+ new LensCodeRewriter(graphLense, appInfo, options).rewrite(code, target);
}
return code;
}
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java b/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
index 0ba49d3..6ea7d60 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
@@ -45,6 +45,7 @@
assert checkSignatures();
switch (invokeType) {
+ case DIRECT:
case STATIC:
case SUPER:
case INTERFACE:
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 d196dd2..5da6f06 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.shaking;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.graph.DefaultUseRegistry;
+import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
@@ -18,7 +20,13 @@
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.GraphLense.Builder;
import com.android.tools.r8.graph.KeyedDexItem;
+import com.android.tools.r8.graph.MethodAccessFlags;
+import com.android.tools.r8.graph.ParameterAnnotationsList;
import com.android.tools.r8.graph.PresortedComparable;
+import com.android.tools.r8.graph.UseRegistry;
+import com.android.tools.r8.ir.code.Invoke.Type;
+import com.android.tools.r8.ir.synthetic.ForwardMethodSourceCode;
+import com.android.tools.r8.ir.synthetic.SynthesizedCode;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.optimize.InvokeSingleTargetExtractor;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
@@ -33,6 +41,7 @@
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
import java.util.ArrayDeque;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Deque;
@@ -41,6 +50,7 @@
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
@@ -59,7 +69,7 @@
private final DexApplication application;
private final AppInfoWithLiveness appInfo;
private final GraphLense graphLense;
- private final Map<DexType, DexType> mergedClasses = new IdentityHashMap<>();
+ private final Map<DexType, DexType> mergedClasses = new HashMap<>();
private final Timing timing;
private Collection<DexMethod> invokes;
@@ -74,13 +84,111 @@
this.timing = timing;
}
- private boolean isMergeCandidate(DexProgramClass clazz) {
- // We can merge program classes if they are not instantiated, have a single subtype
- // and we do not have to keep them.
- return !clazz.isLibraryClass()
- && !appInfo.instantiatedTypes.contains(clazz.type)
- && !appInfo.isPinned(clazz.type)
- && clazz.type.getSingleSubtype() != null;
+ // Returns a set of types that must not be merged into other types.
+ private Set<DexType> getPinnedTypes(Iterable<DexProgramClass> classes) {
+ Set<DexType> pinnedTypes = new HashSet<>();
+ for (DexProgramClass clazz : classes) {
+ for (DexEncodedMethod method : clazz.methods()) {
+ // TODO(christofferqa): Remove the invariant that the graph lense should not modify any
+ // methods from the sets alwaysInline and noSideEffects (see use of assertNotModifiedBy-
+ // Lense).
+ if (appInfo.alwaysInline.contains(method) || appInfo.noSideEffects.containsKey(method)) {
+ DexClass other = appInfo.definitionFor(method.method.proto.returnType);
+ if (other != null && other.isProgramClass()) {
+ // If we were to merge [other] into its sub class, then we would implicitly change the
+ // signature of this method, and therefore break the invariant.
+ pinnedTypes.add(other.type);
+ }
+ for (DexType parameterType : method.method.proto.parameters.values) {
+ other = appInfo.definitionFor(parameterType);
+ if (other != null && other.isProgramClass()) {
+ // If we were to merge [other] into its sub class, then we would implicitly change the
+ // signature of this method, and therefore break the invariant.
+ pinnedTypes.add(other.type);
+ }
+ }
+ }
+
+ // Avoid merging two types if this could remove a NoSuchMethodError, as illustrated by the
+ // following example. (Alternatively, it would be possible to merge A and B and rewrite the
+ // "invoke-super A.m" instruction into "invoke-super Object.m" to preserve the error. This
+ // situation should generally not occur in practice, though.)
+ //
+ // class A {}
+ // class B extends A {
+ // public void m() {}
+ // }
+ // class C extends A {
+ // public void m() {
+ // invoke-super "A.m" <- should yield NoSuchMethodError, cannot merge A and B
+ // }
+ // }
+ if (!method.isStaticMethod()) {
+ method.registerCodeReferences(
+ new DefaultUseRegistry() {
+ @Override
+ public boolean registerInvokeSuper(DexMethod target) {
+ DexClass targetClass = appInfo.definitionFor(target.getHolder());
+ if (targetClass != null
+ && targetClass.isProgramClass()
+ && targetClass.lookupVirtualMethod(target) == null) {
+ pinnedTypes.add(target.getHolder());
+ }
+ return true;
+ }
+ });
+ }
+ }
+ }
+ return pinnedTypes;
+ }
+
+ private boolean isMergeCandidate(DexProgramClass clazz, Set<DexType> pinnedTypes) {
+ if (appInfo.instantiatedTypes.contains(clazz.type)
+ || appInfo.isPinned(clazz.type)
+ || pinnedTypes.contains(clazz.type)) {
+ return false;
+ }
+ DexType singleSubtype = clazz.type.getSingleSubtype();
+ if (singleSubtype == null) {
+ // TODO(christofferqa): Even if [clazz] has multiple subtypes, we could still merge it into
+ // its subclass if [clazz] is not live. This should only be done, though, if it does not
+ // lead to members being duplicated.
+ return false;
+ }
+ DexClass targetClass = appInfo.definitionFor(singleSubtype);
+ if (mergeMayLeadToIllegalAccesses(clazz, targetClass)) {
+ return false;
+ }
+ for (DexEncodedField field : clazz.fields()) {
+ if (appInfo.isPinned(field.field)) {
+ return false;
+ }
+ }
+ for (DexEncodedMethod method : clazz.methods()) {
+ if (appInfo.isPinned(method.method)) {
+ return false;
+ }
+ if (method.isInstanceInitializer() && disallowInlining(method)) {
+ // Cannot guarantee that markForceInline() will work.
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private boolean mergeMayLeadToIllegalAccesses(DexClass clazz, DexClass singleSubclass) {
+ if (clazz.type.isSamePackage(singleSubclass.type)) {
+ return false;
+ }
+ // TODO(christofferqa): To merge [clazz] into a class from another package we need to ensure:
+ // (A) All accesses to [clazz] and its members from inside the current package of [clazz] will
+ // continue to work. This is guaranteed if [clazz] is public and all members of [clazz] are
+ // either private or public.
+ // (B) All accesses from [clazz] to classes or members from the current package of [clazz] will
+ // continue to work. This is guaranteed if the methods of [clazz] do not access any private
+ // or protected classes or members from the current package of [clazz].
+ return true;
}
private void addProgramMethods(Set<Wrapper<DexMethod>> set, DexMethod method,
@@ -170,6 +278,9 @@
int numberOfMerges = 0;
+ // Types that are pinned (in addition to those where appInfo.isPinned returns true).
+ Set<DexType> pinnedTypes = getPinnedTypes(classes);
+
// The resulting graph lense that should be used after class merging.
VerticalClassMergerGraphLense.Builder renamedMembersLense =
new VerticalClassMergerGraphLense.Builder();
@@ -188,7 +299,7 @@
}
DexProgramClass clazz = worklist.removeFirst();
- if (!seenBefore.add(clazz) || !isMergeCandidate(clazz)) {
+ if (!seenBefore.add(clazz) || !isMergeCandidate(clazz, pinnedTypes)) {
continue;
}
@@ -227,6 +338,9 @@
if (merged) {
// Commit the changes to the graph lense.
renamedMembersLense.merge(merger.getRenamings());
+ // Do not allow merging the resulting class into its subclass.
+ // TODO(christofferqa): Get rid of this limitation.
+ pinnedTypes.add(targetClass.type);
}
if (Log.ENABLED) {
if (merged) {
@@ -276,34 +390,87 @@
// targetClass that are not currently contained.
// Step 1: Merge methods
Set<Wrapper<DexMethod>> existingMethods = new HashSet<>();
- addAll(existingMethods, target.directMethods(), MethodSignatureEquivalence.get());
- addAll(existingMethods, target.virtualMethods(), MethodSignatureEquivalence.get());
- Collection<DexEncodedMethod> mergedDirectMethods = mergeItems(
- Iterators.transform(Iterators.forArray(source.directMethods()), this::renameConstructors),
- target.directMethods(),
- MethodSignatureEquivalence.get(),
- existingMethods,
- this::renameMethod
- );
- Iterator<DexEncodedMethod> methods = Iterators.forArray(source.virtualMethods());
- if (source.accessFlags.isInterface()) {
- // If merging an interface, only merge methods that are not otherwise defined in the
- // target class.
- methods = Iterators.transform(methods, this::filterShadowedInterfaceMethods);
+ addAll(existingMethods, target.methods(), MethodSignatureEquivalence.get());
+
+ List<DexEncodedMethod> directMethods = new ArrayList<>();
+ for (DexEncodedMethod directMethod : source.directMethods()) {
+ if (directMethod.isInstanceInitializer()) {
+ directMethods.add(renameConstructor(directMethod));
+ } else {
+ directMethods.add(directMethod);
+ }
}
- Collection<DexEncodedMethod> mergedVirtualMethods = mergeItems(
- methods,
- target.virtualMethods(),
- MethodSignatureEquivalence.get(),
- existingMethods,
- this::abortOnNonAbstract);
+
+ List<DexEncodedMethod> virtualMethods = new ArrayList<>();
+ for (DexEncodedMethod virtualMethod : source.virtualMethods()) {
+ DexEncodedMethod shadowedBy = findMethodInTarget(virtualMethod);
+ if (shadowedBy != null) {
+ if (virtualMethod.accessFlags.isAbstract()) {
+ // Remove abstract/interface methods that are shadowed.
+ deferredRenamings.map(virtualMethod.method, shadowedBy.method);
+ continue;
+ }
+ } else {
+ // The method is shadowed. If it is abstract, we can simply move it to the subclass.
+ // Non-abstract methods are handled below (they cannot simply be moved to the subclass as
+ // a virtual method, because they might be the target of an invoke-super instruction).
+ if (virtualMethod.accessFlags.isAbstract()) {
+ DexEncodedMethod resultingVirtualMethod =
+ renameMethod(virtualMethod, target.type, false);
+ deferredRenamings.map(virtualMethod.method, resultingVirtualMethod.method);
+ virtualMethods.add(resultingVirtualMethod);
+ continue;
+ }
+ }
+
+ // This virtual method could be called directly from a sub class via an invoke-super
+ // instruction. Therefore, we translate this virtual method into a direct method, such that
+ // relevant invoke-super instructions can be rewritten into invoke-direct instructions.
+ DexEncodedMethod resultingDirectMethod = renameMethod(virtualMethod, target.type, true);
+ makePrivate(resultingDirectMethod);
+ directMethods.add(resultingDirectMethod);
+
+ // Record that invoke-super instructions in the target class should be redirected to the
+ // newly created direct method.
+ redirectSuperCallsInTarget(virtualMethod.method, resultingDirectMethod.method);
+
+ if (shadowedBy == null) {
+ // In addition to the newly added direct method, create a virtual method such that we do
+ // not accidentally remove the method from the interface of this class.
+ // Note that this method is added independently of whether it will actually be used. If
+ // it turns out that the method is never used, it will be removed by the final round
+ // of tree shaking.
+ shadowedBy = buildBridgeMethod(virtualMethod, resultingDirectMethod.method);
+ virtualMethods.add(shadowedBy);
+ }
+
+ deferredRenamings.map(virtualMethod.method, shadowedBy.method);
+ }
+
+ Collection<DexEncodedMethod> mergedDirectMethods =
+ mergeItems(
+ directMethods.iterator(),
+ target.directMethods(),
+ MethodSignatureEquivalence.get(),
+ existingMethods,
+ (existing, method) -> {
+ DexEncodedMethod renamedMethod = renameMethod(method, target.type, true);
+ deferredRenamings.map(method.method, renamedMethod.method);
+ return renamedMethod;
+ });
+ Collection<DexEncodedMethod> mergedVirtualMethods =
+ mergeItems(
+ virtualMethods.iterator(),
+ target.virtualMethods(),
+ MethodSignatureEquivalence.get(),
+ existingMethods,
+ this::abortOnNonAbstract);
if (abortMerge) {
return false;
}
// Step 2: Merge fields
Set<Wrapper<DexField>> existingFields = new HashSet<>();
- addAll(existingFields, target.instanceFields(), FieldSignatureEquivalence.get());
- addAll(existingFields, target.staticFields(), FieldSignatureEquivalence.get());
+ addAll(existingFields, target.fields(), FieldSignatureEquivalence.get());
Collection<DexEncodedField> mergedStaticFields = mergeItems(
Iterators.forArray(source.staticFields()),
target.staticFields(),
@@ -354,21 +521,73 @@
return deferredRenamings;
}
- private DexEncodedMethod filterShadowedInterfaceMethods(DexEncodedMethod m) {
- DexEncodedMethod actual = appInfo.resolveMethod(target.type, m.method).asSingleTarget();
- assert actual != null;
- if (actual != m) {
- // We will drop a method here, so record it as a potential renaming.
- deferredRenamings.map(m.method, actual.method);
+ private void redirectSuperCallsInTarget(DexMethod oldTarget, DexMethod newTarget) {
+ // If we merge class B into class C, and class C contains an invocation super.m(), then it
+ // is insufficient to rewrite "invoke-super B.m()" to "invoke-direct C.m$B()" (the method
+ // C.m$B denotes the direct method that has been created in C for B.m). In particular, there
+ // might be an instruction "invoke-super A.m()" in C that resolves to B.m at runtime (A is
+ // a superclass of B), which also needs to be rewritten to "invoke-direct C.m$B()".
+ //
+ // We handle this by adding a mapping for [target] and all of its supertypes.
+ DexClass holder = target;
+ while (holder != null && holder.isProgramClass()) {
+ DexMethod signatureInHolder =
+ application.dexItemFactory.createMethod(holder.type, oldTarget.proto, oldTarget.name);
+ // Only rewrite the invoke-super call if it does not lead to a NoSuchMethodError.
+ if (resolutionSucceeds(signatureInHolder)) {
+ deferredRenamings.mapVirtualMethodToDirectInType(
+ signatureInHolder, newTarget, target.type);
+ holder = holder.superType != null ? appInfo.definitionFor(holder.superType) : null;
+ } else {
+ break;
+ }
+ }
+ }
+
+ private boolean resolutionSucceeds(DexMethod targetMethod) {
+ DexClass enclosingClass = appInfo.definitionFor(targetMethod.holder);
+ return enclosingClass != null
+ && (enclosingClass.lookupVirtualMethod(targetMethod) != null
+ || appInfo.lookupSuperTarget(targetMethod, enclosingClass.type) != null);
+ }
+
+ private DexEncodedMethod buildBridgeMethod(
+ DexEncodedMethod signature, DexMethod invocationTarget) {
+ DexType holder = target.type;
+ DexProto proto = invocationTarget.proto;
+ DexString name = signature.method.name;
+ MethodAccessFlags accessFlags = signature.accessFlags.copy();
+ accessFlags.setBridge();
+ accessFlags.setSynthetic();
+ accessFlags.unsetAbstract();
+ return new DexEncodedMethod(
+ application.dexItemFactory.createMethod(holder, proto, name),
+ accessFlags,
+ DexAnnotationSet.empty(),
+ ParameterAnnotationsList.empty(),
+ new SynthesizedCode(
+ new ForwardMethodSourceCode(holder, proto, holder, invocationTarget, Type.DIRECT),
+ registry -> registry.registerInvokeDirect(invocationTarget)));
+ }
+
+ // Returns the method that shadows the given method, or null if method is not shadowed.
+ private DexEncodedMethod findMethodInTarget(DexEncodedMethod method) {
+ DexEncodedMethod actual = appInfo.resolveMethod(target.type, method.method).asSingleTarget();
+ if (actual == null) {
+ // May happen in case of missing classes.
+ abortMerge = true;
return null;
}
- // We will keep the method, so the class better be abstract.
- assert target.accessFlags.isAbstract();
- return m;
+ if (actual != method) {
+ return actual;
+ }
+ // We will keep the method, so the class better be abstract if there is no implementation.
+ assert !method.accessFlags.isAbstract() || target.accessFlags.isAbstract();
+ return null;
}
private <T extends KeyedDexItem<S>, S extends PresortedComparable<S>> void addAll(
- Collection<Wrapper<S>> collection, T[] items, Equivalence<S> equivalence) {
+ Collection<Wrapper<S>> collection, Iterable<T> items, Equivalence<S> equivalence) {
for (T item : items) {
collection.add(equivalence.wrap(item.getKey()));
}
@@ -420,80 +639,85 @@
}
}
- private DexString makeMergedName(String nameString, DexType holder) {
- return application.dexItemFactory
- .createString(nameString + "$" + holder.toSourceString().replace('.', '$'));
+ // Note that names returned by this function are not necessarily unique. However, class merging
+ // is aborted if it turns out that the returned name is not unique.
+ // TODO(christofferqa): Write a test that checks that class merging is aborted if this function
+ // generates a name that is not unique.
+ private DexString getFreshName(String nameString, DexType holder) {
+ String freshName = nameString + "$" + holder.toSourceString().replace('.', '$');
+ return application.dexItemFactory.createString(freshName);
}
private DexEncodedMethod abortOnNonAbstract(DexEncodedMethod existing,
DexEncodedMethod method) {
- if (existing == null) {
- // This is a conflict between a static and virtual method. Abort.
- abortMerge = true;
- return method;
- }
- // Ignore if we merge in an abstract method or if we override a bridge method that would
- // bridge to the superclasses method.
- if (method.accessFlags.isAbstract()) {
- // We make a method disappear here, so record the renaming so that calls to the previous
- // target get forwarded properly.
- deferredRenamings.map(method.method, existing.method);
- return existing;
- } else if (existing.accessFlags.isBridge()) {
+ // Ignore if we override a bridge method that would bridge to the superclasses method.
+ if (existing != null && existing.accessFlags.isBridge()) {
InvokeSingleTargetExtractor extractor = new InvokeSingleTargetExtractor();
existing.getCode().registerCodeReferences(extractor);
- if (extractor.getTarget() != method.method) {
- abortMerge = true;
+ if (extractor.getTarget() == method.method) {
+ return method;
}
- return method;
- } else {
- abortMerge = true;
- return existing;
}
+
+ // Abstract shadowed methods are removed prior to calling mergeItems.
+ assert !method.accessFlags.isAbstract();
+
+ // If [existing] is null, there is a conflict between a static and virtual method. Otherwise,
+ // there is a non-abstract method that is shadowed. We translate virtual shadowed methods into
+ // direct methods with a fresh name, so this situation should never happen. We currently get
+ // in this situation if getFreshName returns a name that is not unique, though.
+ abortMerge = true;
+ return method;
}
- private DexEncodedMethod renameConstructors(DexEncodedMethod method) {
- // Only rename instance initializers.
- if (!method.isInstanceInitializer()) {
- return method;
- }
+ private DexEncodedMethod renameConstructor(DexEncodedMethod method) {
+ assert method.isInstanceInitializer();
DexType holder = method.method.holder;
- DexEncodedMethod result = method
- .toRenamedMethod(makeMergedName(CONSTRUCTOR_NAME, holder), application.dexItemFactory);
+ DexEncodedMethod result =
+ method.toRenamedMethod(
+ getFreshName(CONSTRUCTOR_NAME, holder), application.dexItemFactory);
result.markForceInline();
deferredRenamings.map(method.method, result.method);
// Renamed constructors turn into ordinary private functions. They can be private, as
// they are only references from their direct subclass, which they were merged into.
result.accessFlags.unsetConstructor();
- result.accessFlags.unsetPublic();
- result.accessFlags.unsetProtected();
- result.accessFlags.setPrivate();
+ makePrivate(result);
return result;
}
- private DexEncodedMethod renameMethod(DexEncodedMethod existing, DexEncodedMethod method) {
+ private DexEncodedMethod renameMethod(
+ DexEncodedMethod method, DexType newHolder, boolean useFreshName) {
// We cannot handle renaming static initializers yet and constructors should have been
// renamed already.
assert !method.accessFlags.isConstructor();
- DexType holder = method.method.holder;
- String name = method.method.name.toSourceString();
- DexEncodedMethod result = method
- .toRenamedMethod(makeMergedName(name, holder), application.dexItemFactory);
- deferredRenamings.map(method.method, result.method);
- return result;
+ DexType oldHolder = method.method.holder;
+ DexString oldName = method.method.name;
+ DexString newName =
+ useFreshName ? getFreshName(oldName.toSourceString(), oldHolder) : oldName;
+ DexMethod newSignature =
+ application.dexItemFactory.createMethod(newHolder, method.method.proto, newName);
+ return method.toTypeSubstitutedMethod(newSignature);
}
private DexEncodedField renameField(DexEncodedField existing, DexEncodedField field) {
DexString oldName = field.field.name;
- DexType holder = field.field.clazz;
- DexEncodedField result = field
- .toRenamedField(makeMergedName(oldName.toSourceString(), holder),
- application.dexItemFactory);
+ DexType oldHolder = field.field.clazz;
+ DexString newName = getFreshName(oldName.toSourceString(), oldHolder);
+ DexField newSignature =
+ application.dexItemFactory.createField(target.type, field.field.type, newName);
+ DexEncodedField result = field.toTypeSubstitutedField(newSignature);
deferredRenamings.map(field.field, result.field);
return result;
}
}
+ private static void makePrivate(DexEncodedMethod method) {
+ assert !method.accessFlags.isAbstract();
+ method.accessFlags.unsetPublic();
+ method.accessFlags.unsetProtected();
+ method.accessFlags.setPrivate();
+ }
+
private class TreeFixer {
private final Builder lense = GraphLense.builder();
@@ -731,6 +955,86 @@
}
}
+ private static boolean disallowInlining(DexEncodedMethod method) {
+ // TODO(christofferqa): Determine the situations where markForceInline() may fail, and ensure
+ // that we always return true here in these cases.
+ MethodInlineDecision registry = new MethodInlineDecision();
+ method.getCode().registerCodeReferences(registry);
+ return registry.isInliningDisallowed();
+ }
+
+ private static class MethodInlineDecision extends UseRegistry {
+ private boolean disallowInlining = false;
+
+ public boolean isInliningDisallowed() {
+ return disallowInlining;
+ }
+
+ private boolean allowInlining() {
+ return true;
+ }
+
+ private boolean disallowInlining() {
+ disallowInlining = true;
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeInterface(DexMethod method) {
+ return disallowInlining();
+ }
+
+ @Override
+ public boolean registerInvokeVirtual(DexMethod method) {
+ return disallowInlining();
+ }
+
+ @Override
+ public boolean registerInvokeDirect(DexMethod method) {
+ return allowInlining();
+ }
+
+ @Override
+ public boolean registerInvokeStatic(DexMethod method) {
+ return allowInlining();
+ }
+
+ @Override
+ public boolean registerInvokeSuper(DexMethod method) {
+ return allowInlining();
+ }
+
+ @Override
+ public boolean registerInstanceFieldWrite(DexField field) {
+ return allowInlining();
+ }
+
+ @Override
+ public boolean registerInstanceFieldRead(DexField field) {
+ return allowInlining();
+ }
+
+ @Override
+ public boolean registerNewInstance(DexType type) {
+ return allowInlining();
+ }
+
+ @Override
+ public boolean registerStaticFieldRead(DexField field) {
+ return allowInlining();
+ }
+
+ @Override
+ public boolean registerStaticFieldWrite(DexField field) {
+ return allowInlining();
+ }
+
+ @Override
+ public boolean registerTypeReference(DexType type) {
+ return allowInlining();
+ }
+ }
+
public Collection<DexType> getRemovedClasses() {
return Collections.unmodifiableCollection(mergedClasses.keySet());
}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
index 54393b5..ea57d1d 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.ir.code.Invoke.Type;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import java.util.HashMap;
import java.util.Map;
import java.util.Set;
@@ -44,14 +45,17 @@
private final Map<DexField, DexField> fieldMap;
private final Map<DexMethod, DexMethod> methodMap;
+ private final Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps;
public VerticalClassMergerGraphLense(
Map<DexField, DexField> fieldMap,
Map<DexMethod, DexMethod> methodMap,
+ Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps,
GraphLense previousLense) {
this.previousLense = previousLense;
this.fieldMap = fieldMap;
this.methodMap = methodMap;
+ this.contextualVirtualToDirectMethodMaps = contextualVirtualToDirectMethodMaps;
}
@Override
@@ -61,11 +65,18 @@
@Override
public DexMethod lookupMethod(DexMethod method, DexEncodedMethod context, Type type) {
- // TODO(christofferqa): If [type] is Type.SUPER and [method] has been merged into the class of
- // [context], then return the DIRECT method that has been created for [method] by SimpleClass-
- // Merger. Otherwise, return the VIRTUAL method corresponding to [method].
- assert isContextFreeForMethod(method) || context != null;
+ assert isContextFreeForMethod(method) || (context != null && type != null);
DexMethod previous = previousLense.lookupMethod(method, context, type);
+ if (type == Type.SUPER) {
+ Map<DexMethod, DexMethod> virtualToDirectMethodMap =
+ contextualVirtualToDirectMethodMaps.get(context.method.holder);
+ if (virtualToDirectMethodMap != null) {
+ DexMethod directMethod = virtualToDirectMethodMap.get(previous);
+ if (directMethod != null) {
+ return directMethod;
+ }
+ }
+ }
return methodMap.getOrDefault(previous, previous);
}
@@ -74,6 +85,13 @@
ImmutableSet.Builder<DexMethod> builder = ImmutableSet.builder();
for (DexMethod previous : previousLense.lookupMethodInAllContexts(method)) {
builder.add(methodMap.getOrDefault(previous, previous));
+ for (Map<DexMethod, DexMethod> virtualToDirectMethodMap :
+ contextualVirtualToDirectMethodMaps.values()) {
+ DexMethod directMethod = virtualToDirectMethodMap.get(previous);
+ if (directMethod != null) {
+ builder.add(directMethod);
+ }
+ }
}
return builder.build();
}
@@ -86,14 +104,22 @@
@Override
public boolean isContextFreeForMethods() {
- return previousLense.isContextFreeForMethods();
+ return contextualVirtualToDirectMethodMaps.isEmpty() && previousLense.isContextFreeForMethods();
}
@Override
public boolean isContextFreeForMethod(DexMethod method) {
- // TODO(christofferqa): Should return false for methods where this graph lense is context
- // sensitive.
- return previousLense.isContextFreeForMethod(method);
+ if (!previousLense.isContextFreeForMethod(method)) {
+ return false;
+ }
+ DexMethod previous = previousLense.lookupMethod(method);
+ for (Map<DexMethod, DexMethod> virtualToDirectMethodMap :
+ contextualVirtualToDirectMethodMaps.values()) {
+ if (virtualToDirectMethodMap.containsKey(previous)) {
+ return false;
+ }
+ }
+ return true;
}
public static class Builder {
@@ -101,16 +127,21 @@
private final ImmutableMap.Builder<DexField, DexField> fieldMapBuilder = ImmutableMap.builder();
private final ImmutableMap.Builder<DexMethod, DexMethod> methodMapBuilder =
ImmutableMap.builder();
+ private final Map<DexType, Map<DexMethod, DexMethod>> contextualVirtualToDirectMethodMaps =
+ new HashMap<>();
public Builder() {}
public GraphLense build(GraphLense previousLense) {
Map<DexField, DexField> fieldMap = fieldMapBuilder.build();
Map<DexMethod, DexMethod> methodMap = methodMapBuilder.build();
- if (fieldMap.isEmpty() && methodMap.isEmpty()) {
+ if (fieldMap.isEmpty()
+ && methodMap.isEmpty()
+ && contextualVirtualToDirectMethodMaps.isEmpty()) {
return previousLense;
}
- return new VerticalClassMergerGraphLense(fieldMap, methodMap, previousLense);
+ return new VerticalClassMergerGraphLense(
+ fieldMap, methodMap, contextualVirtualToDirectMethodMaps, previousLense);
}
public void map(DexField from, DexField to) {
@@ -121,9 +152,24 @@
methodMapBuilder.put(from, to);
}
+ public void mapVirtualMethodToDirectInType(DexMethod from, DexMethod to, DexType type) {
+ Map<DexMethod, DexMethod> virtualToDirectMethodMap =
+ contextualVirtualToDirectMethodMaps.computeIfAbsent(type, key -> new HashMap<>());
+ virtualToDirectMethodMap.put(from, to);
+ }
+
public void merge(VerticalClassMergerGraphLense.Builder builder) {
fieldMapBuilder.putAll(builder.fieldMapBuilder.build());
methodMapBuilder.putAll(builder.methodMapBuilder.build());
+ for (DexType context : builder.contextualVirtualToDirectMethodMaps.keySet()) {
+ Map<DexMethod, DexMethod> current = contextualVirtualToDirectMethodMaps.get(context);
+ Map<DexMethod, DexMethod> other = builder.contextualVirtualToDirectMethodMaps.get(context);
+ if (current != null) {
+ current.putAll(other);
+ } else {
+ contextualVirtualToDirectMethodMaps.put(context, other);
+ }
+ }
}
}
}
diff --git a/src/test/examples/classmerging/SubClassThatReferencesSuperMethod.java b/src/test/examples/classmerging/SubClassThatReferencesSuperMethod.java
index c12804c..c09b11a 100644
--- a/src/test/examples/classmerging/SubClassThatReferencesSuperMethod.java
+++ b/src/test/examples/classmerging/SubClassThatReferencesSuperMethod.java
@@ -7,6 +7,9 @@
@Override
public String referencedMethod() {
- return "From sub: " + super.referencedMethod();
+ System.out.println("In referencedMethod on SubClassThatReferencesSuperMethod");
+ System.out.println("Calling referencedMethod on SuperClassWithReferencedMethod with super");
+ System.out.println("Got: " + super.referencedMethod());
+ return "SubClassThatReferencesSuperMethod.referencedMethod()";
}
}
diff --git a/src/test/examples/classmerging/SuperCallRewritingTest.java b/src/test/examples/classmerging/SuperCallRewritingTest.java
new file mode 100644
index 0000000..7a3d45c
--- /dev/null
+++ b/src/test/examples/classmerging/SuperCallRewritingTest.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 classmerging;
+
+public class SuperCallRewritingTest {
+ public static void main(String[] args) {
+ System.out.println("Calling referencedMethod on SubClassThatReferencesSuperMethod");
+ System.out.println(new SubClassThatReferencesSuperMethod().referencedMethod());
+ }
+}
diff --git a/src/test/examples/classmerging/SuperClassWithReferencedMethod.java b/src/test/examples/classmerging/SuperClassWithReferencedMethod.java
index 8d4e7b5..3bd5886c84 100644
--- a/src/test/examples/classmerging/SuperClassWithReferencedMethod.java
+++ b/src/test/examples/classmerging/SuperClassWithReferencedMethod.java
@@ -6,6 +6,7 @@
public class SuperClassWithReferencedMethod {
public String referencedMethod() {
- return "From Super";
+ System.out.println("In referencedMethod on SuperClassWithReferencedMethod");
+ return "SuperClassWithReferencedMethod.referencedMethod()";
}
}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index da6ef55..01e35f3 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -16,6 +16,9 @@
-keep public class classmerging.SimpleInterfaceAccessTest {
public static void main(...);
}
+-keep public class classmerging.SuperCallRewritingTest {
+ public static void main(...);
+}
-keep public class classmerging.TemplateMethodTest {
public static void main(...);
}
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 000076e..d5d0558 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -807,7 +807,9 @@
public static R8Command.Builder prepareR8CommandBuilder(
AndroidApp app, ProgramConsumer programConsumer) {
- return R8Command.builder(app).setProgramConsumer(programConsumer);
+ return R8Command.builder(app)
+ .setProgramConsumer(programConsumer)
+ .setProguardMapConsumer(StringConsumer.emptyConsumer());
}
public static AndroidApp runR8(AndroidApp app) throws IOException {
@@ -1413,6 +1415,11 @@
return result.stdout;
}
+ public static ProcessResult runProguardRaw(
+ Path inJar, Path outJar, Path lib, Path config, Path map) throws IOException {
+ return runProguardRaw(getProguardScript(), inJar, outJar, lib, ImmutableList.of(config), map);
+ }
+
public static ProcessResult runProguardRaw(Path inJar, Path outJar, List<Path> config, Path map)
throws IOException {
return runProguardRaw(getProguardScript(), inJar, outJar, config, map);
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index 65181dc..7016163 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -17,6 +17,8 @@
import com.android.tools.r8.code.Instruction;
import com.android.tools.r8.code.MoveException;
import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.smali.SmaliBuilder;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
@@ -108,9 +110,77 @@
@Test
public void testSuperCallWasDetected() throws Exception {
- runR8(EXAMPLE_KEEP, this::configure);
- assertTrue(inspector.clazz("classmerging.SuperClassWithReferencedMethod").isPresent());
- assertTrue(inspector.clazz("classmerging.SubClassThatReferencesSuperMethod").isPresent());
+ String main = "classmerging.SuperCallRewritingTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("SubClassThatReferencesSuperMethod.class"),
+ CF_DIR.resolve("SuperClassWithReferencedMethod.class"),
+ CF_DIR.resolve("SuperCallRewritingTest.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.SubClassThatReferencesSuperMethod",
+ "classmerging.SuperCallRewritingTest");
+ runTest(main, programFiles, preservedClassNames);
+ }
+
+ // When a subclass A has been merged into its subclass B, we rewrite invoke-super calls that hit
+ // methods in A to invoke-direct calls. However, we should be careful not to transform invoke-
+ // super instructions into invoke-direct instructions simply because the static target is a method
+ // in the enclosing class.
+ //
+ // This test hand-crafts an invoke-super instruction in SubClassThatReferencesSuperMethod that
+ // targets SubClassThatReferencesSuperMethod.referencedMethod. When running without class
+ // merging, R8 should not rewrite the invoke-super instruction into invoke-direct.
+ @Test
+ public void testSuperCallNotRewrittenToDirect() throws Exception {
+ String main = "classmerging.SuperCallRewritingTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("SuperClassWithReferencedMethod.class"),
+ CF_DIR.resolve("SuperCallRewritingTest.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.SubClassThatReferencesSuperMethod",
+ "classmerging.SuperClassWithReferencedMethod",
+ "classmerging.SuperCallRewritingTest");
+
+ // Build SubClassThatReferencesMethod.
+ SmaliBuilder smaliBuilder =
+ new SmaliBuilder(
+ "classmerging.SubClassThatReferencesSuperMethod",
+ "classmerging.SuperClassWithReferencedMethod");
+ smaliBuilder.addInitializer(
+ ImmutableList.of(),
+ 0,
+ "invoke-direct {p0}, Lclassmerging/SuperClassWithReferencedMethod;-><init>()V",
+ "return-void");
+ smaliBuilder.addInstanceMethod(
+ "java.lang.String",
+ "referencedMethod",
+ ImmutableList.of(),
+ 2,
+ "sget-object v0, Ljava/lang/System;->out:Ljava/io/PrintStream;",
+ "const-string v1, \"In referencedMethod on SubClassThatReferencesSuperMethod\"",
+ "invoke-virtual {v0, v1}, Ljava/io/PrintStream;->println(Ljava/lang/String;)V",
+ "invoke-super {p0}, Lclassmerging/SubClassThatReferencesSuperMethod;->referencedMethod()Ljava/lang/String;",
+ "move-result-object v1",
+ "return-object v1");
+
+ // Build app.
+ AndroidApp.Builder builder = AndroidApp.builder();
+ builder.addProgramFiles(programFiles);
+ builder.addDexProgramData(smaliBuilder.compile(), Origin.unknown());
+
+ // Run test.
+ runTestOnInput(
+ main,
+ builder.build(),
+ preservedClassNames,
+ // Prevent class merging, such that the generated code would be invalid if we rewrite the
+ // invoke-super instruction into an invoke-direct instruction.
+ getProguardConfig(EXAMPLE_KEEP, "-keep class *"));
}
@Test
@@ -168,7 +238,6 @@
assertEquals(2, numberOfMoveExceptionInstructions);
}
- @Ignore("b/73958515")
@Test
public void testNoIllegalClassAccess() throws Exception {
String main = "classmerging.SimpleInterfaceAccessTest";
@@ -188,18 +257,36 @@
"classmerging.pkg.SimpleInterfaceImplRetriever",
"classmerging.pkg.SimpleInterfaceImplRetriever$SimpleInterfaceImpl");
runTest(main, programFiles, preservedClassNames);
+ }
+
+ @Ignore("b/73958515")
+ @Test
+ public void testNoIllegalClassAccessWithAccessModifications() throws Exception {
// If access modifications are allowed then SimpleInterface should be merged into
// SimpleInterfaceImpl.
- preservedClassNames =
+ String main = "classmerging.SimpleInterfaceAccessTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("SimpleInterface.class"),
+ CF_DIR.resolve("SimpleInterfaceAccessTest.class"),
+ CF_DIR.resolve("pkg/SimpleInterfaceImplRetriever.class"),
+ CF_DIR.resolve("pkg/SimpleInterfaceImplRetriever$SimpleInterfaceImpl.class")
+ };
+ ImmutableSet<String> preservedClassNames =
ImmutableSet.of(
"classmerging.SimpleInterfaceAccessTest",
"classmerging.pkg.SimpleInterfaceImplRetriever",
"classmerging.pkg.SimpleInterfaceImplRetriever$SimpleInterfaceImpl");
+ // Allow access modifications (and prevent SimpleInterfaceImplRetriever from being removed as
+ // a result of inlining).
runTest(
main,
programFiles,
preservedClassNames,
- getProguardConfig(EXAMPLE_KEEP, "-allowaccessmodification"));
+ getProguardConfig(
+ EXAMPLE_KEEP,
+ "-allowaccessmodification",
+ "-keep public class classmerging.pkg.SimpleInterfaceImplRetriever"));
}
@Test
@@ -219,13 +306,19 @@
private DexInspector runTest(String main, Path[] programFiles, Set<String> preservedClassNames)
throws Exception {
- return runTest(main, programFiles, preservedClassNames, getProguardConfig(EXAMPLE_KEEP, null));
+ return runTest(main, programFiles, preservedClassNames, getProguardConfig(EXAMPLE_KEEP));
}
private DexInspector runTest(
String main, Path[] programFiles, Set<String> preservedClassNames, String proguardConfig)
throws Exception {
- AndroidApp input = readProgramFiles(programFiles);
+ return runTestOnInput(
+ main, readProgramFiles(programFiles), preservedClassNames, proguardConfig);
+ }
+
+ private DexInspector runTestOnInput(
+ String main, AndroidApp input, Set<String> preservedClassNames, String proguardConfig)
+ throws Exception {
AndroidApp output = compileWithR8(input, proguardConfig, this::configure);
DexInspector inspector = new DexInspector(output);
// Check that all classes in [preservedClassNames] are in fact preserved.
@@ -244,14 +337,15 @@
return inspector;
}
- private String getProguardConfig(Path path, String additionalRules) throws IOException {
+ private String getProguardConfig(Path path, String... additionalRules) throws IOException {
StringBuilder builder = new StringBuilder();
for (String line : Files.readAllLines(path)) {
builder.append(line);
builder.append(System.lineSeparator());
}
- if (additionalRules != null) {
- builder.append(additionalRules);
+ for (String rule : additionalRules) {
+ builder.append(rule);
+ builder.append(System.lineSeparator());
}
return builder.toString();
}
diff --git a/src/test/java/com/android/tools/r8/graph/InvokeSpecialTest.java b/src/test/java/com/android/tools/r8/graph/InvokeSpecialTest.java
new file mode 100644
index 0000000..2c1479e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/graph/InvokeSpecialTest.java
@@ -0,0 +1,24 @@
+// 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.graph;
+
+import com.android.tools.r8.AsmTestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.graph.invokespecial.Main;
+import com.android.tools.r8.graph.invokespecial.TestClassDump;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class InvokeSpecialTest extends AsmTestBase {
+
+ @Ignore("b/110175213")
+ @Test
+ public void testInvokeSpecial() throws Exception {
+ ensureSameOutput(
+ Main.class.getCanonicalName(),
+ ToolHelper.getClassAsBytes(Main.class),
+ TestClassDump.dump());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/graph/invokespecial/Main.java b/src/test/java/com/android/tools/r8/graph/invokespecial/Main.java
new file mode 100644
index 0000000..deb316d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/graph/invokespecial/Main.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.graph.invokespecial;
+
+public class Main {
+
+ public static void main(String[] args) {
+ TestClass x = new TestClass();
+ x.m(true);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/graph/invokespecial/TestClass.java b/src/test/java/com/android/tools/r8/graph/invokespecial/TestClass.java
new file mode 100644
index 0000000..c08091e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/graph/invokespecial/TestClass.java
@@ -0,0 +1,15 @@
+// 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.graph.invokespecial;
+
+public class TestClass {
+
+ public void m(boolean recurse) {
+ System.out.println(recurse);
+ if (recurse) {
+ m(false);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/graph/invokespecial/TestClassDump.java b/src/test/java/com/android/tools/r8/graph/invokespecial/TestClassDump.java
new file mode 100644
index 0000000..ae336834
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/graph/invokespecial/TestClassDump.java
@@ -0,0 +1,67 @@
+// 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.graph.invokespecial;
+
+import org.objectweb.asm.AnnotationVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+// Generated by running ./tools/asmifier.py build/classes/test/com/android/tools/r8/graph/-
+// invokespecial/TestClass.class, and changing the invoke-virtual TestClass.m() instruction to
+// an invoke-special instruction.
+public class TestClassDump implements Opcodes {
+
+ public static byte[] dump() throws Exception {
+
+ ClassWriter cw = new ClassWriter(0);
+ FieldVisitor fv;
+ MethodVisitor mv;
+ AnnotationVisitor av0;
+
+ cw.visit(
+ V1_8,
+ ACC_PUBLIC + ACC_SUPER,
+ "com/android/tools/r8/graph/invokespecial/TestClass",
+ null,
+ "java/lang/Object",
+ null);
+
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(1, 1);
+ mv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "m", "(Z)V", null, null);
+ mv.visitCode();
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitVarInsn(ILOAD, 1);
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/io/PrintStream", "println", "(Z)V", false);
+ mv.visitVarInsn(ILOAD, 1);
+ Label l0 = new Label();
+ mv.visitJumpInsn(IFEQ, l0);
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitInsn(ICONST_0);
+ // Note: Changed from INVOKEVIRTUAL to INVOKESPECIAL.
+ mv.visitMethodInsn(
+ INVOKESPECIAL, "com/android/tools/r8/graph/invokespecial/TestClass", "m", "(Z)V", false);
+ mv.visitLabel(l0);
+ mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(2, 2);
+ mv.visitEnd();
+ }
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/b72391662/B72391662.java b/src/test/java/com/android/tools/r8/naming/b72391662/B72391662.java
index c82bbc9..9a9ca34 100644
--- a/src/test/java/com/android/tools/r8/naming/b72391662/B72391662.java
+++ b/src/test/java/com/android/tools/r8/naming/b72391662/B72391662.java
@@ -4,52 +4,303 @@
package com.android.tools.r8.naming.b72391662;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
-import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper.DexVm.Version;
import com.android.tools.r8.VmTestRunner;
import com.android.tools.r8.VmTestRunner.IgnoreIfVmOlderThan;
import com.android.tools.r8.naming.b72391662.subpackage.OtherPackageSuper;
import com.android.tools.r8.naming.b72391662.subpackage.OtherPackageTestClass;
+import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatabilityTestBase;
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 java.util.List;
+import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
@RunWith(VmTestRunner.class)
-public class B72391662 extends TestBase {
+public class B72391662 extends ProguardCompatabilityTestBase {
- private void doTest(boolean allowAccessModification, boolean minify) throws Exception {
+ private static final List<Class> CLASSES = ImmutableList.of(
+ TestMain.class, Interface.class, Super.class, TestClass.class,
+ OtherPackageSuper.class, OtherPackageTestClass.class);
+
+ private void doTest_keepAll(
+ Shrinker shrinker,
+ String repackagePrefix,
+ boolean allowAccessModification,
+ boolean minify) throws Exception {
Class mainClass = TestMain.class;
+ String keep = !minify ? "-keep" : "-keep,allowobfuscation";
List<String> config = ImmutableList.of(
- allowAccessModification ?"-allowaccessmodification" : "",
+ "-printmapping",
+ repackagePrefix != null ? "-repackageclasses '" + repackagePrefix + "'" : "",
+ allowAccessModification ? "-allowaccessmodification" : "",
!minify ? "-dontobfuscate" : "",
"-keep class " + mainClass.getCanonicalName() + " {",
" public void main(java.lang.String[]);",
"}",
- "-keep class " + TestClass.class.getCanonicalName() + " {",
+ keep + " class " + TestClass.class.getCanonicalName() + " {",
" *;",
"}",
- "-keep class " + OtherPackageTestClass.class.getCanonicalName() + " {",
+ keep + " class " + OtherPackageTestClass.class.getCanonicalName() + " {",
" *;",
- "}"
+ "}",
+ "-dontwarn java.lang.invoke.*"
);
- AndroidApp app = readClassesAndAndriodJar(ImmutableList.of(
- mainClass, Interface.class, Super.class, TestClass.class,
- OtherPackageSuper.class, OtherPackageTestClass.class));
- app = compileWithR8(app, String.join(System.lineSeparator(), config));
+ AndroidApp app = runShrinkerRaw(shrinker, CLASSES, config);
assertEquals("123451234567\nABC\n", runOnArt(app, mainClass.getCanonicalName()));
+
+ DexInspector dexInspector =
+ isR8(shrinker) ? new DexInspector(app) : new DexInspector(app, proguardMap);
+ ClassSubject testClass = dexInspector.clazz(TestClass.class);
+ assertThat(testClass, isPresent());
+
+ // Test the totally unused method.
+ MethodSubject staticMethod = testClass.method("void", "unused", ImmutableList.of());
+ assertThat(staticMethod, isPresent());
+ assertEquals(minify, staticMethod.isRenamed());
+ if (isR8(shrinker)) {
+ assertEquals(allowAccessModification, staticMethod.getMethod().accessFlags.isPublic());
+ } else {
+ assertFalse(staticMethod.getMethod().accessFlags.isPublic());
+ }
+
+ // Test an indirectly referred method.
+ staticMethod = testClass.method("java.lang.String", "staticMethod", ImmutableList.of());
+ assertThat(staticMethod, isPresent());
+ assertEquals(minify, staticMethod.isRenamed());
+ boolean publicizeCondition = isR8(shrinker) ? allowAccessModification
+ : minify && repackagePrefix != null && allowAccessModification;
+ assertEquals(publicizeCondition, staticMethod.getMethod().accessFlags.isPublic());
}
@Test
@IgnoreIfVmOlderThan(Version.V7_0_0)
- public void test() throws Exception {
- doTest(true, true);
- doTest(true, false);
- doTest(false, true);
- doTest(false, false);
+ public void test_keepAll_R8() throws Exception {
+ doTest_keepAll(Shrinker.R8, "r8", true, true);
+ doTest_keepAll(Shrinker.R8, "r8", true, false);
+ doTest_keepAll(Shrinker.R8, "r8", false, true);
+ doTest_keepAll(Shrinker.R8, "r8", false, false);
+ doTest_keepAll(Shrinker.R8, null, true, true);
+ doTest_keepAll(Shrinker.R8, null, true, false);
+ doTest_keepAll(Shrinker.R8, null, false, true);
+ doTest_keepAll(Shrinker.R8, null, false, false);
+ }
+
+ @Ignore("b/92236970")
+ @Test
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
+ public void test_keepAll_R8Compat() throws Exception {
+ doTest_keepAll(Shrinker.R8_COMPAT, "rc", true, true);
+ doTest_keepAll(Shrinker.R8_COMPAT, "rc", true, false);
+ doTest_keepAll(Shrinker.R8_COMPAT, "rc", false, true);
+ doTest_keepAll(Shrinker.R8_COMPAT, "rc", false, false);
+ doTest_keepAll(Shrinker.R8_COMPAT, null, true, true);
+ doTest_keepAll(Shrinker.R8_COMPAT, null, true, false);
+ doTest_keepAll(Shrinker.R8_COMPAT, null, false, true);
+ doTest_keepAll(Shrinker.R8_COMPAT, null, false, false);
+ }
+
+ @Test
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
+ public void test_keepAll_Proguard6() throws Exception {
+ doTest_keepAll(Shrinker.PROGUARD6_THEN_D8, "pg", true, true);
+ doTest_keepAll(Shrinker.PROGUARD6_THEN_D8, "pg", true, false);
+ doTest_keepAll(Shrinker.PROGUARD6_THEN_D8, "pg", false, true);
+ doTest_keepAll(Shrinker.PROGUARD6_THEN_D8, "pg", false, false);
+ doTest_keepAll(Shrinker.PROGUARD6_THEN_D8, null, true, true);
+ doTest_keepAll(Shrinker.PROGUARD6_THEN_D8, null, true, false);
+ doTest_keepAll(Shrinker.PROGUARD6_THEN_D8, null, false, true);
+ doTest_keepAll(Shrinker.PROGUARD6_THEN_D8, null, false, false);
+ }
+
+ private void doTest_keepNonPublic(
+ Shrinker shrinker,
+ String repackagePrefix,
+ boolean allowAccessModification,
+ boolean minify) throws Exception {
+ Class mainClass = TestMain.class;
+ String keep = !minify ? "-keep" : "-keep,allowobfuscation";
+ List<String> config = ImmutableList.of(
+ "-printmapping",
+ repackagePrefix != null ? "-repackageclasses '" + repackagePrefix + "'" : "",
+ allowAccessModification ? "-allowaccessmodification" : "",
+ !minify ? "-dontobfuscate" : "",
+ "-keep class " + mainClass.getCanonicalName() + " {",
+ " public void main(java.lang.String[]);",
+ "}",
+ keep + " class " + TestClass.class.getCanonicalName() + " {",
+ " !public <methods>;",
+ "}",
+ keep + " class " + OtherPackageTestClass.class.getCanonicalName() + " {",
+ " !public <methods>;",
+ "}",
+ "-dontwarn java.lang.invoke.*"
+ );
+
+ AndroidApp app = runShrinkerRaw(shrinker, CLASSES, config);
+ assertEquals("123451234567\nABC\n", runOnArt(app, mainClass.getCanonicalName()));
+
+ DexInspector dexInspector =
+ isR8(shrinker) ? new DexInspector(app) : new DexInspector(app, proguardMap);
+ ClassSubject testClass = dexInspector.clazz(TestClass.class);
+ assertThat(testClass, isPresent());
+
+ // Test the totally unused method.
+ MethodSubject staticMethod = testClass.method("void", "unused", ImmutableList.of());
+ assertThat(staticMethod, isPresent());
+ assertEquals(minify, staticMethod.isRenamed());
+ if (isR8(shrinker)) {
+ assertEquals(allowAccessModification, staticMethod.getMethod().accessFlags.isPublic());
+ } else {
+ assertFalse(staticMethod.getMethod().accessFlags.isPublic());
+ }
+
+ // Test an indirectly referred method.
+ staticMethod = testClass.method("java.lang.String", "staticMethod", ImmutableList.of());
+ assertThat(staticMethod, isPresent());
+ assertEquals(minify, staticMethod.isRenamed());
+ boolean publicizeCondition = isR8(shrinker) ? allowAccessModification
+ : minify && repackagePrefix != null && allowAccessModification;
+ assertEquals(publicizeCondition, staticMethod.getMethod().accessFlags.isPublic());
+ }
+
+ @Test
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
+ public void test_keepNonPublic_R8() throws Exception {
+ doTest_keepNonPublic(Shrinker.R8, "r8", true, true);
+ doTest_keepNonPublic(Shrinker.R8, "r8", true, false);
+ doTest_keepNonPublic(Shrinker.R8, "r8", false, true);
+ doTest_keepNonPublic(Shrinker.R8, "r8", false, false);
+ doTest_keepNonPublic(Shrinker.R8, null, true, true);
+ doTest_keepNonPublic(Shrinker.R8, null, true, false);
+ doTest_keepNonPublic(Shrinker.R8, null, false, true);
+ doTest_keepNonPublic(Shrinker.R8, null, false, false);
+ }
+
+ @Ignore("b/92236970")
+ @Test
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
+ public void test_keepNonPublic_R8Compat() throws Exception {
+ doTest_keepNonPublic(Shrinker.R8_COMPAT, "rc", true, true);
+ doTest_keepNonPublic(Shrinker.R8_COMPAT, "rc", true, false);
+ doTest_keepNonPublic(Shrinker.R8_COMPAT, "rc", false, true);
+ doTest_keepNonPublic(Shrinker.R8_COMPAT, "rc", false, false);
+ doTest_keepNonPublic(Shrinker.R8_COMPAT, null, true, true);
+ doTest_keepNonPublic(Shrinker.R8_COMPAT, null, true, false);
+ doTest_keepNonPublic(Shrinker.R8_COMPAT, null, false, true);
+ doTest_keepNonPublic(Shrinker.R8_COMPAT, null, false, false);
+ }
+
+ @Test
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
+ public void test_keepNonPublic_Proguard6() throws Exception {
+ doTest_keepNonPublic(Shrinker.PROGUARD6_THEN_D8, "pg", true, true);
+ doTest_keepNonPublic(Shrinker.PROGUARD6_THEN_D8, "pg", true, false);
+ doTest_keepNonPublic(Shrinker.PROGUARD6_THEN_D8, "pg", false, true);
+ doTest_keepNonPublic(Shrinker.PROGUARD6_THEN_D8, "pg", false, false);
+ doTest_keepNonPublic(Shrinker.PROGUARD6_THEN_D8, null, true, true);
+ doTest_keepNonPublic(Shrinker.PROGUARD6_THEN_D8, null, true, false);
+ doTest_keepNonPublic(Shrinker.PROGUARD6_THEN_D8, null, false, true);
+ doTest_keepNonPublic(Shrinker.PROGUARD6_THEN_D8, null, false, false);
+ }
+
+ private void doTest_keepPublic(
+ Shrinker shrinker,
+ String repackagePrefix,
+ boolean allowAccessModification,
+ boolean minify) throws Exception {
+ Class mainClass = TestMain.class;
+ String keep = !minify ? "-keep" : "-keep,allowobfuscation";
+ List<String> config = ImmutableList.of(
+ "-printmapping",
+ repackagePrefix != null ? "-repackageclasses '" + repackagePrefix + "'" : "",
+ allowAccessModification ? "-allowaccessmodification" : "",
+ !minify ? "-dontobfuscate" : "",
+ "-keep class " + mainClass.getCanonicalName() + " {",
+ " public void main(java.lang.String[]);",
+ "}",
+ keep + " class " + TestClass.class.getCanonicalName() + " {",
+ " public <methods>;",
+ "}",
+ keep + " class " + OtherPackageTestClass.class.getCanonicalName() + " {",
+ " public <methods>;",
+ "}",
+ "-dontwarn java.lang.invoke.*"
+ );
+
+ AndroidApp app = runShrinkerRaw(shrinker, CLASSES, config);
+ assertEquals("123451234567\nABC\n", runOnArt(app, mainClass.getCanonicalName()));
+
+ DexInspector dexInspector =
+ isR8(shrinker) ? new DexInspector(app) : new DexInspector(app, proguardMap);
+ ClassSubject testClass = dexInspector.clazz(TestClass.class);
+ assertThat(testClass, isPresent());
+
+ // Test the totally unused method.
+ MethodSubject staticMethod = testClass.method("void", "unused", ImmutableList.of());
+ assertThat(staticMethod, not(isPresent()));
+
+ // Test an indirectly referred method.
+ staticMethod = testClass.method("java.lang.String", "staticMethod", ImmutableList.of());
+ if (isR8(shrinker)) {
+ // Inlined.
+ assertThat(staticMethod, not(isPresent()));
+ } else {
+ assertThat(staticMethod, isPresent());
+ assertEquals(minify, staticMethod.isRenamed());
+ boolean publicizeCondition = minify && repackagePrefix != null && allowAccessModification;
+ assertEquals(publicizeCondition, staticMethod.getMethod().accessFlags.isPublic());
+ }
+ }
+
+ @Test
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
+ public void test_keepPublic_R8() throws Exception {
+ doTest_keepPublic(Shrinker.R8, "r8", true, true);
+ doTest_keepPublic(Shrinker.R8, "r8", true, false);
+ doTest_keepPublic(Shrinker.R8, "r8", false, true);
+ doTest_keepPublic(Shrinker.R8, "r8", false, false);
+ doTest_keepPublic(Shrinker.R8, null, true, true);
+ doTest_keepPublic(Shrinker.R8, null, true, false);
+ doTest_keepPublic(Shrinker.R8, null, false, true);
+ doTest_keepPublic(Shrinker.R8, null, false, false);
+ }
+
+ @Ignore("b/92236970")
+ @Test
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
+ public void test_keepPublic_R8Compat() throws Exception {
+ doTest_keepPublic(Shrinker.R8_COMPAT, "rc", true, true);
+ doTest_keepPublic(Shrinker.R8_COMPAT, "rc", true, false);
+ doTest_keepPublic(Shrinker.R8_COMPAT, "rc", false, true);
+ doTest_keepPublic(Shrinker.R8_COMPAT, "rc", false, false);
+ doTest_keepPublic(Shrinker.R8_COMPAT, null, true, true);
+ doTest_keepPublic(Shrinker.R8_COMPAT, null, true, false);
+ doTest_keepPublic(Shrinker.R8_COMPAT, null, false, true);
+ doTest_keepPublic(Shrinker.R8_COMPAT, null, false, false);
+ }
+
+ @Test
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
+ public void test_keepPublic_Proguard6() throws Exception {
+ doTest_keepPublic(Shrinker.PROGUARD6_THEN_D8, "pg", true, true);
+ doTest_keepPublic(Shrinker.PROGUARD6_THEN_D8, "pg", true, false);
+ doTest_keepPublic(Shrinker.PROGUARD6_THEN_D8, "pg", false, true);
+ doTest_keepPublic(Shrinker.PROGUARD6_THEN_D8, "pg", false, false);
+ doTest_keepPublic(Shrinker.PROGUARD6_THEN_D8, null, true, true);
+ doTest_keepPublic(Shrinker.PROGUARD6_THEN_D8, null, true, false);
+ doTest_keepPublic(Shrinker.PROGUARD6_THEN_D8, null, false, true);
+ doTest_keepPublic(Shrinker.PROGUARD6_THEN_D8, null, false, false);
}
}
diff --git a/src/test/java/com/android/tools/r8/naming/b72391662/TestClass.java b/src/test/java/com/android/tools/r8/naming/b72391662/TestClass.java
index 88efc09..f13700c 100644
--- a/src/test/java/com/android/tools/r8/naming/b72391662/TestClass.java
+++ b/src/test/java/com/android/tools/r8/naming/b72391662/TestClass.java
@@ -18,6 +18,10 @@
return value;
}
+ static void unused() {
+ System.out.println("This should be discarded unless there is a keep rule.");
+ }
+
static String staticMethod() {
return "1";
}
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatabilityTestBase.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatabilityTestBase.java
index 3976f89..10c64e3 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatabilityTestBase.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatabilityTestBase.java
@@ -6,24 +6,31 @@
import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
import com.android.tools.r8.CompatProguardCommandBuilder;
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.ToolHelper.ProcessResult;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApiLevel;
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.FileUtils;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
import java.io.File;
import java.nio.file.Path;
import java.util.List;
+import java.util.function.Consumer;
public class ProguardCompatabilityTestBase extends TestBase {
+ protected Path proguardMap;
+
public enum Shrinker {
PROGUARD5,
PROGUARD6,
@@ -32,6 +39,34 @@
R8
}
+ protected static boolean isR8(Shrinker shrinker) {
+ return shrinker == Shrinker.R8_COMPAT || shrinker == Shrinker.R8;
+ }
+
+ protected AndroidApp runShrinkerRaw(
+ Shrinker mode, List<Class> programClasses, List<String> proguadConfigs) throws Exception {
+ return runShrinkerRaw(
+ mode, programClasses, String.join(System.lineSeparator(), proguadConfigs));
+ }
+
+ protected AndroidApp runShrinkerRaw(
+ Shrinker mode, List<Class> programClasses, String proguardConfig) throws Exception {
+ proguardMap = File.createTempFile("proguard", ".map", temp.getRoot()).toPath();
+ switch (mode) {
+ case PROGUARD5:
+ return runProguard5Raw(programClasses, proguardConfig, proguardMap);
+ case PROGUARD6:
+ return runProguard6Raw(programClasses, proguardConfig, proguardMap);
+ case PROGUARD6_THEN_D8:
+ return runProguard6AndD8Raw(programClasses, proguardConfig, proguardMap);
+ case R8_COMPAT:
+ return runR8CompatRaw(programClasses, proguardConfig);
+ case R8:
+ return runR8Raw(programClasses, proguardConfig);
+ }
+ throw new IllegalArgumentException("Unknown shrinker: " + mode);
+ }
+
protected DexInspector runShrinker(
Shrinker mode, List<Class> programClasses, List<String> proguadConfigs) throws Exception {
return runShrinker(mode, programClasses, String.join(System.lineSeparator(), proguadConfigs));
@@ -54,21 +89,38 @@
throw new IllegalArgumentException("Unknown shrinker: " + mode);
}
- protected DexInspector runR8(List<Class> programClasses, String proguardConfig) throws Exception {
- AndroidApp app = readClasses(programClasses);
- R8Command.Builder builder = ToolHelper.prepareR8CommandBuilder(app);
- builder.addProguardConfiguration(ImmutableList.of(proguardConfig), Origin.unknown());
- return new DexInspector(ToolHelper.runR8(builder.build()));
+ protected AndroidApp runR8Raw(List<Class> programClasses, String proguardConfig)
+ throws Exception {
+ return runR8Raw(programClasses, proguardConfig, null);
}
- protected DexInspector runR8Compat(
+ protected AndroidApp runR8Raw(
+ List<Class> programClasses, String proguardConfig, Consumer<InternalOptions> configure)
+ throws Exception {
+ AndroidApp app = readClassesAndAndriodJar(programClasses);
+ R8Command.Builder builder = ToolHelper.prepareR8CommandBuilder(app);
+ builder.addProguardConfiguration(ImmutableList.of(proguardConfig), Origin.unknown());
+ return ToolHelper.runR8(builder.build(), configure);
+ }
+
+ protected DexInspector runR8(List<Class> programClasses, String proguardConfig) throws Exception {
+ return new DexInspector(runR8Raw(programClasses, proguardConfig));
+ }
+
+ protected AndroidApp runR8CompatRaw(
List<Class> programClasses, String proguardConfig) throws Exception {
CompatProguardCommandBuilder builder = new CompatProguardCommandBuilder(true);
builder.addProguardConfiguration(ImmutableList.of(proguardConfig), Origin.unknown());
programClasses.forEach(
clazz -> builder.addProgramFiles(ToolHelper.getClassFileForTestClass(clazz)));
+ builder.addLibraryFiles(ToolHelper.getDefaultAndroidJar());
builder.setProgramConsumer(DexIndexedConsumer.emptyConsumer());
- return new DexInspector(ToolHelper.runR8(builder.build()));
+ return ToolHelper.runR8(builder.build());
+ }
+
+ protected DexInspector runR8Compat(
+ List<Class> programClasses, String proguardConfig) throws Exception {
+ return new DexInspector(runR8CompatRaw(programClasses, proguardConfig));
}
protected DexInspector runR8CompatKeepingMain(Class mainClass, List<Class> programClasses)
@@ -76,41 +128,79 @@
return runR8Compat(programClasses, keepMainProguardConfiguration(mainClass));
}
- protected DexInspector runProguard5(
- List<Class> programClasses, String proguardConfig) throws Exception {
+ protected AndroidApp runProguard5Raw(
+ List<Class> programClasses, String proguardConfig, Path proguardMap) throws Exception {
Path proguardedJar =
File.createTempFile("proguarded", FileUtils.JAR_EXTENSION, temp.getRoot()).toPath();
Path proguardConfigFile = File.createTempFile("proguard", ".config", temp.getRoot()).toPath();
- Path proguardMap = File.createTempFile("proguard", ".map", temp.getRoot()).toPath();
FileUtils.writeTextFile(proguardConfigFile, proguardConfig);
- ToolHelper.runProguard(
- jarTestClasses(programClasses), proguardedJar, proguardConfigFile, proguardMap);
- return new DexInspector(readJar(proguardedJar), proguardMap);
+ ProcessResult result = ToolHelper.runProguardRaw(
+ jarTestClasses(programClasses),
+ proguardedJar,
+ ToolHelper.getAndroidJar(AndroidApiLevel.N),
+ proguardConfigFile,
+ proguardMap);
+ if (result.exitCode != 0) {
+ fail("Proguard failed, exit code " + result.exitCode + ", stderr:\n" + result.stderr);
+ }
+ return readJar(proguardedJar);
+ }
+
+ protected DexInspector runProguard5(
+ List<Class> programClasses, String proguardConfig) throws Exception {
+ proguardMap = File.createTempFile("proguard", ".map", temp.getRoot()).toPath();
+ return new DexInspector(
+ runProguard5Raw(programClasses, proguardConfig, proguardMap), proguardMap);
+ }
+
+ protected AndroidApp runProguard6Raw(
+ List<Class> programClasses, String proguardConfig, Path proguardMap) throws Exception {
+ Path proguardedJar =
+ File.createTempFile("proguarded", FileUtils.JAR_EXTENSION, temp.getRoot()).toPath();
+ Path proguardConfigFile = File.createTempFile("proguard", ".config", temp.getRoot()).toPath();
+ FileUtils.writeTextFile(proguardConfigFile, proguardConfig);
+ ProcessResult result = ToolHelper.runProguard6Raw(
+ jarTestClasses(programClasses),
+ proguardedJar,
+ ToolHelper.getAndroidJar(AndroidApiLevel.N),
+ proguardConfigFile,
+ proguardMap);
+ if (result.exitCode != 0) {
+ fail("Proguard failed, exit code " + result.exitCode + ", stderr:\n" + result.stderr);
+ }
+ return readJar(proguardedJar);
}
protected DexInspector runProguard6(
List<Class> programClasses, String proguardConfig) throws Exception {
+ proguardMap = File.createTempFile("proguard", ".map", temp.getRoot()).toPath();
+ return new DexInspector(
+ runProguard6Raw(programClasses, proguardConfig, proguardMap), proguardMap);
+ }
+
+ protected AndroidApp runProguard6AndD8Raw(
+ List<Class> programClasses, String proguardConfig, Path proguardMap) throws Exception {
Path proguardedJar =
File.createTempFile("proguarded", FileUtils.JAR_EXTENSION, temp.getRoot()).toPath();
Path proguardConfigFile = File.createTempFile("proguard", ".config", temp.getRoot()).toPath();
- Path proguardMap = File.createTempFile("proguard", ".map", temp.getRoot()).toPath();
FileUtils.writeTextFile(proguardConfigFile, proguardConfig);
- ToolHelper.runProguard6(
- jarTestClasses(programClasses), proguardedJar, proguardConfigFile, proguardMap);
- return new DexInspector(readJar(proguardedJar), proguardMap);
+ ProcessResult result = ToolHelper.runProguard6Raw(
+ jarTestClasses(programClasses),
+ proguardedJar,
+ ToolHelper.getAndroidJar(AndroidApiLevel.N),
+ proguardConfigFile,
+ proguardMap);
+ if (result.exitCode != 0) {
+ fail("Proguard failed, exit code " + result.exitCode + ", stderr:\n" + result.stderr);
+ }
+ return ToolHelper.runD8(readJar(proguardedJar));
}
protected DexInspector runProguard6AndD8(
List<Class> programClasses, String proguardConfig) throws Exception {
- Path proguardedJar =
- File.createTempFile("proguarded", FileUtils.JAR_EXTENSION, temp.getRoot()).toPath();
- Path proguardConfigFile = File.createTempFile("proguard", ".config", temp.getRoot()).toPath();
- Path proguardMap = File.createTempFile("proguard", ".map", temp.getRoot()).toPath();
- FileUtils.writeTextFile(proguardConfigFile, proguardConfig);
- ToolHelper.runProguard6(
- jarTestClasses(programClasses), proguardedJar, proguardConfigFile, proguardMap);
- AndroidApp app = ToolHelper.runD8(readJar(proguardedJar));
- return new DexInspector(app, proguardMap);
+ proguardMap = File.createTempFile("proguard", ".map", temp.getRoot()).toPath();
+ return new DexInspector(
+ runProguard6AndD8Raw(programClasses, proguardConfig, proguardMap), proguardMap);
}
protected DexInspector runProguardKeepingMain(Class mainClass, List<Class> programClasses)
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
new file mode 100644
index 0000000..65d1763
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
@@ -0,0 +1,195 @@
+// 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.shaking.ifrule;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatabilityTestBase;
+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 java.util.Collection;
+import java.util.List;
+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 IfOnAccessModifierTest extends ProguardCompatabilityTestBase {
+ private final static List<Class> CLASSES = ImmutableList.of(
+ ClassForIf.class, ClassForSubsequent.class,
+ MainForAccessModifierTest.class);
+
+ private final Shrinker shrinker;
+ private final MethodSignature nonPublicMethod;
+ private final MethodSignature publicMethod;
+
+ public IfOnAccessModifierTest(Shrinker shrinker) {
+ this.shrinker = shrinker;
+ nonPublicMethod = new MethodSignature("nonPublicMethod", "void", ImmutableList.of());
+ publicMethod = new MethodSignature("publicMethod", "void", ImmutableList.of());
+ }
+
+ @Parameters(name = "shrinker: {0}")
+ public static Collection<Object> data() {
+ return ImmutableList.of(Shrinker.PROGUARD6, Shrinker.R8);
+ }
+
+ @Test
+ public void ifOnNonPublic_keepOnPublic() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-printmapping",
+ "-repackageclasses 'top'",
+ "-allowaccessmodification",
+ "-keep class **.Main* {",
+ " public static void callIfNonPublic();",
+ "}",
+ "-if class **.ClassForIf {",
+ " !public <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " public <methods>;",
+ "}"
+ );
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ ClassSubject classSubject = dexInspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, isPresent());
+ assertTrue(methodSubject.getMethod().accessFlags.isPublic());
+
+ classSubject = dexInspector.clazz(ClassForSubsequent.class);
+ if (isR8(shrinker)) {
+ // TODO(b/72109068): ClassForIf#nonPublicMethod becomes public, and -if rule is not applied
+ // at the 2nd tree shaking.
+ assertThat(classSubject, not(isPresent()));
+ return;
+ }
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, isPresent());
+ methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, not(isPresent()));
+ }
+
+ @Test
+ public void ifOnNonPublic_keepOnNonPublic() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-printmapping",
+ "-repackageclasses 'top'",
+ "-allowaccessmodification",
+ "-keep class **.Main* {",
+ " public static void callIfNonPublic();",
+ "}",
+ "-if class **.ClassForIf {",
+ " !public <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " !public <methods>;",
+ "}"
+ );
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ ClassSubject classSubject = dexInspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, isPresent());
+ assertTrue(methodSubject.getMethod().accessFlags.isPublic());
+
+ classSubject = dexInspector.clazz(ClassForSubsequent.class);
+ if (isR8(shrinker)) {
+ // TODO(b/72109068): ClassForIf#nonPublicMethod becomes public, and -if rule is not applied
+ // at the 2nd tree shaking.
+ assertThat(classSubject, not(isPresent()));
+ return;
+ }
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, isPresent());
+ assertFalse(methodSubject.getMethod().accessFlags.isPublic());
+ }
+
+ @Test
+ public void ifOnPublic_keepOnPublic() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-printmapping",
+ "-repackageclasses 'top'",
+ "-allowaccessmodification",
+ "-keep class **.Main* {",
+ " public static void callIfPublic();",
+ "}",
+ "-if class **.ClassForIf {",
+ " public <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " public <methods>;",
+ "}"
+ );
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ ClassSubject classSubject = dexInspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, isPresent());
+ methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, not(isPresent()));
+
+ classSubject = dexInspector.clazz(ClassForSubsequent.class);
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, isPresent());
+ methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, not(isPresent()));
+ }
+
+ @Test
+ public void ifOnPublic_keepOnNonPublic() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-printmapping",
+ "-repackageclasses 'top'",
+ "-allowaccessmodification",
+ "-keep class **.Main* {",
+ " public static void callIfPublic();",
+ "}",
+ "-if class **.ClassForIf {",
+ " public <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " !public <methods>;",
+ "}"
+ );
+ DexInspector dexInspector = runShrinker(shrinker, CLASSES, config);
+ ClassSubject classSubject = dexInspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, isPresent());
+ methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, not(isPresent()));
+
+ classSubject = dexInspector.clazz(ClassForSubsequent.class);
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.method(nonPublicMethod);
+ if (isR8(shrinker)) {
+ // TODO(b/72109068): if kept in the 1st tree shaking, should be kept after publicizing.
+ assertThat(methodSubject, not(isPresent()));
+ return;
+ }
+ assertThat(methodSubject, isPresent());
+ assertFalse(methodSubject.getMethod().accessFlags.isPublic());
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTestClasses.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTestClasses.java
new file mode 100644
index 0000000..1b5bd75
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTestClasses.java
@@ -0,0 +1,40 @@
+// 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.shaking.ifrule;
+
+class ClassForIf {
+ ClassForIf() {
+ }
+
+ synchronized void nonPublicMethod() {
+ System.out.println("ClassForIf::nonPublicMethod");
+ }
+
+ synchronized public void publicMethod() {
+ System.out.println("ClassForIf::publicMethod");
+ }
+}
+
+class ClassForSubsequent {
+ ClassForSubsequent() {
+ }
+
+ synchronized void nonPublicMethod() {
+ System.out.println("ClassForSubsequent::nonPublicMethod");
+ }
+
+ synchronized public void publicMethod() {
+ System.out.println("ClassForSubsequent::publicMethod");
+ }
+}
+
+class MainForAccessModifierTest {
+ public static void callIfNonPublic() {
+ new ClassForIf().nonPublicMethod();
+ }
+
+ public static void callIfPublic() {
+ new ClassForIf().publicMethod();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/inlining/IfRuleWithInlining.java b/src/test/java/com/android/tools/r8/shaking/ifrule/inlining/IfRuleWithInlining.java
new file mode 100644
index 0000000..14391ee
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/inlining/IfRuleWithInlining.java
@@ -0,0 +1,105 @@
+// 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.shaking.ifrule.inlining;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatabilityTestBase;
+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.Collection;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+class A {
+ static public int a() {
+ try {
+ String p = A.class.getPackage().getName();
+ Class.forName(p + ".D");
+ } catch (ClassNotFoundException e) {
+ return 2;
+ }
+ return 1;
+ }
+}
+
+class B {
+ // Depending on inlining option, this method is kept to make inlining of A.a() infeasible.
+ void x() {
+ System.out.print("" + A.a() + A.a() + A.a() + A.a() + A.a() + A.a() + A.a() + A.a());
+ }
+}
+
+class D {
+}
+
+class Main {
+ public static void main(String[] args) {
+ System.out.print("" + A.a());
+ }
+}
+
+@RunWith(Parameterized.class)
+public class IfRuleWithInlining extends ProguardCompatabilityTestBase {
+ private final static List<Class> CLASSES = ImmutableList.of(
+ A.class, B.class, D.class, Main.class);
+
+ private final Shrinker shrinker;
+ private final boolean inlineMethod;
+
+ public IfRuleWithInlining(Shrinker shrinker, boolean inlineMethod) {
+ this.shrinker = shrinker;
+ this.inlineMethod = inlineMethod;
+ }
+
+ @Parameters(name = "shrinker: {0} inlineMethod: {1}")
+ public static Collection<Object[]> data() {
+ // We don't run this on Proguard, as triggering inlining in Proguard is out of our control.
+ return ImmutableList.of(
+ new Object[]{Shrinker.R8, true},
+ new Object[]{Shrinker.R8, false}
+ );
+ }
+
+ private void check(AndroidApp app) throws Exception {
+ DexInspector inspector = new DexInspector(app);
+ ClassSubject clazzA = inspector.clazz(A.class);
+ assertThat(clazzA, isPresent());
+ // A.a might be inlined.
+ assertEquals(!inlineMethod, clazzA.method("int", "a", ImmutableList.of()).isPresent());
+ // TODO(110148109): class D should be present - inlining or not.
+ assertEquals(!inlineMethod, inspector.clazz(D.class).isPresent());
+ ProcessResult result = runOnArtRaw(app, Main.class.getName());
+ assertEquals(0, result.exitCode);
+ // TODO(110148109): Output should be the same - inlining or not.
+ assertEquals(!inlineMethod ? "1" : "2", result.stdout);
+ }
+
+ @Test
+ public void testMergedClassMethodInIfRule() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-keep class **.Main { public static void main(java.lang.String[]); }",
+ inlineMethod
+ ? "-alwaysinline class **.A { int a(); }"
+ : "-keep class **.B { *; }",
+ inlineMethod
+ ? "-checkdiscard class **.A { int a(); }"
+ : "",
+ "-if class **.A { static int a(); }",
+ "-keep class **.D",
+ "-dontobfuscate"
+ );
+
+ check(runShrinkerRaw(shrinker, CLASSES, config));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java
new file mode 100644
index 0000000..b484d73
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java
@@ -0,0 +1,156 @@
+// 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.shaking.ifrule.verticalclassmerging;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatabilityTestBase;
+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.InternalOptions;
+import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+class A {
+ int x = 1;
+ int a() throws ClassNotFoundException {
+ // Class D is expected to be kept - vertical class merging or not. The -if rule say that if
+ // the method A.a is in the output, then class D is needed.
+ String p =getClass().getPackage().getName();
+ Class.forName(p + ".D");
+ return 4;
+ }
+}
+
+class B extends A {
+ int y = 2;
+ int b() {
+ return 5;
+ }
+}
+
+class C extends B {
+ int z = 3;
+ int c() {
+ return 6;
+ }
+}
+
+class D {
+}
+
+class Main {
+ public static void main(String[] args) throws ClassNotFoundException {
+ C c = new C();
+ System.out.print("" + c.x + "" + c.y + "" + c.z + "" + c.a() + "" + c.b() + "" + c.c());
+ }
+}
+
+@RunWith(Parameterized.class)
+public class IfRuleWithVerticalClassMerging extends ProguardCompatabilityTestBase {
+ private final static List<Class> CLASSES = ImmutableList.of(
+ A.class, B.class, C.class, D.class, Main.class);
+
+ private final Shrinker shrinker;
+ private final boolean enableClassMerging;
+
+ public IfRuleWithVerticalClassMerging(Shrinker shrinker, boolean enableClassMerging) {
+ this.shrinker = shrinker;
+ this.enableClassMerging = enableClassMerging;
+ }
+
+ @Parameters(name = "shrinker: {0} classMerging: {1}")
+ public static Collection<Object[]> data() {
+ // We don't run this on Proguard, as Proguard does not merge A into B.
+ return ImmutableList.of(
+ new Object[]{Shrinker.R8, true},
+ new Object[]{Shrinker.R8, false}
+ );
+ }
+
+ private void configure(InternalOptions options) {
+ options.enableClassMerging = enableClassMerging;
+ }
+
+ @Override
+ protected AndroidApp runR8Raw(
+ List<Class> programClasses, String proguardConfig) throws Exception {
+ return super.runR8Raw(programClasses, proguardConfig, this::configure);
+ }
+
+ private void check(AndroidApp app) throws Exception {
+ DexInspector inspector = new DexInspector(app);
+ ClassSubject clazzA = inspector.clazz(A.class);
+ assertEquals(!enableClassMerging, clazzA.isPresent());
+ ClassSubject clazzB = inspector.clazz(B.class);
+ assertThat(clazzB, isPresent());
+ ClassSubject clazzD = inspector.clazz(D.class);
+ // TODO(110141157): Class D should be kept - vertical class merging or not.
+ assertEquals(!enableClassMerging, clazzD.isPresent());
+
+ ProcessResult result = runOnArtRaw(app, Main.class.getName());
+ // TODO(110141157): The code should run - vertical class merging or not.
+ assertEquals(enableClassMerging ? 1 : 0, result.exitCode);
+ if (!enableClassMerging) {
+ assertEquals("123456", result.stdout);
+ }
+ }
+
+ @Test
+ public void testMergedClassInIfRule() throws Exception {
+ // Class C is kept, meaning that it will not be touched.
+ // Class A will be merged into class B.
+ List<String> config = ImmutableList.of(
+ "-keep class **.Main { public static void main(java.lang.String[]); }",
+ "-keep class **.C",
+ "-if class **.A",
+ "-keep class **.D",
+ "-dontobfuscate"
+ );
+
+ check(runShrinkerRaw(shrinker, CLASSES, config));
+ }
+
+ @Test
+ public void testMergedClassFieldInIfRule() throws Exception {
+ // Class C is kept, meaning that it will not be touched.
+ // Class A will be merged into class B.
+ // Main.main access A.x, so that field exists satisfying the if rule.
+ List<String> config = ImmutableList.of(
+ "-keep class **.Main { public static void main(java.lang.String[]); }",
+ "-keep class **.C",
+ "-if class **.A { int x; }",
+ "-keep class **.D",
+ "-dontobfuscate"
+ );
+
+ check(runShrinkerRaw(shrinker, CLASSES, config));
+ }
+
+ @Test
+ public void testMergedClassMethodInIfRule() throws Exception {
+ // Class C is kept, meaning that it will not be touched.
+ // Class A will be merged into class B.
+ // Main.main access A.a(), that method exists satisfying the if rule.
+ List<String> config = ImmutableList.of(
+ "-keep class **.Main { public static void main(java.lang.String[]); }",
+ "-keep class **.C",
+ "-if class **.A { int a(); }",
+ "-keep class **.D",
+ "-dontobfuscate"
+ );
+
+ check(runShrinkerRaw(shrinker, CLASSES, config));
+ }
+}
