Version 2.1.40
Cherry pick: Add test reproducing lack of signature rewriting when
desugaring
CL: https://r8-review.googlesource.com/52041
Cherry pick: Change GenericSignatureRewriter to use a naming lens
CL: https://r8-review.googlesource.com/52044
Cherry pick: Should not use withLiveness as input to GenericSignatureRewriter
CL: https://r8-review.googlesource.com/52090
Bug: 158124557
Change-Id: I3300ba93b09b514cd2ab19d21d1ddf3c4c6ae9fc
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index c443620..c40d755 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -26,6 +26,7 @@
import com.android.tools.r8.jar.CfApplicationWriter;
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.naming.PrefixRewritingNamingLens;
+import com.android.tools.r8.naming.signature.GenericSignatureRewriter;
import com.android.tools.r8.origin.CommandLineOrigin;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
@@ -256,6 +257,8 @@
hasDexResources
? NamingLens.getIdentityLens()
: PrefixRewritingNamingLens.createPrefixRewritingNamingLens(appView);
+ new GenericSignatureRewriter(appView, namingLens)
+ .run(appView.appInfo().classes(), executor);
} else {
// There are both cf and dex inputs in the program, and rewriting is required for
// desugared library only on cf inputs. We cannot easily rewrite part of the program
@@ -315,6 +318,10 @@
}
DexApplication cfApp = app.builder().replaceProgramClasses(nonDexProgramClasses).build();
ConvertedCfFiles convertedCfFiles = new ConvertedCfFiles();
+ NamingLens prefixRewritingNamingLens =
+ PrefixRewritingNamingLens.createPrefixRewritingNamingLens(appView);
+ new GenericSignatureRewriter(appView, prefixRewritingNamingLens)
+ .run(appView.appInfo().classes(), executor);
new ApplicationWriter(
cfApp,
null,
@@ -322,7 +329,7 @@
null,
GraphLense.getIdentityLense(),
InitClassLens.getDefault(),
- PrefixRewritingNamingLens.createPrefixRewritingNamingLens(appView),
+ prefixRewritingNamingLens,
null,
convertedCfFiles)
.write(executor);
diff --git a/src/main/java/com/android/tools/r8/L8.java b/src/main/java/com/android/tools/r8/L8.java
index 4ba88f4..4bef8cec 100644
--- a/src/main/java/com/android/tools/r8/L8.java
+++ b/src/main/java/com/android/tools/r8/L8.java
@@ -15,7 +15,9 @@
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.desugar.PrefixRewritingMapper;
import com.android.tools.r8.jar.CfApplicationWriter;
+import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.naming.PrefixRewritingNamingLens;
+import com.android.tools.r8.naming.signature.GenericSignatureRewriter;
import com.android.tools.r8.origin.CommandLineOrigin;
import com.android.tools.r8.shaking.AnnotationRemover;
import com.android.tools.r8.shaking.L8TreePruner;
@@ -132,13 +134,16 @@
app = converter.convert(app, executor);
assert appView.appInfo() == appInfo;
+ NamingLens namingLens = PrefixRewritingNamingLens.createPrefixRewritingNamingLens(appView);
+ new GenericSignatureRewriter(appView, namingLens).run(appInfo.classes(), executor);
+
new CfApplicationWriter(
app,
appView,
options,
options.getMarker(Tool.L8),
GraphLense.getIdentityLense(),
- PrefixRewritingNamingLens.createPrefixRewritingNamingLens(appView),
+ namingLens,
null)
.write(options.getClassFileConsumer());
options.printWarnings();
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 622224b..c3ad10a 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -803,12 +803,6 @@
namingLens = new Minifier(appView.withLiveness()).run(executorService, timing);
timing.end();
} else {
- // Rewrite signature annotations for applications that are not minified.
- if (appView.appInfo().hasLiveness()) {
- // TODO(b/124726014): Rewrite signature annotations in lens rewriting instead of here?
- new GenericSignatureRewriter(appView.withLiveness())
- .run(appView.appInfo().classes(), executorService);
- }
namingLens = NamingLens.getIdentityLens();
}
@@ -860,6 +854,12 @@
options.syntheticProguardRulesConsumer.accept(synthesizedProguardRules);
}
+ NamingLens prefixRewritingNamingLens =
+ PrefixRewritingNamingLens.createPrefixRewritingNamingLens(appView, namingLens);
+
+ new GenericSignatureRewriter(appView, prefixRewritingNamingLens)
+ .run(appView.appInfo().classes(), executorService);
+
// Generate the resulting application resources.
writeApplication(
executorService,
@@ -867,7 +867,7 @@
appView,
appView.graphLense(),
appView.initClassLens(),
- PrefixRewritingNamingLens.createPrefixRewritingNamingLens(appView, namingLens),
+ prefixRewritingNamingLens,
options,
ProguardMapSupplier.create(classNameMapper, options));
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 2111c0d..8778485 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.1.39";
+ public static final String LABEL = "2.1.40";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index 0c223b1..9b659a0 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -17,13 +17,11 @@
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.InnerClassAttribute;
-import com.android.tools.r8.naming.signature.GenericSignatureRewriter;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.ProguardPackageNameList;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.InternalOptions.PackageObfuscationMode;
-import com.android.tools.r8.utils.IteratorUtils;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
@@ -146,13 +144,6 @@
}
timing.end();
- timing.begin("rename-generic");
- new GenericSignatureRewriter(appView, renaming)
- .run(
- () -> IteratorUtils.filter(classes.iterator(), DexClass::isProgramClass),
- executorService);
- timing.end();
-
timing.begin("rename-arrays");
appView.dexItemFactory().forAllTypes(this::renameArrayTypeIfNeeded);
timing.end();
diff --git a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
index eef8234..1f66108 100644
--- a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
+++ b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
@@ -13,14 +13,13 @@
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.naming.NamingLens;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.ThreadUtils;
-import com.google.common.collect.Maps;
import java.lang.reflect.GenericSignatureFormatError;
-import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.function.BiConsumer;
@@ -32,23 +31,24 @@
public class GenericSignatureRewriter {
private final AppView<?> appView;
- private final Map<DexType, DexString> renaming;
+ private final NamingLens namingLens;
private final InternalOptions options;
private final Reporter reporter;
- public GenericSignatureRewriter(AppView<?> appView) {
- this(appView, Maps.newIdentityHashMap());
- }
-
- public GenericSignatureRewriter(AppView<?> appView, Map<DexType, DexString> renaming) {
+ public GenericSignatureRewriter(AppView<?> appView, NamingLens namingLens) {
this.appView = appView;
- this.renaming = renaming;
+ this.namingLens = namingLens;
this.options = appView.options();
this.reporter = options.reporter;
}
public void run(Iterable<? extends DexProgramClass> classes, ExecutorService executorService)
throws ExecutionException {
+ // Rewrite signature annotations for applications that are minified or if we have liveness
+ // information, since we could have pruned types.
+ if (namingLens.isIdentityLens() && !appView.appInfo().hasLiveness()) {
+ return;
+ }
// Classes may not be the same as appInfo().classes() if applymapping is used on classpath
// arguments. If that is the case, the ProguardMapMinifier will pass in all classes that is
// either ProgramClass or has a mapping. This is then transitively called inside the
@@ -204,7 +204,7 @@
if (appView.appInfo().hasLiveness() && appView.withLiveness().appInfo().wasPruned(type)) {
type = appView.dexItemFactory().objectType;
}
- DexString renamedDescriptor = renaming.getOrDefault(type, type.descriptor);
+ DexString renamedDescriptor = namingLens.lookupDescriptor(type);
if (parserPosition == ParserPosition.CLASS_SUPER_OR_INTERFACE_ANNOTATION
&& currentClassContext != null) {
// We may have merged the type down to the current class type.
@@ -251,16 +251,14 @@
getClassBinaryNameFromDescriptor(enclosingDescriptor)
+ DescriptorUtils.INNER_CLASS_SEPARATOR
+ name));
- String enclosingRenamedBinaryName =
- getClassBinaryNameFromDescriptor(
- renaming.getOrDefault(enclosingType, enclosingType.descriptor).toString());
type = appView.graphLense().lookupType(type);
- DexString renamedDescriptor = renaming.get(type);
- if (renamedDescriptor != null) {
+ String renamedDescriptor = namingLens.lookupDescriptor(type).toString();
+ if (!renamedDescriptor.equals(type.toDescriptorString())) {
// TODO(b/147504070): If this is a merged class equal to the class context, do not add.
// Pick the renamed inner class from the fully renamed binary name.
- String fullRenamedBinaryName =
- getClassBinaryNameFromDescriptor(renamedDescriptor.toString());
+ String fullRenamedBinaryName = getClassBinaryNameFromDescriptor(renamedDescriptor);
+ String enclosingRenamedBinaryName =
+ getClassBinaryNameFromDescriptor(namingLens.lookupDescriptor(enclosingType).toString());
int innerClassPos = enclosingRenamedBinaryName.length() + 1;
if (innerClassPos < fullRenamedBinaryName.length()) {
renamedSignature.append(fullRenamedBinaryName.substring(innerClassPos));
diff --git a/src/main/java/com/android/tools/r8/relocator/Relocator.java b/src/main/java/com/android/tools/r8/relocator/Relocator.java
index aaf83b1..1e8420e 100644
--- a/src/main/java/com/android/tools/r8/relocator/Relocator.java
+++ b/src/main/java/com/android/tools/r8/relocator/Relocator.java
@@ -87,8 +87,7 @@
SimplePackagesRewritingMapper packageRemapper = new SimplePackagesRewritingMapper(appView);
NamingLens namingLens = packageRemapper.compute(command.getMapping());
- new GenericSignatureRewriter(appView, packageRemapper.getTypeMappings())
- .run(appInfo.classes(), executor);
+ new GenericSignatureRewriter(appView, namingLens).run(appInfo.classes(), executor);
new CfApplicationWriter(
app,
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredGenericSignatureTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredGenericSignatureTest.java
new file mode 100644
index 0000000..ff07b24
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredGenericSignatureTest.java
@@ -0,0 +1,129 @@
+// 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.desugar.desugaredlibrary;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.shaking.ProguardKeepAttributes;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.time.LocalDate;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class DesugaredGenericSignatureTest extends DesugaredLibraryTestBase {
+
+ private final TestParameters parameters;
+ private final boolean shrinkDesugaredLibrary;
+ private static final String EXPECTED = StringUtils.lines("1970", "1", "2");
+
+ @Parameters(name = "{1}, shrinkDesugaredLibrary: {0}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDexRuntimes().withAllApiLevels().build(), BooleanUtils.values());
+ }
+
+ public DesugaredGenericSignatureTest(TestParameters parameters, boolean shrinkDesugaredLibrary) {
+ this.parameters = parameters;
+ this.shrinkDesugaredLibrary = shrinkDesugaredLibrary;
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters);
+ testForD8()
+ .addInnerClasses(DesugaredGenericSignatureTest.class)
+ .setMinApi(parameters.getApiLevel())
+ .enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer)
+ .setIncludeClassesChecksum(true)
+ .compile()
+ .inspect(this::checkRewrittenSignature)
+ .addDesugaredCoreLibraryRunClassPath(
+ this::buildDesugaredLibrary,
+ parameters.getApiLevel(),
+ keepRuleConsumer.get(),
+ shrinkDesugaredLibrary)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ KeepRuleConsumer keepRuleConsumer = createKeepRuleConsumer(parameters);
+ testForR8(parameters.getBackend())
+ .addInnerClasses(DesugaredGenericSignatureTest.class)
+ .addKeepMainRule(Main.class)
+ .addKeepAllClassesRuleWithAllowObfuscation()
+ .addKeepAttributes(
+ ProguardKeepAttributes.SIGNATURE,
+ ProguardKeepAttributes.INNER_CLASSES,
+ ProguardKeepAttributes.ENCLOSING_METHOD)
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .enableCoreLibraryDesugaring(parameters.getApiLevel(), keepRuleConsumer)
+ .compile()
+ .inspect(this::checkRewrittenSignature)
+ .addDesugaredCoreLibraryRunClassPath(
+ this::buildDesugaredLibrary,
+ parameters.getApiLevel(),
+ keepRuleConsumer.get(),
+ shrinkDesugaredLibrary)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutput(EXPECTED);
+ }
+
+ private void checkRewrittenSignature(CodeInspector inspector) {
+ if (!requiresEmulatedInterfaceCoreLibDesugaring(parameters)) {
+ return;
+ }
+ ClassSubject javaTimeBox = inspector.clazz(JavaTimeDateBox.class);
+ assertThat(javaTimeBox, isPresent());
+ ClassSubject box = inspector.clazz(Box.class);
+ assertThat(box, isPresent());
+ String finalBoxDescriptor = box.getFinalDescriptor();
+ assertEquals(
+ "Ljava/lang/Object;"
+ + finalBoxDescriptor.substring(0, finalBoxDescriptor.length() - 1)
+ + "<Lj$/time/LocalDate;>;",
+ javaTimeBox.getFinalSignatureAttribute());
+ }
+
+ public interface Box<T> {
+ T addOne(T t);
+ }
+
+ public static class JavaTimeDateBox implements Box<java.time.LocalDate> {
+
+ @Override
+ @NeverInline
+ public LocalDate addOne(LocalDate localDate) {
+ return localDate.plusDays(1);
+ }
+ }
+
+ public static class Main {
+
+ public static Box<java.time.LocalDate> bar() {
+ return new JavaTimeDateBox();
+ }
+
+ public static void main(String[] args) {
+ LocalDate localDate = bar().addOne(LocalDate.of(1970, 1, 1));
+ System.out.println(localDate.getYear());
+ System.out.println(localDate.getMonthValue());
+ System.out.println(localDate.getDayOfMonth());
+ }
+ }
+}
