Version 2.0.53
Revert: Handle collisions during member rebinding
CL: https://r8-review.googlesource.com/49769
Bug: 151480842
Reason for revert: Test failures
Change-Id: Ie8247e279a082983734b1e9baa696546837784f1
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index d2b02d19..5c99d10 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 = "2.0.52";
+ public static final String LABEL = "2.0.53";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index 68c3b19..09a9055 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -21,6 +21,7 @@
import java.util.ArrayDeque;
import java.util.Collections;
import java.util.Deque;
+import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.LinkedList;
import java.util.List;
@@ -1013,7 +1014,7 @@
@Override
public Set<DexMethod> lookupMethodInAllContexts(DexMethod method) {
- Set<DexMethod> result = Sets.newIdentityHashSet();
+ Set<DexMethod> result = new HashSet<>();
for (DexMethod previous : previousLense.lookupMethodInAllContexts(method)) {
result.add(methodMap.getOrDefault(previous, previous));
}
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
index 8163e4c..4d20f6b 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingAnalysis.java
@@ -166,7 +166,7 @@
method, target, originalClass, targetClass, lookupTarget);
}
}
- builder.map(method, lense.lookupMethod(validTargetFor(target.method, method)), invokeType);
+ builder.map(method, lense.lookupMethod(validTargetFor(target.method, method)));
}
}
}
diff --git a/src/main/java/com/android/tools/r8/optimize/MemberRebindingLense.java b/src/main/java/com/android/tools/r8/optimize/MemberRebindingLense.java
index 95e2772..a845de2 100644
--- a/src/main/java/com/android/tools/r8/optimize/MemberRebindingLense.java
+++ b/src/main/java/com/android/tools/r8/optimize/MemberRebindingLense.java
@@ -9,72 +9,45 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
-import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.Invoke.Type;
import com.google.common.collect.ImmutableMap;
-import java.util.Collections;
-import java.util.IdentityHashMap;
import java.util.Map;
-import java.util.Set;
public class MemberRebindingLense extends NestedGraphLense {
- public static class Builder {
+ public static class Builder extends NestedGraphLense.Builder {
private final AppView<?> appView;
- private final Map<DexField, DexField> fieldMap = new IdentityHashMap<>();
- private final Map<Invoke.Type, Map<DexMethod, DexMethod>> methodMaps = new IdentityHashMap<>();
-
protected Builder(AppView<?> appView) {
this.appView = appView;
}
- public void map(DexField from, DexField to) {
- if (from == to) {
- assert !fieldMap.containsKey(from);
- return;
- }
- fieldMap.put(from, to);
- }
-
- public void map(DexMethod from, DexMethod to, Invoke.Type type) {
- if (from == to) {
- assert !methodMaps.containsKey(type) || methodMaps.get(type).getOrDefault(from, to) == to;
- return;
- }
- Map<DexMethod, DexMethod> methodMap =
- methodMaps.computeIfAbsent(type, ignore -> new IdentityHashMap<>());
- assert methodMap.getOrDefault(from, to) == to;
- methodMap.put(from, to);
- }
-
public GraphLense build(GraphLense previousLense) {
- if (fieldMap.isEmpty() && methodMaps.isEmpty()) {
+ assert typeMap.isEmpty();
+ if (methodMap.isEmpty() && fieldMap.isEmpty()) {
return previousLense;
}
- return new MemberRebindingLense(appView, methodMaps, fieldMap, previousLense);
+ return new MemberRebindingLense(appView, methodMap, fieldMap, previousLense);
}
}
private final AppView<?> appView;
- private final Map<Invoke.Type, Map<DexMethod, DexMethod>> methodMaps;
public MemberRebindingLense(
AppView<?> appView,
- Map<Invoke.Type, Map<DexMethod, DexMethod>> methodMaps,
+ Map<DexMethod, DexMethod> methodMap,
Map<DexField, DexField> fieldMap,
GraphLense previousLense) {
super(
ImmutableMap.of(),
- ImmutableMap.of(),
+ methodMap,
fieldMap,
null,
null,
previousLense,
appView.dexItemFactory());
this.appView = appView;
- this.methodMaps = methodMaps;
}
public static Builder builder(AppView<?> appView) {
@@ -82,28 +55,6 @@
}
@Override
- public boolean isLegitimateToHaveEmptyMappings() {
- return true;
- }
-
- @Override
- public GraphLenseLookupResult lookupMethod(DexMethod method, DexMethod context, Type type) {
- GraphLenseLookupResult previous = previousLense.lookupMethod(method, context, type);
- Map<DexMethod, DexMethod> methodMap = methodMaps.getOrDefault(type, Collections.emptyMap());
- DexMethod newMethod = methodMap.get(previous.getMethod());
- if (newMethod != null) {
- return new GraphLenseLookupResult(
- newMethod, mapInvocationType(newMethod, method, previous.getType()));
- }
- return previous;
- }
-
- @Override
- public Set<DexMethod> lookupMethodInAllContexts(DexMethod method) {
- return previousLense.lookupMethodInAllContexts(method);
- }
-
- @Override
protected Type mapInvocationType(DexMethod newMethod, DexMethod originalMethod, Type type) {
return super.mapVirtualInterfaceInvocationTypes(appView, newMethod, originalMethod, type);
}
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingConflictTest.java b/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingConflictTest.java
deleted file mode 100644
index b1ae543..0000000
--- a/src/test/java/com/android/tools/r8/memberrebinding/MemberRebindingConflictTest.java
+++ /dev/null
@@ -1,88 +0,0 @@
-// Copyright (c) 2020, 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.memberrebinding;
-
-import com.android.tools.r8.NeverInline;
-import com.android.tools.r8.NeverMerge;
-import com.android.tools.r8.TestBase;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.graph.MethodAccessFlags;
-import com.android.tools.r8.memberrebinding.testclasses.MemberRebindingConflictTestClasses;
-import com.android.tools.r8.memberrebinding.testclasses.MemberRebindingConflictTestClasses.C;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class MemberRebindingConflictTest extends TestBase {
-
- private final TestParameters parameters;
-
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimesAndApiLevels().build();
- }
-
- public MemberRebindingConflictTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
- @Test
- public void test() throws Exception {
- testForR8(parameters.getBackend())
- .addProgramClasses(A.class, TestClass.class)
- .addProgramClassFileData(
- transformer(B.class)
- .removeMethods(
- (access, name, descriptor, signature, exceptions) -> {
- if (name.equals("foo")) {
- assert MethodAccessFlags.fromCfAccessFlags(access, false).isSynthetic();
- return true;
- }
- return false;
- })
- .transform())
- .addInnerClasses(MemberRebindingConflictTestClasses.class)
- .addKeepMainRule(TestClass.class)
- .enableInliningAnnotations()
- .enableMergeAnnotations()
- .enableNeverClassInliningAnnotations()
- .setMinApi(parameters.getApiLevel())
- .compile()
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutputLines("foo", "bar", "foo", "baz");
- }
-
- static class TestClass {
-
- public static void main(String[] args) {
- C c = new C();
- c.bar();
- c.baz();
- }
- }
-
- @NeverMerge
- static class A {
-
- @NeverInline
- public void foo() {
- System.out.println("foo");
- }
- }
-
- @NeverMerge
- public static class B extends A {
-
- // public synthetic void foo() { super.foo(); }
-
- @NeverInline
- public void bar() {
- foo();
- System.out.println("bar");
- }
- }
-}
diff --git a/src/test/java/com/android/tools/r8/memberrebinding/testclasses/MemberRebindingConflictTestClasses.java b/src/test/java/com/android/tools/r8/memberrebinding/testclasses/MemberRebindingConflictTestClasses.java
deleted file mode 100644
index 79f7dd6..0000000
--- a/src/test/java/com/android/tools/r8/memberrebinding/testclasses/MemberRebindingConflictTestClasses.java
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright (c) 2020, 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.memberrebinding.testclasses;
-
-import com.android.tools.r8.NeverClassInline;
-import com.android.tools.r8.NeverInline;
-import com.android.tools.r8.memberrebinding.MemberRebindingConflictTest;
-
-public class MemberRebindingConflictTestClasses {
-
- @NeverClassInline
- public static class C extends MemberRebindingConflictTest.B {
-
- @NeverInline
- public void baz() {
- super.foo();
- System.out.println("baz");
- }
- }
-}
