Generalize isSimpleRenaming to check for renamed types in members
Bug: 180092122
Change-Id: Iecb3fdfe2705b92701d23451bb346c4be3c1fda6
diff --git a/src/main/java/com/android/tools/r8/graph/DexReference.java b/src/main/java/com/android/tools/r8/graph/DexReference.java
index 27264bb..e813d7c 100644
--- a/src/main/java/com/android/tools/r8/graph/DexReference.java
+++ b/src/main/java/com/android/tools/r8/graph/DexReference.java
@@ -4,7 +4,9 @@
package com.android.tools.r8.graph;
import com.android.tools.r8.dex.IndexedItemCollection;
+import com.android.tools.r8.errors.Unreachable;
import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
@@ -27,6 +29,22 @@
BiConsumer<DexMethod, T> methodConsumer,
T arg);
+ public static <R extends DexReference, T> T applyPair(
+ R one,
+ R other,
+ BiFunction<DexType, DexType, T> classConsumer,
+ BiFunction<DexField, DexField, T> fieldConsumer,
+ BiFunction<DexMethod, DexMethod, T> methodConsumer) {
+ if (one.isDexType()) {
+ return classConsumer.apply(one.asDexType(), other.asDexType());
+ } else if (one.isDexField()) {
+ return fieldConsumer.apply(one.asDexField(), other.asDexField());
+ } else if (one.isDexMethod()) {
+ return methodConsumer.apply(one.asDexMethod(), other.asDexMethod());
+ }
+ throw new Unreachable();
+ }
+
public abstract void collectIndexedItems(IndexedItemCollection indexedItems);
public abstract int compareTo(DexReference other);
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLens.java b/src/main/java/com/android/tools/r8/graph/GraphLens.java
index a296543..43e733c 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLens.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLens.java
@@ -355,10 +355,12 @@
return newMethod.lookupOnProgramClass(holder);
}
- // Predicate indicating if a rewritten type is a simple renaming, meaning the move from type to
- // rewritten is just a renaming of the type to another. In other words, the content of the
- // definition, including the definition of all of its members is the same modulo the renaming.
- public boolean isSimpleRenaming(DexType from, DexType to) {
+ // Predicate indicating if a rewritten reference is a simple renaming, meaning the move from one
+ // reference to another is simply either just a renaming or/also renaming of the references. In
+ // other words, the content of the definition, including the definition of all of its members is
+ // the same modulo the renaming.
+ public <T extends DexReference> boolean isSimpleRenaming(T from, T to) {
+ assert from != to;
return false;
}
diff --git a/src/main/java/com/android/tools/r8/repackaging/RepackagingLens.java b/src/main/java/com/android/tools/r8/repackaging/RepackagingLens.java
index 0fe63b8..63b8d06 100644
--- a/src/main/java/com/android/tools/r8/repackaging/RepackagingLens.java
+++ b/src/main/java/com/android/tools/r8/repackaging/RepackagingLens.java
@@ -6,10 +6,13 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMember;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLens.NestedGraphLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.IterableUtils;
import com.android.tools.r8.utils.collections.BidirectionalOneToOneHashMap;
import com.android.tools.r8.utils.collections.BidirectionalOneToOneMap;
import com.android.tools.r8.utils.collections.MutableBidirectionalOneToOneMap;
@@ -42,8 +45,36 @@
}
@Override
- public boolean isSimpleRenaming(DexType from, DexType to) {
- return originalTypes.get(to) == from || super.isSimpleRenaming(from, to);
+ public <T extends DexReference> boolean isSimpleRenaming(T from, T to) {
+ if (from == to) {
+ assert false : "The from and to references should not be equal";
+ return false;
+ }
+ if (super.isSimpleRenaming(from, to)) {
+ // Repackaging only move classes and therefore if a previous lens has a simple renaming it
+ // will be maintained here.
+ return true;
+ }
+ return DexReference.applyPair(
+ from,
+ to,
+ this::isSimpleTypeRenamingOrEqual,
+ this::isSimpleTypeRenamingOrEqual,
+ this::isSimpleTypeRenamingOrEqual);
+ }
+
+ private boolean isSimpleTypeRenamingOrEqual(DexType from, DexType to) {
+ return from == to || originalTypes.get(to) == from;
+ }
+
+ private boolean isSimpleTypeRenamingOrEqual(DexMember<?, ?> from, DexMember<?, ?> to) {
+ if (!isSimpleTypeRenamingOrEqual(from.getHolderType(), to.getHolderType())) {
+ return false;
+ }
+ return IterableUtils.testPairs(
+ this::isSimpleTypeRenamingOrEqual,
+ from.getReferencedBaseTypes(dexItemFactory),
+ to.getReferencedBaseTypes(dexItemFactory));
}
public static class Builder {
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java
index fd63ea2..dfc240b 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java
@@ -57,7 +57,7 @@
DexMethod rewritten = lens.lookupMethod(method);
// If the reference has been non-trivially rewritten the compiler has changed it and it can no
// longer be considered a synthetic. The context may or may not have changed.
- if (method != rewritten && !lens.isSimpleRenaming(method.holder, rewritten.holder)) {
+ if (method != rewritten && !lens.isSimpleRenaming(method, rewritten)) {
// If the referenced item is rewritten, it should be moved to another holder as the
// synthetic holder is no longer part of the synthetic collection.
assert method.holder != rewritten.holder : "The synthetic method reference should have moved";
diff --git a/src/main/java/com/android/tools/r8/utils/IterableUtils.java b/src/main/java/com/android/tools/r8/utils/IterableUtils.java
index 2b0267d..ce8396f 100644
--- a/src/main/java/com/android/tools/r8/utils/IterableUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/IterableUtils.java
@@ -8,8 +8,10 @@
import com.google.common.collect.Iterators;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Iterator;
import java.util.List;
import java.util.function.BiFunction;
+import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -130,4 +132,23 @@
return iterable;
}
}
+
+ /**
+ * Utility method for testing the the elements in one and other pair-wise. Returns false if the
+ * lengths differ.
+ */
+ public static <T> boolean testPairs(
+ BiPredicate<T, T> predicate, Iterable<T> one, Iterable<T> other) {
+ Iterator<T> iterator = other.iterator();
+ for (T first : one) {
+ if (!iterator.hasNext()) {
+ return false;
+ }
+ T second = iterator.next();
+ if (!predicate.test(first, second)) {
+ return false;
+ }
+ }
+ return !iterator.hasNext();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageParameterSyntheticOutlineTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageParameterSyntheticOutlineTest.java
index 0636a50..9e2f6c2 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageParameterSyntheticOutlineTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageParameterSyntheticOutlineTest.java
@@ -4,11 +4,8 @@
package com.android.tools.r8.repackage;
-import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
import static com.android.tools.r8.shaking.ProguardConfigurationParser.REPACKAGE_CLASSES;
-import static org.hamcrest.CoreMatchers.containsString;
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.utils.DescriptorUtils;
@@ -49,7 +46,7 @@
.assertSuccessWithOutputLines(EXPECTED);
}
- @Test(expected = CompilationFailedException.class)
+ @Test
public void testR8() throws Exception {
testForR8(parameters.getBackend())
.addProgramClasses(Param.class, Return.class)
@@ -67,13 +64,8 @@
})
.apply(this::configureRepackaging)
.addKeepPackageNamesRule("bar**")
- .compileWithExpectedDiagnostics(
- diagnostics -> {
- // TODO(b/180092122): This should not fail.
- diagnostics.assertErrorsMatch(
- diagnosticMessage(
- containsString("The synthetic method reference should have moved")));
- });
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines(EXPECTED);
}
private byte[] rewrittenPackageForClassWithCodeToBeOutlined() throws Exception {
