Handle already desugared input for (some part of) library desugaring
Bug: 171867367
Bug: 172433489
Change-Id: I5e88c86e90749e36a6fb9bc7c46302db002d7fcb
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index 70db016..1e7ec9c 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -94,6 +94,10 @@
private Map<DexClass, DexValueString> sourceDebugExtensions = new IdentityHashMap<>();
+ // When input has been (partially) desugared these are the classes which has been library
+ // desugared. This information is populated in the IR converter.
+ private Set<DexProgramClass> alreadyLibraryDesugared = null;
+
private AppView(
T appInfo,
WholeProgramOptimizations wholeProgramOptimizations,
@@ -663,4 +667,17 @@
}
});
}
+
+ public void setAlreadyLibraryDesugared(Set<DexProgramClass> alreadyLibraryDesugared) {
+ assert this.alreadyLibraryDesugared == null;
+ this.alreadyLibraryDesugared = alreadyLibraryDesugared;
+ }
+
+ public boolean isAlreadyLibraryDesugared(DexProgramClass clazz) {
+ if (!options().desugarSpecificOptions().allowAllDesugaredInput) {
+ return false;
+ }
+ assert alreadyLibraryDesugared != null;
+ return alreadyLibraryDesugared.contains(clazz);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index ec95001..0974715 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -107,10 +107,12 @@
import com.android.tools.r8.utils.collections.SortedProgramMethodSet;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -480,7 +482,33 @@
executor, OptimizationFeedbackIgnore.getInstance());
DexApplication application = appView.appInfo().app();
timing.begin("IR conversion");
- ThreadUtils.processItems(application.classes(), this::convertMethods, executor);
+
+ if (appView.options().desugarSpecificOptions().allowAllDesugaredInput) {
+ // Classes which has already been through library desugaring will not go through IR
+ // processing again.
+ LibraryDesugaredChecker libraryDesugaredChecker = new LibraryDesugaredChecker(appView);
+ Set<DexProgramClass> alreadyLibraryDesugared = Sets.newConcurrentHashSet();
+ ThreadUtils.processItems(
+ application.classes(),
+ clazz -> {
+ if (libraryDesugaredChecker.isClassLibraryDesugared(clazz)) {
+ if (appView.options().desugarSpecificOptions().allowAllDesugaredInput) {
+ alreadyLibraryDesugared.add(clazz);
+ } else {
+ throw new CompilationError(
+ "Code for "
+ + clazz.getType().getDescriptor()
+ + "has already been library desugared.");
+ }
+ } else {
+ convertMethods(clazz);
+ }
+ },
+ executor);
+ appView.setAlreadyLibraryDesugared(alreadyLibraryDesugared);
+ } else {
+ ThreadUtils.processItems(application.classes(), this::convertMethods, executor);
+ }
// Build a new application with jumbo string info,
Builder<?> builder = application.builder();
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LibraryDesugaredChecker.java b/src/main/java/com/android/tools/r8/ir/conversion/LibraryDesugaredChecker.java
new file mode 100644
index 0000000..7707d9e
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LibraryDesugaredChecker.java
@@ -0,0 +1,183 @@
+// 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.ir.conversion;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexAnnotation;
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.DexValue;
+import com.android.tools.r8.graph.DexValue.DexValueArray;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.UseRegistry;
+
+public class LibraryDesugaredChecker {
+ private final AppView<?> appView;
+ private final DexString jDollarDescriptorPrefix;
+
+ LibraryDesugaredChecker(AppView<?> appView) {
+ this.appView = appView;
+ this.jDollarDescriptorPrefix = appView.dexItemFactory().createString("Lj$/");
+ }
+
+ public boolean isClassLibraryDesugared(DexProgramClass clazz) {
+ IsLibraryDesugaredTracer tracer =
+ new IsLibraryDesugaredTracer(appView, jDollarDescriptorPrefix, clazz);
+ tracer.run();
+ return tracer.isLibraryDesugared();
+ }
+
+ private static class IsLibraryDesugaredTracer extends UseRegistry {
+
+ private final DexString jDollarDescriptorPrefix;
+ private final AppView<?> appView;
+ private final DexProgramClass clazz;
+ private boolean isLibraryDesugared = false;
+
+ public IsLibraryDesugaredTracer(
+ AppView<?> appView, DexString jDollarDescriptorPrefix, DexProgramClass clazz) {
+ super(appView.dexItemFactory());
+ this.jDollarDescriptorPrefix = jDollarDescriptorPrefix;
+ this.appView = appView;
+ this.clazz = clazz;
+ }
+
+ public void run() {
+ registerClass(clazz);
+ }
+
+ public boolean isLibraryDesugared() {
+ return isLibraryDesugared;
+ }
+
+ private void registerClass(DexProgramClass clazz) {
+ if (clazz.superType != null) {
+ registerTypeReference(clazz.superType);
+ }
+ for (DexType implementsType : clazz.interfaces.values) {
+ registerTypeReference(implementsType);
+ }
+ if (isLibraryDesugared) {
+ return;
+ }
+ for (DexEncodedMethod method : clazz.methods()) {
+ registerMethod(new ProgramMethod(clazz, method));
+ if (isLibraryDesugared) {
+ return;
+ }
+ }
+ clazz.forEachField(this::registerField);
+ }
+
+ private void registerType(DexType type) {
+ isLibraryDesugared =
+ isLibraryDesugared || type.descriptor.startsWith(jDollarDescriptorPrefix);
+ }
+
+ private void registerField(DexField field) {
+ registerType(field.getHolderType());
+ registerType(field.getType());
+ }
+
+ private void registerMethod(DexMethod method) {
+ for (DexType type : method.getParameters().values) {
+ registerTypeReference(type);
+ }
+ registerTypeReference(method.getReturnType());
+ }
+
+ private void registerField(DexEncodedField field) {
+ registerField(field.getReference());
+ }
+
+ private void registerMethod(ProgramMethod method) {
+ registerMethod(method.getReference());
+ for (DexAnnotation annotation : method.getDefinition().annotations().annotations) {
+ if (annotation.annotation.type == appView.dexItemFactory().annotationThrows) {
+ DexValueArray dexValues = annotation.annotation.elements[0].value.asDexValueArray();
+ for (DexValue dexValType : dexValues.getValues()) {
+ registerType(dexValType.asDexValueType().value);
+ }
+ }
+ }
+ method.registerCodeReferences(this);
+ }
+
+ @Override
+ public void registerInitClass(DexType type) {
+ registerType(type);
+ }
+
+ @Override
+ public void registerInvokeVirtual(DexMethod method) {
+ registerMethod(method);
+ }
+
+ @Override
+ public void registerInvokeDirect(DexMethod method) {
+ registerMethod(method);
+ }
+
+ @Override
+ public void registerInvokeStatic(DexMethod method) {
+ registerMethod(method);
+ }
+
+ @Override
+ public void registerInvokeInterface(DexMethod method) {
+ registerMethod(method);
+ }
+
+ @Override
+ public void registerInvokeStatic(DexMethod method, boolean itf) {
+ registerMethod(method);
+ }
+
+ @Override
+ public void registerInvokeSuper(DexMethod method) {
+ registerMethod(method);
+ }
+
+ @Override
+ public void registerInstanceFieldRead(DexField field) {
+ registerField(field);
+ }
+
+ @Override
+ public void registerInstanceFieldWrite(DexField field) {
+ registerField(field);
+ }
+
+ @Override
+ public void registerNewInstance(DexType type) {
+ registerType(type);
+ }
+
+ @Override
+ public void registerStaticFieldRead(DexField field) {
+ registerField(field);
+ }
+
+ @Override
+ public void registerStaticFieldWrite(DexField field) {
+ registerField(field);
+ }
+
+ @Override
+ public void registerTypeReference(DexType type) {
+ registerType(type);
+ }
+
+ @Override
+ public void registerInstanceOf(DexType type) {
+ registerType(type);
+ }
+ }
+}
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 0884ab5..a61f62e 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
@@ -437,7 +437,7 @@
iface -> {
for (int i = 0; i < extraInterfaceSignatures.size(); i++) {
if (extraInterfaceSignatures.get(i).type() == iface) {
- if (!appView.options().desugarSpecificOptions().allowDesugaredInput) {
+ if (!appView.options().desugarSpecificOptions().allowAllDesugaredInput) {
throw new CompilationError(
"Code has already been library desugared. Interface "
+ iface.getDescriptor()
@@ -476,6 +476,9 @@
ClassInfo superInfo,
MethodSignatures signatures,
Builder<DexEncodedMethod> additionalForwards) {
+ if (clazz.isProgramClass() && appView.isAlreadyLibraryDesugared(clazz.asProgramClass())) {
+ return;
+ }
for (Wrapper<DexMethod> wrapper : signatures.signatures) {
resolveForwardForSignature(
clazz,
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryRetargeter.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryRetargeter.java
index 214c17a..e04505d 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryRetargeter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryRetargeter.java
@@ -446,6 +446,9 @@
}
SortedProgramMethodSet addedMethods = SortedProgramMethodSet.create();
for (DexProgramClass clazz : appView.appInfo().classes()) {
+ if (appView.isAlreadyLibraryDesugared(clazz)) {
+ continue;
+ }
if (clazz.superType == null) {
assert clazz.type == appView.dexItemFactory().objectType : clazz.type.toSourceString();
continue;
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
index ddcb3c7..862cb47 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceMethodRewriter.java
@@ -1022,6 +1022,9 @@
// results, since each class implementing an emulated interface should also implement the
// rewritten one.
private void transformEmulatedInterfaces(DexProgramClass clazz) {
+ if (appView.isAlreadyLibraryDesugared(clazz)) {
+ return;
+ }
List<GenericSignature.ClassTypeSignature> newInterfaces = new ArrayList<>();
GenericSignature.ClassSignature classSignature = clazz.getClassSignature();
for (int i = 0; i < clazz.interfaces.size(); i++) {
@@ -1074,6 +1077,9 @@
// First we compute all desugaring *without* introducing forwarding methods.
for (DexProgramClass clazz : builder.getProgramClasses()) {
if (shouldProcess(clazz, flavour, false)) {
+ if (appView.isAlreadyLibraryDesugared(clazz)) {
+ continue;
+ }
processor.processClass(clazz);
}
}
diff --git a/src/main/java/com/android/tools/r8/jar/CfApplicationWriter.java b/src/main/java/com/android/tools/r8/jar/CfApplicationWriter.java
index 97ef2dc..58c9f2d 100644
--- a/src/main/java/com/android/tools/r8/jar/CfApplicationWriter.java
+++ b/src/main/java/com/android/tools/r8/jar/CfApplicationWriter.java
@@ -361,6 +361,13 @@
LensCodeRewriterUtils rewriter,
ClassWriter writer,
ImmutableMap<DexString, DexValue> defaults) {
+ NamingLens namingLens = this.namingLens;
+
+ // For "pass through" classes which has already been library desugared use the identity lens.
+ if (appView.isAlreadyLibraryDesugared(method.getHolder())) {
+ namingLens = NamingLens.getIdentityLens();
+ }
+
DexEncodedMethod definition = method.getDefinition();
int access = definition.getAccessFlags().getAsCfAccessFlags();
if (definition.isDeprecated()) {
@@ -383,7 +390,7 @@
writeAnnotations(visitor::visitAnnotation, definition.annotations().annotations);
writeParameterAnnotations(visitor, definition.parameterAnnotationsList);
if (!definition.shouldNotHaveCode()) {
- writeCode(method, classFileVersion, rewriter, visitor);
+ writeCode(method, classFileVersion, namingLens, rewriter, visitor);
}
visitor.visitEnd();
}
@@ -520,6 +527,7 @@
private void writeCode(
ProgramMethod method,
CfVersion classFileVersion,
+ NamingLens namingLens,
LensCodeRewriterUtils rewriter,
MethodVisitor visitor) {
CfCode code = method.getDefinition().getCode().asCfCode();
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 052b249..47c084e 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1184,8 +1184,11 @@
// b/172508621
public boolean sortMethodsOnCfOutput =
System.getProperty("com.android.tools.r8.sortMethodsOnCfWriting") != null;
- public boolean allowDesugaredInput =
- System.getProperty("com.android.tools.r8.allowDesugaredInput") != null;
+ // Desugaring is not fully idempotent. With this option turned on all desugared input is
+ // allowed, and if it is detected that the desugared input cannot be reprocessed, that input
+ // will be passed-through without the problematic rewritings applied.
+ public boolean allowAllDesugaredInput =
+ System.getProperty("com.android.tools.r8.allowAllDesugaredInput") != null;
}
public static class CallSiteOptimizationOptions {
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/IteratorTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/IteratorTest.java
index 2a968a0..46cb2bd 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/IteratorTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/IteratorTest.java
@@ -112,7 +112,9 @@
diagnosticMessage(
containsString(
"Code has already been library desugared. "
- + "Interface Lj$/util/Iterator; is already implemented"))));
+ + "Interface Lj$/util/Iterator; is already implemented by "
+ + "Lcom/android/tools/r8/desugar/desugaredlibrary/"
+ + "IteratorTest$MyIterator;"))));
fail("Expected failure");
} catch (CompilationFailedException e) {
// Expected.
@@ -124,7 +126,7 @@
testForD8(Backend.CF)
.addOptionsModification(
options ->
- options.desugarSpecificOptions().allowDesugaredInput =
+ options.desugarSpecificOptions().allowAllDesugaredInput =
parameters.getApiLevel().isLessThan(apiLevelNotRequiringDesugaring))
.setMinApi(parameters.getApiLevel())
.addProgramFiles(firstJar)
@@ -141,9 +143,8 @@
assertEquals(
canUseDefaultAndStaticInterfaceMethods ? 0 : 1,
info.getInterfaces().stream().filter(name -> name.equals("j$/util/Iterator")).count());
- // TODO(b/171867367): This should only be 2.
assertEquals(
- canUseDefaultAndStaticInterfaceMethods ? 1 : 3,
+ canUseDefaultAndStaticInterfaceMethods ? 1 : 2,
info.getMethodNames().stream().filter(name -> name.equals("forEachRemaining")).count());
if (parameters.getRuntime().isDex()) {
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/CallBackConversionTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/CallBackConversionTest.java
index 210216f..cb8c24f 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/CallBackConversionTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/CallBackConversionTest.java
@@ -127,6 +127,8 @@
// Use D8 to desugar with Java classfile output.
Path secondJar =
testForD8(Backend.CF)
+ .addOptionsModification(
+ options -> options.desugarSpecificOptions().allowAllDesugaredInput = true)
.setMinApi(parameters.getApiLevel())
.addProgramFiles(firstJar)
.addLibraryClasses(CustomLibClass.class)
@@ -141,8 +143,7 @@
assertEquals(
Impl.class.getTypeName(),
DescriptorUtils.getJavaTypeFromBinaryName(info.getClassBinaryName()));
- // TODO(b/171867367): This should only be 2.
- assertEquals(3, info.getMethodNames().stream().filter(name -> name.equals("foo")).count());
+ assertEquals(2, info.getMethodNames().stream().filter(name -> name.equals("foo")).count());
// Convert to DEX without desugaring and run.
testForD8()
