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"); - } - } -}