Merge "Unique names in case of class merging conflicts"
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 01dcf0d..b05fd4f 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.utils.InternalOptions;
import java.util.ListIterator;
import java.util.function.Predicate;
@@ -30,6 +31,7 @@
private final CallSiteInformation callSiteInformation;
private final Predicate<DexEncodedMethod> isProcessedConcurrently;
private final InliningInfo info;
+ private final InternalOptions options;
private final int inliningInstructionLimit;
private int instructionAllowance;
@@ -40,6 +42,7 @@
TypeEnvironment typeEnvironment,
CallSiteInformation callSiteInformation,
Predicate<DexEncodedMethod> isProcessedConcurrently,
+ InternalOptions options,
int inliningInstructionLimit,
int inliningInstructionAllowance) {
this.inliner = inliner;
@@ -49,6 +52,7 @@
this.callSiteInformation = callSiteInformation;
this.isProcessedConcurrently = isProcessedConcurrently;
info = Log.ENABLED ? new InliningInfo(method) : null;
+ this.options = options;
this.inliningInstructionLimit = inliningInstructionLimit;
this.instructionAllowance = inliningInstructionAllowance;
}
@@ -167,6 +171,11 @@
return false;
}
+ if (options.testing.validInliningReasons != null
+ && !options.testing.validInliningReasons.contains(reason)) {
+ return false;
+ }
+
// Abort inlining attempt if method -> target access is not right.
if (!inliner.hasInliningAccess(method, candidate)) {
if (info != null) {
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 22b14a4..44ebdc7 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
@@ -413,6 +413,7 @@
typeEnvironment,
callSiteInformation,
isProcessedConcurrently,
+ options,
inliningInstructionLimit,
inliningInstructionAllowance);
}
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 eeaa9f3..c999cf3 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,7 @@
package com.android.tools.r8.shaking;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.DefaultUseRegistry;
import com.android.tools.r8.graph.DexAnnotationSet;
@@ -30,7 +31,6 @@
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;
import com.android.tools.r8.utils.FieldSignatureEquivalence;
import com.android.tools.r8.utils.MethodSignatureEquivalence;
@@ -57,6 +57,7 @@
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
/**
@@ -133,6 +134,12 @@
}
}
+ private enum Rename {
+ ALWAYS,
+ IF_NEEDED,
+ NEVER
+ }
+
private final DexApplication application;
private final AppInfoWithLiveness appInfo;
private final GraphLense graphLense;
@@ -603,20 +610,38 @@
Set<Wrapper<DexMethod>> existingMethods = new HashSet<>();
addAll(existingMethods, target.methods(), MethodSignatureEquivalence.get());
- List<DexEncodedMethod> directMethods = new ArrayList<>();
+ Map<Wrapper<DexMethod>, DexEncodedMethod> directMethods = new HashMap<>();
+ Map<Wrapper<DexMethod>, DexEncodedMethod> virtualMethods = new HashMap<>();
+
+ Predicate<DexMethod> availableMethodSignatures =
+ (method) -> {
+ Wrapper<DexMethod> wrapped = MethodSignatureEquivalence.get().wrap(method);
+ return !existingMethods.contains(wrapped)
+ && !directMethods.containsKey(wrapped)
+ && !virtualMethods.containsKey(wrapped);
+ };
+
for (DexEncodedMethod directMethod : source.directMethods()) {
if (directMethod.isInstanceInitializer()) {
- directMethods.add(renameConstructor(directMethod));
+ add(
+ directMethods,
+ renameConstructor(directMethod, availableMethodSignatures),
+ MethodSignatureEquivalence.get());
} else {
- directMethods.add(directMethod);
+ DexEncodedMethod resultingDirectMethod =
+ renameMethod(
+ directMethod,
+ availableMethodSignatures,
+ directMethod.isClassInitializer() ? Rename.NEVER : Rename.IF_NEEDED);
+ add(directMethods, resultingDirectMethod, MethodSignatureEquivalence.get());
+ deferredRenamings.map(directMethod.method, resultingDirectMethod.method);
if (!directMethod.isStaticMethod()) {
- blockRedirectionOfSuperCalls(directMethod.method);
+ blockRedirectionOfSuperCalls(resultingDirectMethod.method);
}
}
}
- List<DexEncodedMethod> virtualMethods = new ArrayList<>();
for (DexEncodedMethod virtualMethod : source.virtualMethods()) {
DexEncodedMethod shadowedBy = findMethodInTarget(virtualMethod);
if (shadowedBy != null) {
@@ -626,14 +651,20 @@
continue;
}
} else {
+ if (abortMerge) {
+ // If [virtualMethod] does not resolve to a single method in [target], abort.
+ return false;
+ }
+
// The method is not 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()) {
+ // Update the holder of [virtualMethod] using renameMethod().
DexEncodedMethod resultingVirtualMethod =
- renameMethod(virtualMethod, target.type, false);
+ renameMethod(virtualMethod, availableMethodSignatures, Rename.NEVER);
deferredRenamings.map(virtualMethod.method, resultingVirtualMethod.method);
- virtualMethods.add(resultingVirtualMethod);
+ add(virtualMethods, resultingVirtualMethod, MethodSignatureEquivalence.get());
continue;
}
}
@@ -641,9 +672,10 @@
// 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);
+ DexEncodedMethod resultingDirectMethod =
+ renameMethod(virtualMethod, availableMethodSignatures, Rename.ALWAYS);
makePrivate(resultingDirectMethod);
- directMethods.add(resultingDirectMethod);
+ add(directMethods, resultingDirectMethod, MethodSignatureEquivalence.get());
// Record that invoke-super instructions in the target class should be redirected to the
// newly created direct method.
@@ -657,7 +689,7 @@
// 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);
+ add(virtualMethods, shadowedBy, MethodSignatureEquivalence.get());
}
deferredRenamings.map(virtualMethod.method, shadowedBy.method);
@@ -665,41 +697,32 @@
Collection<DexEncodedMethod> mergedDirectMethods =
mergeItems(
- directMethods.iterator(),
+ directMethods.values().iterator(),
target.directMethods(),
MethodSignatureEquivalence.get(),
- existingMethods,
- (existing, method) -> {
- DexEncodedMethod renamedMethod = renameMethod(method, target.type, true);
- deferredRenamings.map(method.method, renamedMethod.method);
- blockRedirectionOfSuperCalls(renamedMethod.method);
- return renamedMethod;
- });
+ this::resolveMethodConflict);
Collection<DexEncodedMethod> mergedVirtualMethods =
mergeItems(
- virtualMethods.iterator(),
+ virtualMethods.values().iterator(),
target.virtualMethods(),
MethodSignatureEquivalence.get(),
- existingMethods,
- this::abortOnNonAbstract);
+ this::resolveMethodConflict);
if (abortMerge) {
return false;
}
// Step 2: Merge fields
- Set<Wrapper<DexField>> existingFields = new HashSet<>();
- addAll(existingFields, target.fields(), FieldSignatureEquivalence.get());
- Collection<DexEncodedField> mergedStaticFields = mergeItems(
- Iterators.forArray(source.staticFields()),
- target.staticFields(),
- FieldSignatureEquivalence.get(),
- existingFields,
- this::renameField);
- Collection<DexEncodedField> mergedInstanceFields = mergeItems(
- Iterators.forArray(source.instanceFields()),
- target.instanceFields(),
- FieldSignatureEquivalence.get(),
- existingFields,
- this::renameField);
+ Collection<DexEncodedField> mergedStaticFields =
+ mergeItems(
+ Iterators.forArray(source.staticFields()),
+ target.staticFields(),
+ FieldSignatureEquivalence.get(),
+ this::resolveFieldConflict);
+ Collection<DexEncodedField> mergedInstanceFields =
+ mergeItems(
+ Iterators.forArray(source.instanceFields()),
+ target.instanceFields(),
+ FieldSignatureEquivalence.get(),
+ this::resolveFieldConflict);
// Step 3: Merge interfaces
Set<DexType> interfaces = mergeArrays(target.interfaces.values, source.interfaces.values);
// Now destructively update the class.
@@ -845,6 +868,11 @@
return null;
}
+ private <T extends KeyedDexItem<S>, S extends PresortedComparable<S>> void add(
+ Map<Wrapper<S>, T> map, T item, Equivalence<S> equivalence) {
+ map.put(equivalence.wrap(item.getKey()), item);
+ }
+
private <T extends KeyedDexItem<S>, S extends PresortedComparable<S>> void addAll(
Collection<Wrapper<S>> collection, Iterable<T> items, Equivalence<S> equivalence) {
for (T item : items) {
@@ -863,78 +891,71 @@
Iterator<S> fromItems,
S[] toItems,
Equivalence<T> equivalence,
- Set<Wrapper<T>> existing,
- BiFunction<S, S, S> onConflict) {
- HashMap<Wrapper<T>, S> methods = new HashMap<>();
+ BiFunction<S, Predicate<T>, S> onConflict) {
+ Map<Wrapper<T>, S> map = new HashMap<>();
// First add everything from the target class. These items are not preprocessed.
for (S item : toItems) {
- methods.put(equivalence.wrap(item.getKey()), item);
+ map.put(equivalence.wrap(item.getKey()), item);
}
- // Now add the new methods, resolving shadowing.
- addNonShadowed(fromItems, methods, equivalence, existing, onConflict);
- return methods.values();
+ // Now add the new items, resolving shadowing.
+ addNonShadowed(fromItems, map, equivalence, onConflict);
+ return map.values();
}
private <T extends PresortedComparable<T>, S extends KeyedDexItem<T>> void addNonShadowed(
Iterator<S> items,
- HashMap<Wrapper<T>, S> map,
+ Map<Wrapper<T>, S> map,
Equivalence<T> equivalence,
- Set<Wrapper<T>> existing,
- BiFunction<S, S, S> onConflict) {
+ BiFunction<S, Predicate<T>, S> onConflict) {
+ Predicate<T> availableSignatures = key -> !map.containsKey(equivalence.wrap(key));
while (items.hasNext()) {
S item = items.next();
- if (item == null) {
- // This item was filtered out by a preprocessing.
- continue;
- }
+ assert item != null;
Wrapper<T> wrapped = equivalence.wrap(item.getKey());
- if (existing.contains(wrapped)) {
- S resolved = onConflict.apply(map.get(wrapped), item);
+ if (map.containsKey(wrapped)) {
+ S resolved = onConflict.apply(item, availableSignatures);
+ assert availableSignatures.test(resolved.getKey());
wrapped = equivalence.wrap(resolved.getKey());
map.put(wrapped, resolved);
+ assert !availableSignatures.test(resolved.getKey());
} else {
map.put(wrapped, item);
}
}
}
- // 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) {
+ // Note that names returned by this function are not necessarily unique. Clients should
+ // repeatedly try to generate a fresh name until it is unique.
+ private DexString getFreshName(String nameString, int index, DexType holder) {
String freshName = nameString + "$" + holder.toSourceString().replace('.', '$');
+ if (index > 1) {
+ freshName += index;
+ }
return application.dexItemFactory.createString(freshName);
}
- private DexEncodedMethod abortOnNonAbstract(DexEncodedMethod existing,
- DexEncodedMethod method) {
- // 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) {
- return method;
- }
- }
-
- // 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.
+ private DexEncodedMethod resolveMethodConflict(
+ DexEncodedMethod method, Predicate<DexMethod> availableMethodSignatures) {
+ assert false;
abortMerge = true;
return method;
}
- private DexEncodedMethod renameConstructor(DexEncodedMethod method) {
+ private DexEncodedMethod renameConstructor(
+ DexEncodedMethod method, Predicate<DexMethod> availableMethodSignatures) {
assert method.isInstanceInitializer();
- DexType holder = method.method.holder;
- DexEncodedMethod result =
- method.toRenamedMethod(
- getFreshName(CONSTRUCTOR_NAME, holder), application.dexItemFactory);
+ DexType oldHolder = method.method.holder;
+
+ DexMethod newSignature;
+ int count = 1;
+ do {
+ DexString newName = getFreshName(CONSTRUCTOR_NAME, count, oldHolder);
+ newSignature =
+ application.dexItemFactory.createMethod(target.type, method.method.proto, newName);
+ count++;
+ } while (!availableMethodSignatures.test(newSignature));
+
+ DexEncodedMethod result = method.toTypeSubstitutedMethod(newSignature);
result.markForceInline();
deferredRenamings.map(method.method, result.method);
// Renamed constructors turn into ordinary private functions. They can be private, as
@@ -945,25 +966,60 @@
}
private DexEncodedMethod renameMethod(
- DexEncodedMethod method, DexType newHolder, boolean useFreshName) {
+ DexEncodedMethod method, Predicate<DexMethod> availableMethodSignatures, Rename strategy) {
// We cannot handle renaming static initializers yet and constructors should have been
// renamed already.
- assert !method.accessFlags.isConstructor();
- DexType oldHolder = method.method.holder;
+ assert !method.accessFlags.isConstructor() || strategy == Rename.NEVER;
DexString oldName = method.method.name;
- DexString newName =
- useFreshName ? getFreshName(oldName.toSourceString(), oldHolder) : oldName;
- DexMethod newSignature =
- application.dexItemFactory.createMethod(newHolder, method.method.proto, newName);
+ DexType oldHolder = method.method.holder;
+
+ DexMethod newSignature;
+ switch (strategy) {
+ case IF_NEEDED:
+ newSignature =
+ application.dexItemFactory.createMethod(target.type, method.method.proto, oldName);
+ if (availableMethodSignatures.test(newSignature)) {
+ break;
+ }
+ // Fall-through to ALWAYS so that we assign a new name.
+
+ case ALWAYS:
+ int count = 1;
+ do {
+ DexString newName = getFreshName(oldName.toSourceString(), count, oldHolder);
+ newSignature =
+ application.dexItemFactory.createMethod(target.type, method.method.proto, newName);
+ count++;
+ } while (!availableMethodSignatures.test(newSignature));
+ break;
+
+ case NEVER:
+ newSignature =
+ application.dexItemFactory.createMethod(target.type, method.method.proto, oldName);
+ assert availableMethodSignatures.test(newSignature);
+ break;
+
+ default:
+ throw new Unreachable();
+ }
+
return method.toTypeSubstitutedMethod(newSignature);
}
- private DexEncodedField renameField(DexEncodedField existing, DexEncodedField field) {
+ private DexEncodedField resolveFieldConflict(
+ DexEncodedField field, Predicate<DexField> availableFieldSignatures) {
DexString oldName = field.field.name;
DexType oldHolder = field.field.clazz;
- DexString newName = getFreshName(oldName.toSourceString(), oldHolder);
- DexField newSignature =
- application.dexItemFactory.createField(target.type, field.field.type, newName);
+
+ DexField newSignature;
+ int count = 1;
+ do {
+ DexString newName = getFreshName(oldName.toSourceString(), count, oldHolder);
+ newSignature =
+ application.dexItemFactory.createField(target.type, field.field.type, newName);
+ count++;
+ } while (!availableFieldSignatures.test(newSignature));
+
DexEncodedField result = field.toTypeSubstitutedField(newSignature);
deferredRenamings.map(field.field, result.field);
return result;
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 5f0ec45..ec54641 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.optimize.Inliner;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.ProguardConfiguration;
import com.android.tools.r8.shaking.ProguardConfigurationRule;
@@ -446,6 +447,7 @@
public boolean placeExceptionalBlocksLast = false;
public boolean dontCreateMarkerInD8 = false;
public boolean forceJumboStringProcessing = false;
+ public Set<Inliner.Reason> validInliningReasons = null;
}
public boolean canUseInvokePolymorphicOnVarHandle() {
diff --git a/src/test/examples/classmerging/ConflictInGeneratedNameTest.java b/src/test/examples/classmerging/ConflictInGeneratedNameTest.java
new file mode 100644
index 0000000..6d566ec
--- /dev/null
+++ b/src/test/examples/classmerging/ConflictInGeneratedNameTest.java
@@ -0,0 +1,113 @@
+// 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 ConflictInGeneratedNameTest {
+ public static void main(String[] args) {
+ B obj = new B();
+ obj.run();
+ }
+
+ public static class A {
+ private String name = "A";
+
+ public A() {
+ print("In A.<init>()");
+ constructor$classmerging$ConflictInGeneratedNameTest$A();
+ }
+
+ private void constructor$classmerging$ConflictInGeneratedNameTest$A() {
+ print("In A.constructor$classmerging$ConflictInGeneratedNameTest$A()");
+ }
+
+ public void printState() {
+ print("In A.printState()");
+ print("A=" + name + "=" + getName());
+ }
+
+ // This method is not overridden.
+ public void foo() {
+ print("In A.foo()");
+ }
+
+ // This method is overridden.
+ public void bar() {
+ print("In A.bar()");
+ }
+
+ // This method is overridden and there is a PUBLIC method in B with the same name
+ // as the direct version of this method in B.
+ public void baz() {
+ print("In A.baz()");
+ }
+
+ // This method is overridden and there is a PRIVATE method in B with the same name
+ // as the direct version of this method in B.
+ public void boo() {
+ print("In A.boo()");
+ }
+
+ // There is a private method in B with the same name as this one.
+ private String getName() {
+ return name;
+ }
+ }
+
+ public static class B extends A {
+ private String name = "B";
+ private String name$classmerging$ConflictInGeneratedNameTest$A = "C";
+
+ public B() {
+ print("In B.<init>()");
+ print("A=" + super.name + "=" + super.getName());
+ print("B=" + name + "=" + getName());
+ print("C=" + name$classmerging$ConflictInGeneratedNameTest$A);
+ }
+
+ public void run() {
+ printState();
+ foo();
+ bar();
+ baz();
+ baz$classmerging$ConflictInGeneratedNameTest$A();
+ boo();
+ boo$classmerging$ConflictInGeneratedNameTest$A();
+ }
+
+ @Override
+ public void bar() {
+ print("In B.bar()");
+ super.bar();
+ }
+
+ @Override
+ public void baz() {
+ print("In B.baz()");
+ super.baz();
+ }
+
+ public void baz$classmerging$ConflictInGeneratedNameTest$A() {
+ print("In B.baz$classmerging$ConflictInGeneratedNameTest$A()");
+ }
+
+ @Override
+ public void boo() {
+ print("In B.boo()");
+ super.boo();
+ }
+
+ private void boo$classmerging$ConflictInGeneratedNameTest$A() {
+ print("In B.boo$classmerging$ConflictInGeneratedNameTest$A()");
+ }
+
+ private String getName() {
+ return name;
+ }
+ }
+
+ public static void print(String message) {
+ System.out.println(message);
+ }
+}
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index 1a20ce3..fa48747 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -7,6 +7,9 @@
-keep public class classmerging.Test {
public static void main(...);
}
+-keep public class classmerging.ConflictInGeneratedNameTest {
+ public static void main(...);
+}
-keep public class classmerging.ConflictingInterfaceSignaturesTest {
public static void main(...);
}
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 8f98fd3..7d200bd 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -15,9 +15,13 @@
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.VmTestRunner;
+import com.android.tools.r8.VmTestRunner.IgnoreForRangeOfVmVersions;
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.ir.optimize.Inliner.Reason;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.smali.SmaliBuilder;
import com.android.tools.r8.utils.AndroidApp;
@@ -37,12 +41,13 @@
import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
import java.util.function.Predicate;
-import org.junit.Ignore;
import org.junit.Test;
+import org.junit.runner.RunWith;
// TODO(christofferqa): Add tests to check that statically typed invocations on method handles
// continue to work after class merging. Rewriting of method handles should be carried out by
// LensCodeRewriter.rewriteDexMethodHandle.
+@RunWith(VmTestRunner.class)
public class ClassMergingTest extends TestBase {
private static final Path CF_DIR =
@@ -108,6 +113,70 @@
}
@Test
+ public void testConflictInGeneratedName() throws Exception {
+ String main = "classmerging.ConflictInGeneratedNameTest";
+ Path[] programFiles =
+ new Path[] {
+ CF_DIR.resolve("ConflictInGeneratedNameTest.class"),
+ CF_DIR.resolve("ConflictInGeneratedNameTest$A.class"),
+ CF_DIR.resolve("ConflictInGeneratedNameTest$B.class")
+ };
+ Set<String> preservedClassNames =
+ ImmutableSet.of(
+ "classmerging.ConflictInGeneratedNameTest",
+ "classmerging.ConflictInGeneratedNameTest$B");
+ DexInspector inspector =
+ runTestOnInput(
+ main,
+ readProgramFiles(programFiles),
+ preservedClassNames::contains,
+ getProguardConfig(EXAMPLE_KEEP),
+ options -> {
+ configure(options);
+ // Avoid that direct methods in B get inlined.
+ options.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE);
+ });
+
+ ClassSubject clazzSubject = inspector.clazz("classmerging.ConflictInGeneratedNameTest$B");
+ assertThat(clazzSubject, isPresent());
+
+ String suffix = "$classmerging$ConflictInGeneratedNameTest$A";
+ List<String> EMPTY = ImmutableList.of();
+
+ // There should be three fields.
+ assertThat(clazzSubject.field("java.lang.String", "name"), isPresent());
+ assertThat(clazzSubject.field("java.lang.String", "name" + suffix), isPresent());
+ assertThat(clazzSubject.field("java.lang.String", "name" + suffix + "2"), isPresent());
+
+ // The direct method "constructor$classmerging$ConflictInGeneratedNameTest$A" is processed after
+ // the method "<init>" is renamed to exactly that name. Therefore the conflict should have been
+ // resolved by appending [suffix] to it.
+ assertThat(clazzSubject.method("void", "constructor" + suffix + suffix, EMPTY), isPresent());
+
+ // There should be two foo's.
+ assertThat(clazzSubject.method("void", "foo", EMPTY), isPresent());
+ assertThat(clazzSubject.method("void", "foo" + suffix, EMPTY), isPresent());
+
+ // There should be two bar's.
+ assertThat(clazzSubject.method("void", "bar", EMPTY), isPresent());
+ assertThat(clazzSubject.method("void", "bar" + suffix, EMPTY), isPresent());
+
+ // There should be three baz's.
+ assertThat(clazzSubject.method("void", "baz", EMPTY), isPresent());
+ assertThat(clazzSubject.method("void", "baz" + suffix, EMPTY), isPresent());
+ assertThat(clazzSubject.method("void", "baz" + suffix + "2", EMPTY), isPresent());
+
+ // There should be three boo's.
+ assertThat(clazzSubject.method("void", "boo", EMPTY), isPresent());
+ assertThat(clazzSubject.method("void", "boo" + suffix, EMPTY), isPresent());
+ assertThat(clazzSubject.method("void", "boo" + suffix + "2", EMPTY), isPresent());
+
+ // There should be two getName's.
+ assertThat(clazzSubject.method("java.lang.String", "getName", EMPTY), isPresent());
+ assertThat(clazzSubject.method("java.lang.String", "getName" + suffix, EMPTY), isPresent());
+ }
+
+ @Test
public void testConflictWasDetected() throws Exception {
runR8(EXAMPLE_KEEP, this::configure);
assertTrue(inspector.clazz("classmerging.ConflictingInterface").isPresent());
@@ -240,8 +309,8 @@
// public void invokeMethodOnE() { "invoke-super E.m()" }
// public void invokeMethodOnF() { "invoke-super F.m()" }
// }
- @Ignore
@Test
+ @IgnoreForRangeOfVmVersions(from = Version.V5_1_1, to = Version.V6_0_1)
public void testSuperCallToMergedClassIsRewritten() throws Exception {
String main = "classmerging.SuperCallToMergedClassIsRewrittenTest";
Set<String> preservedClassNames =
@@ -523,7 +592,17 @@
private DexInspector runTestOnInput(
String main, AndroidApp input, Predicate<String> preservedClassNames, String proguardConfig)
throws Exception {
- AndroidApp output = compileWithR8(input, proguardConfig, this::configure);
+ return runTestOnInput(main, input, preservedClassNames, proguardConfig, this::configure);
+ }
+
+ private DexInspector runTestOnInput(
+ String main,
+ AndroidApp input,
+ Predicate<String> preservedClassNames,
+ String proguardConfig,
+ Consumer<InternalOptions> optionsConsumer)
+ throws Exception {
+ AndroidApp output = compileWithR8(input, proguardConfig, optionsConsumer);
DexInspector inputInspector = new DexInspector(input);
DexInspector outputInspector = new DexInspector(output);
// Check that all classes in [preservedClassNames] are in fact preserved.
