Change GenericSignatureRewriter to use a naming lens
Bug: 158124557
Change-Id: Ic52380ad05a952188fd2618e6a8dae93c92ce53c
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index c443620..9b401a9 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.withLiveness(), 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.withLiveness(), 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 c36d1d6..7500235 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -802,12 +802,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.withLiveness(), 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/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
index a4c50eb..ff07b24 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredGenericSignatureTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredGenericSignatureTest.java
@@ -93,11 +93,10 @@
ClassSubject box = inspector.clazz(Box.class);
assertThat(box, isPresent());
String finalBoxDescriptor = box.getFinalDescriptor();
- // TODO(b/158124557): This should be J$.
assertEquals(
"Ljava/lang/Object;"
+ finalBoxDescriptor.substring(0, finalBoxDescriptor.length() - 1)
- + "<Ljava/time/LocalDate;>;",
+ + "<Lj$/time/LocalDate;>;",
javaTimeBox.getFinalSignatureAttribute());
}
