Desugared library fix LinkedHashSet subclasses
- When looking up emulated interface in the
class processor to generate virtual overrides
retarget core lib members should be considered
if they overlap.
Bug: 142846107
Change-Id: I0abc1f4c4f5fe1143ad3ae41de0bbe773bd593c5
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
index d81f3fe..688e751 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.ir.code.Invoke;
@@ -114,7 +115,12 @@
DexClass target = appView.definitionFor(method.holder);
// NOTE: Never add a forwarding method to methods of classes unknown or coming from android.jar
// even if this results in invalid code, these classes are never desugared.
- assert target != null && !rewriter.isNonDesugaredLibraryClass(target);
+ assert target != null;
+ // In desugared library, emulated interface methods can be overridden by retarget lib members.
+ DexMethod forwardMethod =
+ target.isInterface()
+ ? rewriter.defaultAsMethodOfCompanionClass(method)
+ : retargetMethod(method);
// New method will have the same name, proto, and also all the flags of the
// default method, including bridge flag.
DexMethod newMethod = dexItemFactory.createMethod(clazz.type, method.proto, method.name);
@@ -125,7 +131,7 @@
ForwardMethodSourceCode.builder(newMethod);
forwardSourceCodeBuilder
.setReceiver(clazz.type)
- .setTarget(rewriter.defaultAsMethodOfCompanionClass(method))
+ .setTarget(forwardMethod)
.setInvokeType(Invoke.Type.STATIC)
.setIsInterface(false); // Holder is companion class, not an interface.
return new DexEncodedMethod(
@@ -136,6 +142,18 @@
new SynthesizedCode(forwardSourceCodeBuilder::build));
}
+ private DexMethod retargetMethod(DexMethod method) {
+ Map<DexString, Map<DexType, DexType>> retargetCoreLibMember =
+ appView.options().desugaredLibraryConfiguration.getRetargetCoreLibMember();
+ Map<DexType, DexType> typeMap = retargetCoreLibMember.get(method.name);
+ assert typeMap != null;
+ assert typeMap.get(method.holder) != null;
+ return dexItemFactory.createMethod(
+ typeMap.get(method.holder),
+ dexItemFactory.prependTypeToProto(method.holder, method.proto),
+ method.name);
+ }
+
// For a given class `clazz` inspects all interfaces it implements directly or
// indirectly and collect a set of all default methods to be implemented
// in this class.
@@ -215,17 +233,29 @@
// Remove from candidates methods defined in class or any of its superclasses.
List<DexEncodedMethod> toBeImplemented = new ArrayList<>(candidates.size());
current = clazz;
+ Map<DexString, Map<DexType, DexType>> retargetCoreLibMember =
+ appView.options().desugaredLibraryConfiguration.getRetargetCoreLibMember();
while (true) {
// In desugared library look-up, methods from library classes cannot hide methods from
// emulated interfaces (the method being desugared implied the implementation is not
- // present in the library class).
+ // present in the library class), except through retarget core lib member.
if (desugaredLibraryLookup && current.isLibraryClass()) {
Iterator<DexEncodedMethod> iterator = candidates.iterator();
while (iterator.hasNext()) {
DexEncodedMethod candidate = iterator.next();
- if (rewriter.isEmulatedInterface(candidate.method.holder)) {
- toBeImplemented.add(candidate);
- iterator.remove();
+ if (rewriter.isEmulatedInterface(candidate.method.holder)
+ && current.lookupVirtualMethod(candidate.method) != null) {
+ // A library class overrides an emulated interface method. This override is valid
+ // only if it goes through retarget core lib member, else it needs to be implemented.
+ Map<DexType, DexType> typeMap = retargetCoreLibMember.get(candidate.method.name);
+ if (typeMap != null && typeMap.containsKey(current.type)) {
+ // A rewrite needs to be performed, but instead of rewriting to the companion class,
+ // D8/R8 needs to rewrite to the retarget member.
+ toBeImplemented.add(current.lookupVirtualMethod(candidate.method));
+ } else {
+ toBeImplemented.add(candidate);
+ iterator.remove();
+ }
}
}
}
diff --git a/src/test/java/com/android/tools/r8/desugar/corelib/LinkedHashSetTest.java b/src/test/java/com/android/tools/r8/desugar/corelib/LinkedHashSetTest.java
new file mode 100644
index 0000000..178af61
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/corelib/LinkedHashSetTest.java
@@ -0,0 +1,120 @@
+// Copyright (c) 2019, 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.desugar.corelib;
+
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import java.util.LinkedHashSet;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class LinkedHashSetTest extends CoreLibDesugarTestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDexRuntimes().withAllApiLevels().build();
+ }
+
+ public LinkedHashSetTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testLinkedHashSetOverrides() throws Exception {
+ String stdOut =
+ testForD8()
+ .addInnerClasses(LinkedHashSetTest.class)
+ .setMinApi(parameters.getApiLevel())
+ .enableCoreLibraryDesugaring(parameters.getApiLevel())
+ .compile()
+ .addDesugaredCoreLibraryRunClassPath(
+ this::buildDesugaredLibrary, parameters.getApiLevel())
+ .run(parameters.getRuntime(), Executor.class)
+ .assertSuccess()
+ .getStdOut();
+ assertLines2By2Correct(stdOut);
+ }
+
+ static class Executor {
+ @SuppressWarnings("RedundantOperationOnEmptyContainer")
+ public static void main(String[] args) {
+ Spliterator<String> spliterator;
+
+ // Spliterator of Set is only distinct.
+ // Spliterator of LinkedHashSet is distinct and ordered.
+ // Spliterator of CustomLinkedHashSetOverride is distinct, ordered and immutable.
+ // If an incorrect method is found, characteristics are incorrect.
+
+ spliterator = new LinkedHashSet<String>().spliterator();
+ System.out.println(spliterator.hasCharacteristics(Spliterator.DISTINCT));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.ORDERED));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.IMMUTABLE));
+ System.out.println("false");
+
+ spliterator = new CustomLinkedHashSetOverride<String>().spliterator();
+ System.out.println(spliterator.hasCharacteristics(Spliterator.DISTINCT));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.ORDERED));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.IMMUTABLE));
+ System.out.println("true");
+
+ spliterator = new CustomLinkedHashSetNoOverride<String>().spliterator();
+ System.out.println(spliterator.hasCharacteristics(Spliterator.DISTINCT));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.ORDERED));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.IMMUTABLE));
+ System.out.println("false");
+
+ spliterator = new CustomLinkedHashSetOverride<String>().superSpliterator();
+ System.out.println(spliterator.hasCharacteristics(Spliterator.DISTINCT));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.ORDERED));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.IMMUTABLE));
+ System.out.println("false");
+
+ spliterator = new SubclassOverride<String>().superSpliterator();
+ System.out.println(spliterator.hasCharacteristics(Spliterator.DISTINCT));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.ORDERED));
+ System.out.println("true");
+ System.out.println(spliterator.hasCharacteristics(Spliterator.IMMUTABLE));
+ System.out.println("true");
+ }
+ }
+
+ static class CustomLinkedHashSetOverride<E> extends LinkedHashSet<E> {
+
+ @Override
+ public Spliterator<E> spliterator() {
+ return Spliterators.spliterator(
+ this, Spliterator.DISTINCT | Spliterator.ORDERED | Spliterator.IMMUTABLE);
+ }
+
+ public Spliterator<E> superSpliterator() {
+ return super.spliterator();
+ }
+ }
+
+ static class SubclassOverride<E> extends CustomLinkedHashSetOverride<E> {
+ @Override
+ public Spliterator<E> superSpliterator() {
+ return super.spliterator();
+ }
+ }
+
+ @SuppressWarnings("WeakerAccess")
+ static class CustomLinkedHashSetNoOverride<E> extends LinkedHashSet<E> {}
+}
