Only rename classpath and program with -applymapping
Bug: 133091438
Bug: 215318785
Change-Id: I286732576026cc49a3a9929e09847610b9cb85f1
diff --git a/src/main/java/com/android/tools/r8/graph/DexApplication.java b/src/main/java/com/android/tools/r8/graph/DexApplication.java
index c3e65b0..e08f2ed 100644
--- a/src/main/java/com/android/tools/r8/graph/DexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/DexApplication.java
@@ -126,13 +126,14 @@
return classesWithDeterministicOrder(new ArrayList<>(programClasses()));
}
- public static <T extends DexClass> List<T> classesWithDeterministicOrder(Collection<T> classes) {
+ public static <T extends ClassDefinition> List<T> classesWithDeterministicOrder(
+ Collection<T> classes) {
return classesWithDeterministicOrder(new ArrayList<>(classes));
}
- public static <T extends DexClass> List<T> classesWithDeterministicOrder(List<T> classes) {
+ public static <T extends ClassDefinition> List<T> classesWithDeterministicOrder(List<T> classes) {
// To keep the order deterministic, we sort the classes by their type, which is a unique key.
- classes.sort(Comparator.comparing(DexClass::getType));
+ classes.sort(Comparator.comparing(ClassDefinition::getType));
return classes;
}
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 ca54e1b..9a801b9 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -11,12 +11,13 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
-import com.android.tools.r8.graph.DexEncodedField;
-import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexClassAndField;
+import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexProto;
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.graph.ProgramOrClasspathClass;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
@@ -35,7 +36,7 @@
private final AppView<AppInfoWithLiveness> appView;
private final ClassNamingStrategy classNamingStrategy;
- private final Iterable<? extends DexClass> classes;
+ private final Iterable<? extends ProgramOrClasspathClass> classes;
private final Set<String> usedTypeNames = Sets.newHashSet();
private final Map<DexType, DexString> renaming = Maps.newIdentityHashMap();
private final Map<String, Namespace> states = new HashMap<>();
@@ -48,7 +49,7 @@
ClassNameMinifier(
AppView<AppInfoWithLiveness> appView,
ClassNamingStrategy classNamingStrategy,
- Iterable<? extends DexClass> classes) {
+ Iterable<? extends ProgramOrClasspathClass> classes) {
this.appView = appView;
this.classNamingStrategy = classNamingStrategy;
this.classes = classes;
@@ -86,11 +87,11 @@
ClassRenaming computeRenaming(Timing timing) {
// Collect names we have to keep.
timing.begin("reserve");
- for (DexClass clazz : classes) {
- DexString descriptor = classNamingStrategy.reservedDescriptor(clazz.type);
+ for (ProgramOrClasspathClass clazz : classes) {
+ DexString descriptor = classNamingStrategy.reservedDescriptor(clazz.getType());
if (descriptor != null) {
- assert !renaming.containsKey(clazz.type);
- registerClassAsUsed(clazz.type, descriptor);
+ assert !renaming.containsKey(clazz.getType());
+ registerClassAsUsed(clazz.getType(), descriptor);
}
}
appView
@@ -100,24 +101,17 @@
timing.end();
timing.begin("rename-classes");
- for (DexClass clazz : classes) {
- if (!renaming.containsKey(clazz.type)) {
- DexString renamed = computeName(clazz.type);
- renaming.put(clazz.type, renamed);
- // If the class is a member class and it has used $ separator, its renamed name should have
- // the same separator (as long as inner-class attribute is honored).
- assert !keepInnerClassStructure
- || !clazz.isMemberClass()
- || !clazz.type.getInternalName().contains(String.valueOf(INNER_CLASS_SEPARATOR))
- || renamed.toString().contains(String.valueOf(INNER_CLASS_SEPARATOR))
- || classNamingStrategy.isRenamedByApplyMapping(clazz.type)
- : clazz.toSourceString() + " -> " + renamed;
+ for (ProgramOrClasspathClass clazz : classes) {
+ if (!renaming.containsKey(clazz.getType())) {
+ DexString renamed = computeName(clazz.getType());
+ renaming.put(clazz.getType(), renamed);
+ assert verifyMemberRenamingOfInnerClasses(clazz.asDexClass(), renamed);
}
}
timing.end();
timing.begin("rename-dangling-types");
- for (DexClass clazz : classes) {
+ for (ProgramOrClasspathClass clazz : classes) {
renameDanglingTypes(clazz);
}
timing.end();
@@ -125,6 +119,18 @@
return new ClassRenaming(Collections.unmodifiableMap(renaming), getPackageRenaming());
}
+ private boolean verifyMemberRenamingOfInnerClasses(DexClass clazz, DexString renamed) {
+ // If the class is a member class and it has used $ separator, its renamed name should have
+ // the same separator (as long as inner-class attribute is honored).
+ assert !keepInnerClassStructure
+ || !clazz.isMemberClass()
+ || !clazz.getType().getInternalName().contains(String.valueOf(INNER_CLASS_SEPARATOR))
+ || renamed.toString().contains(String.valueOf(INNER_CLASS_SEPARATOR))
+ || classNamingStrategy.isRenamedByApplyMapping(clazz.getType())
+ : clazz + " -> " + renamed;
+ return true;
+ }
+
private Map<String, String> getPackageRenaming() {
ImmutableMap.Builder<String, String> packageRenaming = ImmutableMap.builder();
for (Entry<String, Namespace> entry : states.entrySet()) {
@@ -137,16 +143,16 @@
return packageRenaming.build();
}
- private void renameDanglingTypes(DexClass clazz) {
- clazz.forEachMethod(this::renameDanglingTypesInMethod);
- clazz.forEachField(this::renameDanglingTypesInField);
+ private void renameDanglingTypes(ProgramOrClasspathClass clazz) {
+ clazz.forEachClassMethod(this::renameDanglingTypesInMethod);
+ clazz.forEachClassField(this::renameDanglingTypesInField);
}
- private void renameDanglingTypesInField(DexEncodedField field) {
+ private void renameDanglingTypesInField(DexClassAndField field) {
renameDanglingType(field.getReference().type);
}
- private void renameDanglingTypesInMethod(DexEncodedMethod method) {
+ private void renameDanglingTypesInMethod(DexClassAndMethod method) {
DexProto proto = method.getReference().proto;
renameDanglingType(proto.returnType);
for (DexType type : proto.parameters.values) {
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java b/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
index 43cf17d..174614b 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
@@ -22,6 +22,7 @@
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramField;
+import com.android.tools.r8.graph.ProgramOrClasspathClass;
import com.android.tools.r8.graph.SubtypingInfo;
import com.android.tools.r8.naming.ClassNameMinifier.ClassRenaming;
import com.android.tools.r8.naming.FieldNameMinifier.FieldRenaming;
@@ -78,7 +79,7 @@
private final SeedMapper seedMapper;
private final BiMap<DexType, DexString> mappedNames = HashBiMap.create();
// To keep the order deterministic, we sort the classes by their type, which is a unique key.
- private final Set<DexClass> mappedClasses = Sets.newIdentityHashSet();
+ private final Set<ProgramOrClasspathClass> mappedClasses = Sets.newIdentityHashSet();
private final Map<DexReference, MemberNaming> memberNames = Maps.newIdentityHashMap();
private final Map<DexMethod, DexString> defaultInterfaceMethodImplementationNames =
Maps.newIdentityHashMap();
@@ -193,14 +194,17 @@
DexClass clazz = appView.definitionFor(type);
// Keep track of classes that needs to get renamed.
- if (clazz != null && (classNaming != null || clazz.isProgramClass())) {
- mappedClasses.add(clazz);
+ if (clazz != null) {
+ if (clazz.isClasspathClass() && classNaming != null) {
+ mappedClasses.add(clazz.asClasspathClass());
+ } else if (clazz.isProgramClass()) {
+ mappedClasses.add(clazz.asProgramClass());
+ }
}
Map<DexReference, MemberNaming> nonPrivateMembers = new IdentityHashMap<>();
- if (classNaming != null) {
- // TODO(b/133091438) assert that !dexClass.isLibraryClass();
+ if (classNaming != null && (clazz == null || !clazz.isLibraryClass())) {
DexString mappedName = factory.createString(classNaming.renamedName);
checkAndAddMappedNames(type, mappedName, classNaming.position);
classNaming.forAllMemberNaming(
diff --git a/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingOnLibraryPathTest.java b/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingOnLibraryPathTest.java
index ce65a85..df6fe4d 100644
--- a/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingOnLibraryPathTest.java
+++ b/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingOnLibraryPathTest.java
@@ -5,7 +5,6 @@
package com.android.tools.r8.naming.applymapping;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
-import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
@@ -42,19 +41,16 @@
.compile()
.inspect(
inspector -> {
- // TODO(b/215318785): We should never rename on library path.
ClassSubject clazz = inspector.clazz(Main.class);
assertThat(clazz, isPresentAndNotRenamed());
FoundClassSubject foundClassSubject = clazz.asFoundClassSubject();
- assertEquals("a.a", foundClassSubject.getSuperClass().getOriginalName());
+ assertEquals(
+ typeName(LibraryClass.class),
+ foundClassSubject.getSuperClass().getOriginalName());
})
.addRunClasspathClasses(LibraryClass.class)
.run(parameters.getRuntime(), Main.class)
- // TODO(b/215318785): Running the program should work.
- .assertFailureWithErrorThatMatchesIf(
- parameters.isCfRuntime(), containsString("Could not find or load main class"))
- .assertFailureWithErrorThatThrowsIf(
- parameters.isDexRuntime(), ClassNotFoundException.class);
+ .assertSuccessWithOutputLines("LibraryClass::foo");
}
public static class LibraryClass {
