Introduce new field name minifier
Bug: 128973195, 127932803, 128656974, 128600647
Change-Id: I0f64019de1fe02e3b87ed94bc86b03a2478e3946
diff --git a/src/main/java/com/android/tools/r8/graph/BottomUpClassHierarchyTraversal.java b/src/main/java/com/android/tools/r8/graph/BottomUpClassHierarchyTraversal.java
index 80f3d93..f1cadda 100644
--- a/src/main/java/com/android/tools/r8/graph/BottomUpClassHierarchyTraversal.java
+++ b/src/main/java/com/android/tools/r8/graph/BottomUpClassHierarchyTraversal.java
@@ -5,7 +5,7 @@
package com.android.tools.r8.graph;
public class BottomUpClassHierarchyTraversal<T extends DexClass>
- extends ClassHierarchyTraversal<T> {
+ extends ClassHierarchyTraversal<T, BottomUpClassHierarchyTraversal<T>> {
private BottomUpClassHierarchyTraversal(
AppView<? extends AppInfoWithSubtyping> appView, Scope scope) {
@@ -31,10 +31,19 @@
}
@Override
+ BottomUpClassHierarchyTraversal<T> self() {
+ return this;
+ }
+
+ @Override
void addDependentsToWorklist(DexClass clazz) {
@SuppressWarnings("unchecked")
T clazzWithTypeT = (T) clazz;
+ if (excludeInterfaces && clazzWithTypeT.isInterface()) {
+ return;
+ }
+
if (visited.contains(clazzWithTypeT)) {
return;
}
diff --git a/src/main/java/com/android/tools/r8/graph/ClassHierarchyTraversal.java b/src/main/java/com/android/tools/r8/graph/ClassHierarchyTraversal.java
index eb21910..7bf1464 100644
--- a/src/main/java/com/android/tools/r8/graph/ClassHierarchyTraversal.java
+++ b/src/main/java/com/android/tools/r8/graph/ClassHierarchyTraversal.java
@@ -11,7 +11,8 @@
import java.util.Set;
import java.util.function.Consumer;
-abstract class ClassHierarchyTraversal<T extends DexClass> {
+abstract class ClassHierarchyTraversal<
+ T extends DexClass, CHT extends ClassHierarchyTraversal<T, CHT>> {
enum Scope {
ALL_CLASSES,
@@ -24,11 +25,20 @@
final Set<T> visited = new HashSet<>();
final Deque<T> worklist = new ArrayDeque<>();
+ boolean excludeInterfaces = false;
+
ClassHierarchyTraversal(AppView<? extends AppInfoWithSubtyping> appView, Scope scope) {
this.appView = appView;
this.scope = scope;
}
+ abstract CHT self();
+
+ public CHT excludeInterfaces() {
+ this.excludeInterfaces = true;
+ return self();
+ }
+
public void visit(Iterable<DexProgramClass> sources, Consumer<T> visitor) {
Iterator<DexProgramClass> sourceIterator = sources.iterator();
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index cfc1507..a7c20c0 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -12,10 +12,12 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Predicates;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Iterators;
import com.google.common.collect.Sets;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.Set;
@@ -712,6 +714,15 @@
return initializationOfParentTypesMayHaveSideEffects(definitions, ignore);
}
+ public Iterable<DexType> allImmediateSupertypes() {
+ Iterator<DexType> iterator =
+ superType != null
+ ? Iterators.concat(
+ Iterators.singletonIterator(superType), Iterators.forArray(interfaces.values))
+ : Iterators.forArray(interfaces.values);
+ return () -> iterator;
+ }
+
public boolean initializationOfParentTypesMayHaveSideEffects(DexDefinitionSupplier definitions) {
return initializationOfParentTypesMayHaveSideEffects(definitions, Predicates.alwaysFalse());
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index 594ea9b..410025a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -245,6 +245,11 @@
|| name.contains(Java8MethodRewriter.UTILITY_CLASS_NAME_PREFIX);
}
+ public boolean isProgramType(DexDefinitionSupplier definitions) {
+ DexClass clazz = definitions.definitionFor(this);
+ return clazz != null && clazz.isProgramClass();
+ }
+
public int elementSizeForPrimitiveArrayType() {
assert isPrimitiveArrayType();
switch (descriptor.content[1]) {
diff --git a/src/main/java/com/android/tools/r8/graph/TopDownClassHierarchyTraversal.java b/src/main/java/com/android/tools/r8/graph/TopDownClassHierarchyTraversal.java
index 9e7fbe4..2fe8bd2 100644
--- a/src/main/java/com/android/tools/r8/graph/TopDownClassHierarchyTraversal.java
+++ b/src/main/java/com/android/tools/r8/graph/TopDownClassHierarchyTraversal.java
@@ -4,7 +4,8 @@
package com.android.tools.r8.graph;
-public class TopDownClassHierarchyTraversal<T extends DexClass> extends ClassHierarchyTraversal<T> {
+public class TopDownClassHierarchyTraversal<T extends DexClass>
+ extends ClassHierarchyTraversal<T, TopDownClassHierarchyTraversal<T>> {
private TopDownClassHierarchyTraversal(
AppView<? extends AppInfoWithSubtyping> appView, Scope scope) {
@@ -30,10 +31,19 @@
}
@Override
+ TopDownClassHierarchyTraversal<T> self() {
+ return this;
+ }
+
+ @Override
void addDependentsToWorklist(DexClass clazz) {
@SuppressWarnings("unchecked")
T clazzWithTypeT = (T) clazz;
+ if (excludeInterfaces && clazzWithTypeT.isInterface()) {
+ return;
+ }
+
if (visited.contains(clazzWithTypeT)) {
return;
}
@@ -51,11 +61,13 @@
}
// Add super interfaces to worklist.
- for (DexType interfaceType : clazz.interfaces.values) {
- DexClass definition = appView.definitionFor(interfaceType);
- if (definition != null) {
- if (scope != Scope.ONLY_PROGRAM_CLASSES || definition.isProgramClass()) {
- addDependentsToWorklist(definition);
+ if (!excludeInterfaces) {
+ for (DexType interfaceType : clazz.interfaces.values) {
+ DexClass definition = appView.definitionFor(interfaceType);
+ if (definition != null) {
+ if (scope != Scope.ONLY_PROGRAM_CLASSES || definition.isProgramClass()) {
+ addDependentsToWorklist(definition);
+ }
}
}
}
diff --git a/src/main/java/com/android/tools/r8/naming/FieldNameMinifier.java b/src/main/java/com/android/tools/r8/naming/FieldNameMinifier.java
index 22f5379..2130b37 100644
--- a/src/main/java/com/android/tools/r8/naming/FieldNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/FieldNameMinifier.java
@@ -4,36 +4,37 @@
package com.android.tools.r8.naming;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.BottomUpClassHierarchyTraversal;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.TopDownClassHierarchyTraversal;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Sets;
-import java.io.PrintStream;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.Deque;
+import java.util.IdentityHashMap;
+import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.function.Function;
+import java.util.TreeSet;
-class FieldNameMinifier extends MemberNameMinifier<DexField, DexType> {
+class FieldNameMinifier {
+
+ private final AppView<AppInfoWithLiveness> appView;
+ private final Map<DexField, DexString> renaming = new IdentityHashMap<>();
+ private Map<DexType, ReservedFieldNamingState> reservedNamingStates = new IdentityHashMap<>();
+ private final MemberNamingStrategy strategy;
FieldNameMinifier(AppView<AppInfoWithLiveness> appView, MemberNamingStrategy strategy) {
- super(appView, strategy);
- }
-
- @Override
- Function<DexType, ?> getKeyTransform() {
- if (overloadAggressively) {
- // Use the type as the key, hence reuse names per type.
- return Function.identity();
- } else {
- // Always use the same key, hence do not reuse names per type.
- return a -> Void.class;
- }
+ this.appView = appView;
+ this.strategy = strategy;
}
FieldRenaming computeRenaming(Collection<DexClass> interfaces, Timing timing) {
@@ -41,19 +42,14 @@
// shadow a reserved field in subclasses. While there is no concept of virtual field
// dispatch in Java, field resolution still traverses the super type chain and external
// code might use a subtype to reference the field.
- timing.begin("reserve-classes");
- reserveNamesInSubtypes(appView.dexItemFactory().objectType, globalState);
- timing.end();
- // Next, reserve field names in interfaces. These should only be static.
- timing.begin("reserve-interfaces");
- for (DexClass iface : interfaces) {
- reserveNamesInSubtypes(iface.type, globalState);
- }
+ timing.begin("reserve-names");
+ reserveFieldNames();
timing.end();
// Rename the definitions.
timing.begin("rename-definitions");
- renameFieldsInClasses();
renameFieldsInInterfaces(interfaces);
+ propagateReservedFieldNamesUpwards();
+ renameFieldsInClasses();
timing.end();
// Rename the references that are not rebound to definitions for some reasons.
timing.begin("rename-references");
@@ -75,81 +71,148 @@
}
}
- private void reserveNamesInSubtypes(DexType type, NamingState<DexType, ?> state) {
- DexClass holder = appView.definitionFor(type);
- if (holder == null) {
- return;
- }
- // If there is a mapping file, it can be that fields in libraries should be renamed and
- // therefore not reserved.
- NamingState<DexType, ?> newState = computeStateIfAbsent(type, t -> state.createChild());
- holder.forEachField(
- field -> reserveFieldName(field, newState, alwaysReserveMemberNames(holder)));
- appView
- .appInfo()
- .forAllExtendsSubtypes(type, subtype -> reserveNamesInSubtypes(subtype, newState));
+ private ReservedFieldNamingState getReservedFieldNamingState(DexType type) {
+ return reservedNamingStates.get(type);
}
- private void reserveFieldName(
- DexEncodedField encodedField, NamingState<DexType, ?> state, boolean alwaysReserve) {
- DexField field = encodedField.field;
- if (alwaysReserve || strategy.noObfuscation().contains(field)) {
- state.reserveName(field.name, field.type);
- }
+ private ReservedFieldNamingState getOrCreateReservedFieldNamingState(DexType type) {
+ return reservedNamingStates.computeIfAbsent(
+ type, ignore -> new ReservedFieldNamingState(appView));
}
- private void renameFieldsInClasses() {
- renameFieldsInSubclasses(appView.dexItemFactory().objectType, null);
- }
+ private void reserveFieldNames() {
+ // Reserve all field names that need to be reserved.
+ for (DexClass clazz : appView.appInfo().app().asDirect().allClasses()) {
+ ReservedFieldNamingState reservedNames = null;
+ for (DexEncodedField field : clazz.fields()) {
+ if (clazz.isLibraryClass() || appView.rootSet().noObfuscation.contains(field.field)) {
+ if (reservedNames == null) {
+ reservedNames = getOrCreateReservedFieldNamingState(clazz.type);
+ }
+ reservedNames.markReservedDirectly(field.field);
+ }
+ }
- private void renameFieldsInSubclasses(DexType type, DexType parent) {
- DexClass clazz = appView.definitionFor(type);
- if (clazz == null) {
- return;
- }
- assert !clazz.isInterface();
- assert clazz.superType == parent;
-
- NamingState<DexType, ?> state = minifierState.getState(clazz.type);
- assert state != null;
- for (DexEncodedField field : clazz.fields()) {
- renameField(field, state);
- }
- for (DexType subclass : appView.appInfo().allExtendsSubtypes(type)) {
- renameFieldsInSubclasses(subclass, type);
- }
- }
-
- private void renameFieldsInInterfaces(Collection<DexClass> interfaces) {
- for (DexClass iface : interfaces) {
- renameFieldsInInterface(iface);
- }
- }
-
- private void renameFieldsInInterface(DexClass clazz) {
- assert clazz.isInterface();
- NamingState<DexType, ?> state = minifierState.getState(clazz.type);
- assert state != null;
- for (DexEncodedField field : clazz.fields()) {
- renameField(field, state);
- }
- }
-
- private void renameField(DexEncodedField encodedField, NamingState<DexType, ?> state) {
- DexField field = encodedField.field;
-
- Set<String> loggingFilter = appView.options().extensiveFieldMinifierLoggingFilter;
- if (!loggingFilter.isEmpty()) {
- if (loggingFilter.contains(field.toSourceString())) {
- print(field, state, System.out);
+ // For interfaces, propagate reserved names to all implementing classes.
+ if (clazz.isInterface() && reservedNames != null) {
+ for (DexType implementationType : appView.appInfo().allImplementsSubtypes(clazz.type)) {
+ DexClass implementation = appView.definitionFor(implementationType);
+ if (implementation != null) {
+ assert !implementation.isInterface();
+ getOrCreateReservedFieldNamingState(implementationType)
+ .includeReservations(reservedNames);
+ }
+ }
}
}
- if (!state.isReserved(field.name, field.type)) {
- renaming.put(field, state.assignNewNameFor(field, field.name, field.type));
+ propagateReservedFieldNamesUpwards();
+ }
+
+ private void propagateReservedFieldNamesUpwards() {
+ BottomUpClassHierarchyTraversal.forProgramClasses(appView)
+ .visit(
+ appView.appInfo().classes(),
+ clazz -> {
+ ReservedFieldNamingState reservedNames = getReservedFieldNamingState(clazz.type);
+ if (reservedNames != null) {
+ for (DexType supertype : clazz.allImmediateSupertypes()) {
+ if (supertype.isProgramType(appView)) {
+ getOrCreateReservedFieldNamingState(supertype)
+ .includeReservationsFromBelow(reservedNames);
+ }
+ }
+ }
+ });
+ }
+
+ private void renameFieldsInClasses() {
+ Map<DexType, FieldNamingState> states = new IdentityHashMap<>();
+ TopDownClassHierarchyTraversal.forAllClasses(appView)
+ .excludeInterfaces()
+ .visit(
+ appView.appInfo().classes(),
+ clazz -> {
+ assert !clazz.isInterface();
+
+ FieldNamingState parentState =
+ clazz.superType == null
+ ? new FieldNamingState(appView, strategy)
+ : states
+ .computeIfAbsent(
+ clazz.superType, key -> new FieldNamingState(appView, strategy))
+ .clone();
+
+ ReservedFieldNamingState reservedNames =
+ getOrCreateReservedFieldNamingState(clazz.type);
+ FieldNamingState state = parentState.createChildState(reservedNames);
+ if (clazz.isProgramClass()) {
+ for (DexEncodedField field : clazz.fields()) {
+ renameField(field, state);
+ }
+ }
+
+ assert !states.containsKey(clazz.type);
+ states.put(clazz.type, state);
+ });
+ }
+
+ private void renameFieldsInInterfaces(Collection<DexClass> interfaces) {
+ InterfacePartitioning partioning = new InterfacePartitioning(appView);
+ for (Set<DexClass> partition : partioning.sortedPartitions(interfaces)) {
+ renameFieldsInInterfacePartition(partition);
}
}
+ private void renameFieldsInInterfacePartition(Set<DexClass> partition) {
+ ReservedFieldNamingState reservedNamesInPartition = new ReservedFieldNamingState(appView);
+ for (DexClass clazz : partition) {
+ ReservedFieldNamingState reservedNamesInInterface = getReservedFieldNamingState(clazz.type);
+ if (reservedNamesInInterface != null) {
+ reservedNamesInPartition.includeReservations(reservedNamesInInterface);
+ reservedNamesInPartition.includeReservationsFromBelow(reservedNamesInInterface);
+ }
+ }
+
+ ReservedFieldNamingState namesToBeReservedInImplementsSubclasses =
+ new ReservedFieldNamingState(appView);
+
+ FieldNamingState state = new FieldNamingState(appView, strategy, reservedNamesInPartition);
+ for (DexClass clazz : partition) {
+ if (clazz.isProgramClass()) {
+ assert clazz.isInterface();
+ for (DexEncodedField field : clazz.fields()) {
+ DexString newName = renameField(field, state);
+ namesToBeReservedInImplementsSubclasses.markReservedDirectly(newName, field.field.type);
+ }
+ }
+ }
+
+ Set<DexType> visited = Sets.newIdentityHashSet();
+ for (DexClass clazz : partition) {
+ for (DexType implementationType : appView.appInfo().allImplementsSubtypes(clazz.type)) {
+ if (!visited.add(implementationType)) {
+ continue;
+ }
+
+ DexClass implementation = appView.definitionFor(implementationType);
+ if (implementation != null) {
+ getOrCreateReservedFieldNamingState(implementationType)
+ .includeReservations(namesToBeReservedInImplementsSubclasses);
+ }
+ }
+ }
+ }
+
+ private DexString renameField(DexEncodedField encodedField, FieldNamingState state) {
+ DexField field = encodedField.field;
+ DexString newName = state.getOrCreateNameFor(field);
+ if (newName != field.name) {
+ renaming.put(field, newName);
+ }
+ return newName;
+ }
+
private void renameNonReboundReferences() {
// TODO(b/123068484): Collect non-rebound references instead of visiting all references.
AppInfoWithLiveness appInfo = appView.appInfo();
@@ -190,11 +253,70 @@
}
}
- private void print(DexField field, NamingState<DexType, ?> state, PrintStream out) {
- out.println("--------------------------------------------------------------------------------");
- out.println("FieldNameMinifier(`" + field.toSourceString() + "`)");
- out.println("--------------------------------------------------------------------------------");
- state.printState(field.type, minifierState::getStateKey, "", out);
- out.println();
+ static class InterfacePartitioning {
+
+ private final AppView<AppInfoWithLiveness> appView;
+ private final Set<DexType> visited = Sets.newIdentityHashSet();
+
+ InterfacePartitioning(AppView<AppInfoWithLiveness> appView) {
+ this.appView = appView;
+ }
+
+ private List<Set<DexClass>> sortedPartitions(Collection<DexClass> interfaces) {
+ List<Set<DexClass>> partitions = new ArrayList<>();
+ for (DexClass clazz : interfaces) {
+ if (clazz != null && visited.add(clazz.type)) {
+ Set<DexClass> partition = buildSortedPartition(clazz);
+ assert !partition.isEmpty();
+ assert partition.stream().allMatch(DexClass::isInterface);
+ assert partition.stream().map(DexClass::getType).allMatch(visited::contains);
+ partitions.add(partition);
+ }
+ }
+ return partitions;
+ }
+
+ private Set<DexClass> buildSortedPartition(DexClass src) {
+ Set<DexClass> partition = new TreeSet<>((x, y) -> x.type.slowCompareTo(y.type));
+
+ Deque<DexType> worklist = new ArrayDeque<>();
+ worklist.add(src.type);
+
+ while (!worklist.isEmpty()) {
+ DexType type = worklist.removeFirst();
+
+ DexClass clazz = appView.definitionFor(type);
+ if (clazz == null) {
+ continue;
+ }
+
+ for (DexType superinterface : clazz.interfaces.values) {
+ if (visited.add(superinterface)) {
+ worklist.add(superinterface);
+ }
+ }
+
+ if (clazz.isInterface()) {
+ partition.add(clazz);
+
+ for (DexType subtype : appView.appInfo().allImmediateSubtypes(type)) {
+ if (visited.add(subtype)) {
+ worklist.add(subtype);
+ }
+ }
+ } else if (clazz.type != appView.dexItemFactory().objectType) {
+ if (visited.add(clazz.superType)) {
+ worklist.add(clazz.superType);
+ }
+ for (DexType subclass : appView.appInfo().allExtendsSubtypes(type)) {
+ if (visited.add(subclass)) {
+ worklist.add(subclass);
+ }
+ }
+ }
+ }
+
+ return partition;
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/naming/FieldNamingState.java b/src/main/java/com/android/tools/r8/naming/FieldNamingState.java
new file mode 100644
index 0000000..acb09f0
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/naming/FieldNamingState.java
@@ -0,0 +1,136 @@
+// 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.naming;
+
+import static com.android.tools.r8.naming.Minifier.MinifierMemberNamingStrategy.EMPTY_CHAR_ARRAY;
+
+import com.android.tools.r8.graph.AppInfo;
+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.DexField;
+import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.naming.FieldNamingState.InternalState;
+import com.android.tools.r8.utils.StringUtils;
+import java.util.IdentityHashMap;
+import java.util.Iterator;
+import java.util.Map;
+
+public class FieldNamingState extends FieldNamingStateBase<InternalState> implements Cloneable {
+
+ private final ReservedFieldNamingState reservedNames;
+ private final MemberNamingStrategy strategy;
+
+ public FieldNamingState(AppView<? extends AppInfo> appView, MemberNamingStrategy strategy) {
+ this(appView, strategy, new ReservedFieldNamingState(appView));
+ }
+
+ public FieldNamingState(
+ AppView<? extends AppInfo> appView,
+ MemberNamingStrategy strategy,
+ ReservedFieldNamingState reservedNames) {
+ this(appView, strategy, reservedNames, new IdentityHashMap<>());
+ }
+
+ private FieldNamingState(
+ AppView<? extends AppInfo> appView,
+ MemberNamingStrategy strategy,
+ ReservedFieldNamingState reservedNames,
+ Map<DexType, InternalState> internalStates) {
+ super(appView, internalStates);
+ this.reservedNames = reservedNames;
+ this.strategy = strategy;
+ }
+
+ public FieldNamingState createChildState(ReservedFieldNamingState reservedNames) {
+ FieldNamingState childState =
+ new FieldNamingState(appView, strategy, reservedNames, internalStates);
+ childState.includeReservations(this.reservedNames);
+ return childState;
+ }
+
+ public DexString getOrCreateNameFor(DexField field) {
+ DexEncodedField encodedField = appView.appInfo().resolveField(field);
+ if (encodedField != null) {
+ DexClass clazz = appView.definitionFor(encodedField.field.holder);
+ if (clazz == null || clazz.isLibraryClass()) {
+ return field.name;
+ }
+ if (!appView.options().getProguardConfiguration().hasApplyMappingFile()
+ && appView.rootSet().noObfuscation.contains(encodedField.field)) {
+ return field.name;
+ }
+ }
+ return getOrCreateInternalState(field).createNewName(field);
+ }
+
+ public void includeReservations(ReservedFieldNamingState reservedNames) {
+ this.reservedNames.includeReservations(reservedNames);
+ }
+
+ @Override
+ public InternalState createInternalState() {
+ return new InternalState();
+ }
+
+ @Override
+ public FieldNamingState clone() {
+ Map<DexType, InternalState> internalStatesClone = new IdentityHashMap<>();
+ for (Map.Entry<DexType, InternalState> entry : internalStates.entrySet()) {
+ internalStatesClone.put(entry.getKey(), entry.getValue().clone());
+ }
+ return new FieldNamingState(appView, strategy, reservedNames, internalStatesClone);
+ }
+
+ class InternalState implements Cloneable {
+
+ private final Iterator<String> dictionaryIterator;
+ private int nextNameIndex;
+
+ public InternalState() {
+ this(1);
+ }
+
+ public InternalState(int nextNameIndex) {
+ this(
+ nextNameIndex,
+ appView.options().getProguardConfiguration().getObfuscationDictionary().iterator());
+ }
+
+ public InternalState(int nextNameIndex, Iterator<String> dictionaryIterator) {
+ this.dictionaryIterator = dictionaryIterator;
+ this.nextNameIndex = nextNameIndex;
+ }
+
+ public DexString createNewName(DexField field) {
+ DexString name;
+ do {
+ name = nextNameAccordingToStrategy(field);
+ } while (reservedNames.isReserved(name, field.type)
+ && !strategy.breakOnNotAvailable(field, name));
+ return name;
+ }
+
+ private DexString nextNameAccordingToStrategy(DexField field) {
+ if (!strategy.bypassDictionary() && dictionaryIterator.hasNext()) {
+ return appView.dexItemFactory().createString(dictionaryIterator.next());
+ } else {
+ return strategy.next(field, this);
+ }
+ }
+
+ public DexString nextNameAccordingToState() {
+ return appView
+ .dexItemFactory()
+ .createString(StringUtils.numberToIdentifier(EMPTY_CHAR_ARRAY, nextNameIndex++, false));
+ }
+
+ @Override
+ public InternalState clone() {
+ return new InternalState(nextNameIndex, dictionaryIterator);
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/naming/FieldNamingStateBase.java b/src/main/java/com/android/tools/r8/naming/FieldNamingStateBase.java
new file mode 100644
index 0000000..94a4c53
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/naming/FieldNamingStateBase.java
@@ -0,0 +1,46 @@
+// 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.naming;
+
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.shaking.ProguardConfiguration;
+import java.util.Map;
+
+abstract class FieldNamingStateBase<T> {
+
+ final AppView<? extends AppInfo> appView;
+ final Map<DexType, T> internalStates;
+
+ FieldNamingStateBase(AppView<? extends AppInfo> appView, Map<DexType, T> internalStates) {
+ this.appView = appView;
+ this.internalStates = internalStates;
+ }
+
+ final T getInternalState(DexType type) {
+ DexType internalStateKey = getInternalStateKey(type);
+ return internalStates.get(internalStateKey);
+ }
+
+ final T getOrCreateInternalState(DexField field) {
+ return getOrCreateInternalState(field.type);
+ }
+
+ final T getOrCreateInternalState(DexType type) {
+ DexType internalStateKey = getInternalStateKey(type);
+ return internalStates.computeIfAbsent(internalStateKey, key -> createInternalState());
+ }
+
+ private DexType getInternalStateKey(DexType type) {
+ ProguardConfiguration proguardConfiguration = appView.options().getProguardConfiguration();
+ return proguardConfiguration.isOverloadAggressively()
+ ? type
+ : appView.dexItemFactory().voidType;
+ }
+
+ abstract T createInternalState();
+}
diff --git a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
index 4ec7337..ee769f0 100644
--- a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
@@ -14,7 +14,6 @@
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.naming.MethodNameMinifier.FrontierState;
-import com.android.tools.r8.naming.MethodNameMinifier.MethodNamingState;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Timing;
import com.google.common.base.Equivalence;
@@ -33,24 +32,84 @@
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
+import java.util.function.Function;
import java.util.stream.Collectors;
public class InterfaceMethodNameMinifier {
+ /**
+ * Capture a (name, proto)-pair for a {@link MethodNamingState}. Each method
+ * methodState.METHOD(...) simply defers to parent.METHOD(name, proto, ...). This allows R8 to
+ * assign the same name to methods with different prototypes, which is needed for multi-interface
+ * lambdas.
+ */
+ static class InterfaceMethodNamingState {
+
+ private final MethodNamingState<?> parent;
+ private final DexString name;
+ private final DexProto proto;
+ private final DexMethod method;
+
+ InterfaceMethodNamingState(
+ MethodNamingState<?> parent, DexMethod method, DexString name, DexProto proto) {
+ this.method = method;
+ assert parent != null;
+ this.parent = parent;
+ this.name = name;
+ this.proto = proto;
+ }
+
+ DexString assignNewName() {
+ return parent.assignNewNameFor(method, name, proto);
+ }
+
+ boolean isReserved() {
+ return parent.isReserved(name, proto);
+ }
+
+ boolean isAvailable(DexString candidate) {
+ return parent.isAvailable(proto, candidate);
+ }
+
+ void addRenaming(DexString newName) {
+ parent.addRenaming(name, proto, newName);
+ }
+
+ DexString getName() {
+ return name;
+ }
+
+ DexProto getProto() {
+ return proto;
+ }
+
+ void print(
+ String indentation,
+ Function<MethodNamingState<?>, DexType> stateKeyGetter,
+ PrintStream out) {
+ DexType stateKey = stateKeyGetter.apply(parent);
+ out.print(indentation);
+ out.print(stateKey != null ? stateKey.toSourceString() : "<?>");
+ out.print(".");
+ out.print(name.toSourceString());
+ out.println(proto.toSmaliString());
+ parent.printState(proto, stateKeyGetter, indentation + " ", out);
+ }
+ }
+
private final AppView<AppInfoWithLiveness> appView;
private final Set<DexCallSite> desugaredCallSites;
private final Equivalence<DexMethod> equivalence;
private final FrontierState frontierState;
- private final MemberNameMinifier<DexMethod, DexProto>.State minifierState;
+ private final MethodNameMinifier.State minifierState;
private final Map<DexCallSite, DexString> callSiteRenamings = new IdentityHashMap<>();
/** A map from DexMethods to all the states linked to interfaces they appear in. */
- private final Map<Wrapper<DexMethod>, Set<NamingState<DexProto, ?>>> globalStateMap =
- new HashMap<>();
+ private final Map<Wrapper<DexMethod>, Set<MethodNamingState<?>>> globalStateMap = new HashMap<>();
/** A map from DexMethods to the first interface state it was seen in. Used to pick good names. */
- private final Map<Wrapper<DexMethod>, NamingState<DexProto, ?>> originStates = new HashMap<>();
+ private final Map<Wrapper<DexMethod>, MethodNamingState<?>> originStates = new HashMap<>();
/**
* A map from DexMethods to all the definitions seen. Needed as the Wrapper equalizes them all.
@@ -62,7 +121,7 @@
Set<DexCallSite> desugaredCallSites,
Equivalence<DexMethod> equivalence,
FrontierState frontierState,
- MemberNameMinifier<DexMethod, DexProto>.State minifierState) {
+ MethodNameMinifier.State minifierState) {
this.appView = appView;
this.desugaredCallSites = desugaredCallSites;
this.equivalence = equivalence;
@@ -98,8 +157,7 @@
allInterfaceTypes.add(iface.type);
}
for (DexClass iface : interfaces) {
- Set<NamingState<DexProto, ?>> collectedStates =
- getReachableStates(iface.type, allInterfaceTypes);
+ Set<MethodNamingState<?>> collectedStates = getReachableStates(iface.type, allInterfaceTypes);
for (DexEncodedMethod method : shuffleMethods(iface.methods(), appView.options())) {
addStatesToGlobalMapForMethod(method.method, collectedStates, iface.type);
}
@@ -133,7 +191,7 @@
callSites.put(callSite, implementedMethods.iterator().next().method);
for (DexEncodedMethod method : implementedMethods) {
DexType iface = method.method.holder;
- Set<NamingState<DexProto, ?>> collectedStates =
+ Set<MethodNamingState<?>> collectedStates =
getReachableStates(iface, allInterfaceTypes);
addStatesToGlobalMapForMethod(method.method, collectedStates, iface);
callSiteMethods.add(equivalence.wrap(method.method));
@@ -222,7 +280,7 @@
for (Wrapper<DexMethod> wrapper : unifiedMethods) {
DexMethod unifiedMethod = wrapper.get();
assert unifiedMethod != null;
- for (NamingState<DexProto, ?> namingState : globalStateMap.get(wrapper)) {
+ for (MethodNamingState<?> namingState : globalStateMap.get(wrapper)) {
if (!namingState.isReserved(unifiedMethod.name, unifiedMethod.proto)) {
namingState.reserveName(unifiedMethod.name, unifiedMethod.proto);
changed = true;
@@ -234,15 +292,15 @@
private void assignNameToInterfaceMethod(
Wrapper<DexMethod> key, Map<Wrapper<DexMethod>, Set<Wrapper<DexMethod>>> unification) {
- List<MethodNamingState> collectedStates = new ArrayList<>();
+ List<InterfaceMethodNamingState> collectedStates = new ArrayList<>();
Set<DexMethod> sourceMethods = Sets.newIdentityHashSet();
for (Wrapper<DexMethod> k : unification.getOrDefault(key, Collections.singleton(key))) {
DexMethod unifiedMethod = k.get();
assert unifiedMethod != null;
sourceMethods.addAll(sourceMethodsMap.get(k));
- for (NamingState<DexProto, ?> namingState : globalStateMap.get(k)) {
+ for (MethodNamingState<?> namingState : globalStateMap.get(k)) {
collectedStates.add(
- new MethodNamingState(
+ new InterfaceMethodNamingState(
namingState, unifiedMethod, unifiedMethod.name, unifiedMethod.proto));
}
}
@@ -257,15 +315,15 @@
}
}
- MethodNamingState originState =
- new MethodNamingState(originStates.get(key), method, method.name, method.proto);
+ InterfaceMethodNamingState originState =
+ new InterfaceMethodNamingState(originStates.get(key), method, method.name, method.proto);
assignNameForInterfaceMethodInAllStates(collectedStates, sourceMethods, originState);
}
private void assignNameForInterfaceMethodInAllStates(
- List<MethodNamingState> collectedStates,
+ List<InterfaceMethodNamingState> collectedStates,
Set<DexMethod> sourceMethods,
- MethodNamingState originState) {
+ InterfaceMethodNamingState originState) {
assert !anyIsReserved(collectedStates);
// We use the origin state to allocate a name here so that we can reuse names between different
@@ -274,14 +332,14 @@
DexString candidate;
do {
candidate = originState.assignNewName();
- for (MethodNamingState state : collectedStates) {
+ for (InterfaceMethodNamingState state : collectedStates) {
if (!state.isAvailable(candidate)) {
candidate = null;
break;
}
}
} while (candidate == null);
- for (MethodNamingState state : collectedStates) {
+ for (InterfaceMethodNamingState state : collectedStates) {
state.addRenaming(candidate);
}
// Rename all methods in interfaces that gave rise to this renaming.
@@ -291,7 +349,7 @@
}
private void addStatesToGlobalMapForMethod(
- DexMethod method, Set<NamingState<DexProto, ?>> collectedStates, DexType originInterface) {
+ DexMethod method, Set<MethodNamingState<?>> collectedStates, DexType originInterface) {
assert collectedStates.stream().noneMatch(Objects::isNull);
Wrapper<DexMethod> key = equivalence.wrap(method);
globalStateMap.computeIfAbsent(key, k -> new HashSet<>()).addAll(collectedStates);
@@ -307,7 +365,7 @@
DexMethod unifiedMethod = wrapper.get();
assert unifiedMethod != null;
- for (NamingState<DexProto, ?> namingState : globalStateMap.get(wrapper)) {
+ for (MethodNamingState<?> namingState : globalStateMap.get(wrapper)) {
if (namingState.isReserved(unifiedMethod.name, unifiedMethod.proto)) {
return true;
}
@@ -316,10 +374,10 @@
return false;
}
- private boolean anyIsReserved(List<MethodNamingState> collectedStates) {
+ private boolean anyIsReserved(List<InterfaceMethodNamingState> collectedStates) {
DexString name = collectedStates.get(0).getName();
Map<DexProto, Boolean> globalStateCache = new IdentityHashMap<>();
- for (MethodNamingState state : collectedStates) {
+ for (InterfaceMethodNamingState state : collectedStates) {
assert state.getName() == name;
boolean isReservedInGlobalState =
globalStateCache.computeIfAbsent(
@@ -332,18 +390,18 @@
return false;
}
- private Set<NamingState<DexProto, ?>> getReachableStates(DexType type, Set<DexType> allInterfaces) {
+ private Set<MethodNamingState<?>> getReachableStates(DexType type, Set<DexType> allInterfaces) {
Set<DexType> reachableInterfaces = getReachableInterfaces(type, allInterfaces);
- Set<NamingState<DexProto, ?>> reachableStates = new HashSet<>();
+ Set<MethodNamingState<?>> reachableStates = new HashSet<>();
for (DexType reachableInterface : reachableInterfaces) {
// Add the interface itself.
- NamingState<DexProto, ?> ifaceState = minifierState.getState(reachableInterface);
+ MethodNamingState<?> ifaceState = minifierState.getState(reachableInterface);
assert ifaceState != null;
reachableStates.add(ifaceState);
// And the frontiers that correspond to the classes that implement the interface.
for (DexType frontier : appView.appInfo().allImplementsSubtypes(reachableInterface)) {
- NamingState<DexProto, ?> state = minifierState.getState(frontierState.get(frontier));
+ MethodNamingState<?> state = minifierState.getState(frontierState.get(frontier));
assert state != null;
reachableStates.add(state);
}
@@ -388,7 +446,7 @@
private void print(
DexMethod method,
Set<DexMethod> sourceMethods,
- List<MethodNamingState> collectedStates,
+ List<InterfaceMethodNamingState> collectedStates,
PrintStream out) {
out.println("-----------------------------------------------------------------------");
out.println("assignNameToInterfaceMethod(`" + method.toSourceString() + "`)");
diff --git a/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
deleted file mode 100644
index d8007fc..0000000
--- a/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
+++ /dev/null
@@ -1,96 +0,0 @@
-// Copyright (c) 2017, 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.naming;
-
-import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.CachedHashValueDexItem;
-import com.android.tools.r8.graph.DexClass;
-import com.android.tools.r8.graph.DexReference;
-import com.android.tools.r8.graph.DexString;
-import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.naming.NamingState.InternalState;
-import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.shaking.ProguardConfiguration;
-import com.google.common.collect.BiMap;
-import com.google.common.collect.HashBiMap;
-import java.util.IdentityHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.function.Function;
-
-abstract class MemberNameMinifier<MemberType, StateType extends CachedHashValueDexItem> {
-
- protected final AppView<AppInfoWithLiveness> appView;
- protected final List<String> dictionary;
-
- protected final Map<MemberType, DexString> renaming = new IdentityHashMap<>();
- protected final NamingState<StateType, ?> globalState;
- protected final boolean overloadAggressively;
- protected final boolean useApplyMapping;
- protected final MemberNamingStrategy strategy;
-
- protected final State minifierState = new State();
-
- // The use of a bidirectional map allows us to map a naming state to the type it represents,
- // which is useful for debugging.
- private final BiMap<DexType, NamingState<StateType, ?>> states = HashBiMap.create();
-
- MemberNameMinifier(AppView<AppInfoWithLiveness> appView, MemberNamingStrategy strategy) {
- ProguardConfiguration proguardConfiguration = appView.options().getProguardConfiguration();
- this.appView = appView;
- this.dictionary = proguardConfiguration.getObfuscationDictionary();
- this.overloadAggressively = proguardConfiguration.isOverloadAggressively();
- this.globalState =
- NamingState.createRoot(appView.dexItemFactory(), dictionary, getKeyTransform(), strategy);
- this.useApplyMapping = proguardConfiguration.hasApplyMappingFile();
- this.strategy = strategy;
- }
-
- abstract Function<StateType, ?> getKeyTransform();
-
- protected NamingState<StateType, ?> computeStateIfAbsent(
- DexType type, Function<DexType, NamingState<StateType, ?>> f) {
- return states.computeIfAbsent(type, f);
- }
-
- protected boolean alwaysReserveMemberNames(DexClass holder) {
- return !useApplyMapping && holder.isNotProgramClass();
- }
-
- // A class that provides access to the minification state. An instance of this class is passed
- // from the method name minifier to the interface method name minifier.
- class State {
-
- DexString getRenaming(MemberType key) {
- return renaming.get(key);
- }
-
- void putRenaming(MemberType key, DexString value) {
- renaming.put(key, value);
- }
-
- NamingState<StateType, ?> getState(DexType type) {
- return states.get(type);
- }
-
- DexType getStateKey(NamingState<StateType, ?> state) {
- return states.inverse().get(state);
- }
-
- boolean isReservedInGlobalState(DexString name, StateType state) {
- return globalState.isReserved(name, state);
- }
- }
-
- interface MemberNamingStrategy {
- DexString next(DexReference source, InternalState internalState);
-
- boolean bypassDictionary();
-
- boolean breakOnNotAvailable(DexReference source, DexString name);
-
- Set<DexReference> noObfuscation();
- }
-}
diff --git a/src/main/java/com/android/tools/r8/naming/MemberNamingStrategy.java b/src/main/java/com/android/tools/r8/naming/MemberNamingStrategy.java
new file mode 100644
index 0000000..5c37c46
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/naming/MemberNamingStrategy.java
@@ -0,0 +1,24 @@
+// 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.naming;
+
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexReference;
+import com.android.tools.r8.graph.DexString;
+import java.util.Set;
+
+public interface MemberNamingStrategy {
+
+ DexString next(DexMethod method, MethodNamingState.InternalState internalState);
+
+ DexString next(DexField field, FieldNamingState.InternalState internalState);
+
+ boolean bypassDictionary();
+
+ boolean breakOnNotAvailable(DexReference source, DexString name);
+
+ Set<DexReference> noObfuscation();
+}
diff --git a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
index d26a3d9..a8892d8 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
@@ -18,8 +18,9 @@
import com.android.tools.r8.utils.Timing;
import com.google.common.base.Equivalence;
import com.google.common.base.Equivalence.Wrapper;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableMap;
-import java.io.PrintStream;
import java.util.Collection;
import java.util.HashMap;
import java.util.IdentityHashMap;
@@ -29,82 +30,126 @@
/**
* A pass to rename methods using common, short names.
- * <p>
- * To assign names, we model the scopes of methods names and overloading/shadowing based on the
- * subtyping tree of classes. Such a naming scope is encoded by {@link NamingState}. It keeps
+ *
+ * <p>To assign names, we model the scopes of methods names and overloading/shadowing based on the
+ * subtyping tree of classes. Such a naming scope is encoded by {@link MethodNamingState}. It keeps
* track of its parent node, names that have been reserved (due to keep annotations or otherwise)
* and what names have been used for renaming so far.
- * <p>
- * As in the Dalvik VM method dispatch takes argument and return types of methods into account, we
- * can further reuse names if the prototypes of two methods differ. For this, we store the above
- * state separately for each proto using a map from protos to {@link NamingState.InternalState}
- * objects. These internal state objects are also linked.
- * <p>
- * Name assignment happens in 4 stages. In the first stage, we record all names that are used by
- * library classes or are flagged using a keep rule as reserved. This step also allocates the
- * {@link NamingState} objects for library classes. We can fully allocate these objects as we
- * never perform naming for library classes. For non-library classes, we only allocate a state
- * for the highest non-library class, i.e., we allocate states for every direct subtype of a library
- * class. The states at the boundary between library and program classes are referred to as the
- * frontier states in the code.
- * <p>
- * When reserving names in program classes, we reserve them in the state of the corresponding
+ *
+ * <p>As in the Dalvik VM method dispatch takes argument and return types of methods into account,
+ * we can further reuse names if the prototypes of two methods differ. For this, we store the above
+ * state separately for each proto using a map from protos to {@link
+ * MethodNamingState.InternalState} objects. These internal state objects are also linked.
+ *
+ * <p>Name assignment happens in 4 stages. In the first stage, we record all names that are used by
+ * library classes or are flagged using a keep rule as reserved. This step also allocates the {@link
+ * MethodNamingState} objects for library classes. We can fully allocate these objects as we never
+ * perform naming for library classes. For non-library classes, we only allocate a state for the
+ * highest non-library class, i.e., we allocate states for every direct subtype of a library class.
+ * The states at the boundary between library and program classes are referred to as the frontier
+ * states in the code.
+ *
+ * <p>When reserving names in program classes, we reserve them in the state of the corresponding
* frontier class. This is to ensure that the names are not used for renaming in any supertype.
* Thus, they will still be available in the subtype where they are reserved. Note that name
* reservation only blocks names from being used for minification. We assume that the input program
* is correctly named.
- * <p>
- * In stage 2, we reserve names that stem from interfaces. These are not propagated to
+ *
+ * <p>In stage 2, we reserve names that stem from interfaces. These are not propagated to
* subinterfaces or implementing classes. Instead, stage 3 makes sure to query related states when
* making naming decisions.
- * <p>
- * In stage 3, we compute minified names for all interface methods. We do this first to reduce
+ *
+ * <p>In stage 3, we compute minified names for all interface methods. We do this first to reduce
* assignment conflicts. Interfaces do not build a tree-like inheritance structure we can exploit.
* Thus, we have to infer the structure on the fly. For this, we compute a sets of reachable
- * interfaces. i.e., interfaces that are related via subtyping. Based on these sets, we then
- * find, for each method signature, the classes and interfaces this method signature is defined in.
- * For classes, as we still use frontier states at this point, we do not have to consider subtype
+ * interfaces. i.e., interfaces that are related via subtyping. Based on these sets, we then find,
+ * for each method signature, the classes and interfaces this method signature is defined in. For
+ * classes, as we still use frontier states at this point, we do not have to consider subtype
* relations. For interfaces, we reserve the name in all reachable interfaces and thus ensure
* availability.
- * <p>
- * Name assignment in this phase is a search over all impacted naming states. Using the naming state
- * of the interface this method first originated from, we propose names until we find a matching
- * one. We use the naming state of the interface to not impact name availability in naming states of
- * classes. Hence, skipping over names during interface naming does not impact their availability in
- * the next phase.
- * <p>
- * In the final stage, we assign names to methods by traversing the subtype tree, now allocating
+ *
+ * <p>Name assignment in this phase is a search over all impacted naming states. Using the naming
+ * state of the interface this method first originated from, we propose names until we find a
+ * matching one. We use the naming state of the interface to not impact name availability in naming
+ * states of classes. Hence, skipping over names during interface naming does not impact their
+ * availability in the next phase.
+ *
+ * <p>In the final stage, we assign names to methods by traversing the subtype tree, now allocating
* separate naming states for each class starting from the frontier. In the first swoop, we allocate
* all non-private methods, updating naming states accordingly. In a second swoop, we then allocate
* private methods, as those may safely use names that are used by a public method further down in
* the subtyping tree.
- * <p>
- * Finally, the computed renamings are returned as a map from {@link DexMethod} to
- * {@link DexString}. The MethodNameMinifier object should not be retained to ensure all
- * intermediate state is freed.
- * <p>
- * TODO(herhut): Currently, we do not minify members of annotation interfaces, as this would require
- * parsing and minification of the string arguments to annotations.
+ *
+ * <p>Finally, the computed renamings are returned as a map from {@link DexMethod} to {@link
+ * DexString}. The MethodNameMinifier object should not be retained to ensure all intermediate state
+ * is freed.
+ *
+ * <p>TODO(b/130338621): Currently, we do not minify members of annotation interfaces, as this would
+ * require parsing and minification of the string arguments to annotations.
*/
-class MethodNameMinifier extends MemberNameMinifier<DexMethod, DexProto> {
+class MethodNameMinifier {
- private final Equivalence<DexMethod> equivalence;
+ // A class that provides access to the minification state. An instance of this class is passed
+ // from the method name minifier to the interface method name minifier.
+ class State {
- private final FrontierState frontierState = new FrontierState();
- private final MemberNamingStrategy strategy;
+ DexString getRenaming(DexMethod key) {
+ return renaming.get(key);
+ }
- MethodNameMinifier(AppView<AppInfoWithLiveness> appView, MemberNamingStrategy strategy) {
- super(appView, strategy);
- this.strategy = strategy;
- equivalence =
- overloadAggressively
- ? MethodSignatureEquivalence.get()
- : MethodJavaSignatureEquivalence.get();
+ void putRenaming(DexMethod key, DexString value) {
+ renaming.put(key, value);
+ }
+
+ MethodNamingState<?> getState(DexType type) {
+ return states.get(type);
+ }
+
+ DexType getStateKey(MethodNamingState<?> state) {
+ return states.inverse().get(state);
+ }
+
+ boolean isReservedInGlobalState(DexString name, DexProto state) {
+ return globalState.isReserved(name, state);
+ }
}
- @Override
- Function<DexProto, ?> getKeyTransform() {
- if (overloadAggressively) {
+ private final AppView<AppInfoWithLiveness> appView;
+ private final Equivalence<DexMethod> equivalence;
+ private final MemberNamingStrategy strategy;
+
+ private final Map<DexMethod, DexString> renaming = new IdentityHashMap<>();
+ private final MethodNamingState<?> globalState;
+
+ private final State minifierState = new State();
+ private final FrontierState frontierState = new FrontierState();
+
+ // The use of a bidirectional map allows us to map a naming state to the type it represents,
+ // which is useful for debugging.
+ private final BiMap<DexType, MethodNamingState<?>> states = HashBiMap.create();
+
+ MethodNameMinifier(AppView<AppInfoWithLiveness> appView, MemberNamingStrategy strategy) {
+ this.appView = appView;
+ this.equivalence =
+ appView.options().getProguardConfiguration().isOverloadAggressively()
+ ? MethodSignatureEquivalence.get()
+ : MethodJavaSignatureEquivalence.get();
+ this.globalState = MethodNamingState.createRoot(appView, getKeyTransform(), strategy);
+ this.strategy = strategy;
+ }
+
+ private MethodNamingState<?> computeStateIfAbsent(
+ DexType type, Function<DexType, MethodNamingState<?>> f) {
+ return states.computeIfAbsent(type, f);
+ }
+
+ private boolean alwaysReserveMemberNames(DexClass holder) {
+ return !appView.options().getProguardConfiguration().hasApplyMappingFile()
+ && holder.isNotProgramClass();
+ }
+
+ private Function<DexProto, ?> getKeyTransform() {
+ if (appView.options().getProguardConfiguration().isOverloadAggressively()) {
// Use the full proto as key, hence reuse names based on full signature.
return a -> a;
} else {
@@ -162,7 +207,7 @@
boolean shouldAssignName = holder != null && !alwaysReserveMemberNames(holder);
if (shouldAssignName) {
Map<Wrapper<DexMethod>, DexString> renamingAtThisLevel = new HashMap<>();
- NamingState<DexProto, ?> state =
+ MethodNamingState<?> state =
computeStateIfAbsent(type, k -> minifierState.getState(holder.superType).createChild());
for (DexEncodedMethod method : holder.allMethodsSorted()) {
assignNameToMethod(method, state, renamingAtThisLevel, doPrivates);
@@ -182,7 +227,7 @@
private void assignNameToMethod(
DexEncodedMethod encodedMethod,
- NamingState<DexProto, ?> state,
+ MethodNamingState<?> state,
Map<Wrapper<DexMethod>, DexString> renamingAtThisLevel,
boolean doPrivates) {
if (encodedMethod.accessFlags.isPrivate() != doPrivates) {
@@ -207,8 +252,10 @@
}
private void reserveNamesInClasses(
- DexType type, DexType libraryFrontier, NamingState<DexProto, ?> parent) {
- NamingState<DexProto, ?> state =
+ DexType type, DexType libraryFrontier, MethodNamingState<?> parent) {
+ assert appView.isInterface(type).isFalse();
+
+ MethodNamingState<?> state =
frontierState.allocateNamingStateAndReserve(type, libraryFrontier, parent);
// If this is a library class (or effectively a library class as it is missing) move the
@@ -224,19 +271,18 @@
private final Map<DexType, DexType> frontiers = new IdentityHashMap<>();
- NamingState<DexProto, ?> allocateNamingStateAndReserve(
- DexType type, DexType frontier, NamingState<DexProto, ?> parent) {
+ MethodNamingState<?> allocateNamingStateAndReserve(
+ DexType type, DexType frontier, MethodNamingState<?> parent) {
if (frontier != type) {
frontiers.put(type, frontier);
}
- NamingState<DexProto, ?> state =
+ MethodNamingState<?> state =
computeStateIfAbsent(
frontier,
ignore ->
parent == null
- ? NamingState.createRoot(
- appView.dexItemFactory(), dictionary, getKeyTransform(), strategy)
+ ? MethodNamingState.createRoot(appView, getKeyTransform(), strategy)
: parent.createChild());
DexClass holder = appView.definitionFor(type);
@@ -256,7 +302,7 @@
return state;
}
- private void reserveNamesForMethod(DexMethod method, NamingState<DexProto, ?> state) {
+ private void reserveNamesForMethod(DexMethod method, MethodNamingState<?> state) {
state.reserveName(method.name, method.proto);
globalState.reserveName(method.name, method.proto);
}
@@ -271,65 +317,6 @@
}
}
- /**
- * Capture a (name, proto)-pair for a {@link NamingState}. Each method methodState.METHOD(...)
- * simply defers to parent.METHOD(name, proto, ...). This allows R8 to assign the same name to
- * methods with different prototypes, which is needed for multi-interface lambdas.
- */
- static class MethodNamingState {
-
- private final NamingState<DexProto, ?> parent;
- private final DexString name;
- private final DexProto proto;
- private final DexMethod method;
-
- MethodNamingState(
- NamingState<DexProto, ?> parent, DexMethod method, DexString name, DexProto proto) {
- this.method = method;
- assert parent != null;
- this.parent = parent;
- this.name = name;
- this.proto = proto;
- }
-
- DexString assignNewName() {
- return parent.assignNewNameFor(method, name, proto);
- }
-
- boolean isReserved() {
- return parent.isReserved(name, proto);
- }
-
- boolean isAvailable(DexString candidate) {
- return parent.isAvailable(proto, candidate);
- }
-
- void addRenaming(DexString newName) {
- parent.addRenaming(name, proto, newName);
- }
-
- DexString getName() {
- return name;
- }
-
- DexProto getProto() {
- return proto;
- }
-
- void print(
- String indentation,
- Function<NamingState<DexProto, ?>, DexType> stateKeyGetter,
- PrintStream out) {
- DexType stateKey = stateKeyGetter.apply(parent);
- out.print(indentation);
- out.print(stateKey != null ? stateKey.toSourceString() : "<?>");
- out.print(".");
- out.print(name.toSourceString());
- out.println(proto.toSmaliString());
- parent.printState(proto, stateKeyGetter, indentation + " ", out);
- }
- }
-
// Shuffles the given methods if assertions are enabled and deterministic debugging is disabled.
// Used to ensure that the generated output is deterministic.
static Iterable<DexEncodedMethod> shuffleMethods(
diff --git a/src/main/java/com/android/tools/r8/naming/NamingState.java b/src/main/java/com/android/tools/r8/naming/MethodNamingState.java
similarity index 80%
rename from src/main/java/com/android/tools/r8/naming/NamingState.java
rename to src/main/java/com/android/tools/r8/naming/MethodNamingState.java
index 4a4be05..ada0dfa 100644
--- a/src/main/java/com/android/tools/r8/naming/NamingState.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNamingState.java
@@ -5,12 +5,13 @@
import static com.android.tools.r8.naming.Minifier.MinifierMemberNamingStrategy.EMPTY_CHAR_ARRAY;
-import com.android.tools.r8.graph.CachedHashValueDexItem;
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexItemFactory;
-import com.android.tools.r8.graph.DexReference;
+import com.android.tools.r8.graph.DexMethod;
+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.naming.MemberNameMinifier.MemberNamingStrategy;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.Sets;
@@ -18,44 +19,39 @@
import java.io.PrintStream;
import java.util.HashMap;
import java.util.Iterator;
-import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.function.Function;
-class NamingState<ProtoType extends CachedHashValueDexItem, KeyType> {
+class MethodNamingState<KeyType> {
- private final NamingState<ProtoType, KeyType> parent;
+ private final AppView<? extends AppInfo> appView;
+ private final MethodNamingState<KeyType> parent;
private final Map<KeyType, InternalState> usedNames = new HashMap<>();
- private final DexItemFactory itemFactory;
- private final List<String> dictionary;
- private final Function<ProtoType, KeyType> keyTransform;
+ private final Function<DexProto, KeyType> keyTransform;
private final MemberNamingStrategy strategy;
- static <S, T extends CachedHashValueDexItem> NamingState<T, S> createRoot(
- DexItemFactory itemFactory,
- List<String> dictionary,
- Function<T, S> keyTransform,
+ static <S> MethodNamingState<S> createRoot(
+ AppView<? extends AppInfo> appView,
+ Function<DexProto, S> keyTransform,
MemberNamingStrategy strategy) {
- return new NamingState<>(null, itemFactory, dictionary, keyTransform, strategy);
+ return new MethodNamingState<>(null, appView, keyTransform, strategy);
}
- private NamingState(
- NamingState<ProtoType, KeyType> parent,
- DexItemFactory itemFactory,
- List<String> dictionary,
- Function<ProtoType, KeyType> keyTransform,
+ private MethodNamingState(
+ MethodNamingState<KeyType> parent,
+ AppView<? extends AppInfo> appView,
+ Function<DexProto, KeyType> keyTransform,
MemberNamingStrategy strategy) {
+ this.appView = appView;
this.parent = parent;
- this.itemFactory = itemFactory;
- this.dictionary = dictionary;
this.keyTransform = keyTransform;
this.strategy = strategy;
}
- public NamingState<ProtoType, KeyType> createChild() {
- return new NamingState<>(this, itemFactory, dictionary, keyTransform, strategy);
+ public MethodNamingState<KeyType> createChild() {
+ return new MethodNamingState<>(this, appView, keyTransform, strategy);
}
private InternalState findInternalStateFor(KeyType key) {
@@ -71,7 +67,7 @@
InternalState result = usedNames.get(key);
if (result == null) {
InternalState parentState = parent != null ? parent.getOrCreateInternalStateFor(key) : null;
- result = new InternalState(itemFactory, parentState, dictionary);
+ result = new InternalState(appView, parentState);
usedNames.put(key, result);
}
return result;
@@ -85,7 +81,7 @@
return state.getAssignedNameFor(name, key);
}
- public DexString assignNewNameFor(DexReference source, DexString original, ProtoType proto) {
+ public DexString assignNewNameFor(DexMethod source, DexString original, DexProto proto) {
KeyType key = keyTransform.apply(proto);
DexString result = getAssignedNameFor(original, key);
if (result == null) {
@@ -95,13 +91,13 @@
return result;
}
- public void reserveName(DexString name, ProtoType proto) {
+ public void reserveName(DexString name, DexProto proto) {
KeyType key = keyTransform.apply(proto);
InternalState state = getOrCreateInternalStateFor(key);
state.reserveName(name);
}
- public boolean isReserved(DexString name, ProtoType proto) {
+ public boolean isReserved(DexString name, DexProto proto) {
KeyType key = keyTransform.apply(proto);
InternalState state = findInternalStateFor(key);
if (state == null) {
@@ -110,7 +106,7 @@
return state.isReserved(name);
}
- public boolean isAvailable(ProtoType proto, DexString candidate) {
+ public boolean isAvailable(DexProto proto, DexString candidate) {
KeyType key = keyTransform.apply(proto);
InternalState state = findInternalStateFor(key);
if (state == null) {
@@ -119,15 +115,15 @@
return state.isAvailable(candidate);
}
- public void addRenaming(DexString original, ProtoType proto, DexString newName) {
+ public void addRenaming(DexString original, DexProto proto, DexString newName) {
KeyType key = keyTransform.apply(proto);
InternalState state = getOrCreateInternalStateFor(key);
state.addRenaming(original, key, newName);
}
void printState(
- ProtoType proto,
- Function<NamingState<ProtoType, ?>, DexType> stateKeyGetter,
+ DexProto proto,
+ Function<MethodNamingState<?>, DexType> stateKeyGetter,
String indentation,
PrintStream out) {
KeyType key = keyTransform.apply(proto);
@@ -170,9 +166,11 @@
this.dictionaryIterator = dictionaryIterator;
}
- private InternalState(
- DexItemFactory itemFactory, InternalState parentInternalState, List<String> dictionary) {
- this(itemFactory, parentInternalState, dictionary.iterator());
+ private InternalState(AppView<? extends AppInfo> appView, InternalState parentInternalState) {
+ this(
+ appView.dexItemFactory(),
+ parentInternalState,
+ appView.options().getProguardConfiguration().getObfuscationDictionary().iterator());
}
private boolean isReserved(DexString name) {
@@ -219,7 +217,7 @@
return result;
}
- private DexString getNewNameFor(DexReference source) {
+ private DexString getNewNameFor(DexMethod source) {
DexString name;
do {
name = nextSuggestedName(source);
@@ -234,7 +232,7 @@
renamings.put(original, proto, newName);
}
- DexString nextSuggestedName(DexReference source) {
+ DexString nextSuggestedName(DexMethod source) {
if (!strategy.bypassDictionary() && dictionaryIterator.hasNext()) {
return itemFactory.createString(dictionaryIterator.next());
} else {
@@ -243,11 +241,11 @@
}
void printInternalState(
- NamingState<ProtoType, ?> expectedNamingState,
- Function<NamingState<ProtoType, ?>, DexType> stateKeyGetter,
+ MethodNamingState<?> expectedNamingState,
+ Function<MethodNamingState<?>, DexType> stateKeyGetter,
String indentation,
PrintStream out) {
- assert expectedNamingState == NamingState.this;
+ assert expectedNamingState == MethodNamingState.this;
DexType stateKey = stateKeyGetter.apply(expectedNamingState);
out.print(indentation);
diff --git a/src/main/java/com/android/tools/r8/naming/Minifier.java b/src/main/java/com/android/tools/r8/naming/Minifier.java
index f01665a..e9dc45b 100644
--- a/src/main/java/com/android/tools/r8/naming/Minifier.java
+++ b/src/main/java/com/android/tools/r8/naming/Minifier.java
@@ -6,7 +6,9 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
@@ -15,9 +17,7 @@
import com.android.tools.r8.naming.ClassNameMinifier.Namespace;
import com.android.tools.r8.naming.ClassNameMinifier.PackageNamingStrategy;
import com.android.tools.r8.naming.FieldNameMinifier.FieldRenaming;
-import com.android.tools.r8.naming.MemberNameMinifier.MemberNamingStrategy;
import com.android.tools.r8.naming.MethodNameMinifier.MethodRenaming;
-import com.android.tools.r8.naming.NamingState.InternalState;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.Timing;
@@ -151,12 +151,17 @@
}
@Override
- public DexString next(DexReference dexReference, InternalState internalState) {
+ public DexString next(DexMethod method, MethodNamingState.InternalState internalState) {
int counter = internalState.incrementAndGet();
return factory.createString(StringUtils.numberToIdentifier(EMPTY_CHAR_ARRAY, counter, false));
}
@Override
+ public DexString next(DexField field, FieldNamingState.InternalState internalState) {
+ return internalState.nextNameAccordingToState();
+ }
+
+ @Override
public boolean bypassDictionary() {
return false;
}
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 3a62413..4796cb9 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
@@ -18,13 +18,11 @@
import com.android.tools.r8.naming.ClassNameMinifier.ClassRenaming;
import com.android.tools.r8.naming.ClassNameMinifier.Namespace;
import com.android.tools.r8.naming.FieldNameMinifier.FieldRenaming;
-import com.android.tools.r8.naming.MemberNameMinifier.MemberNamingStrategy;
import com.android.tools.r8.naming.MemberNaming.FieldSignature;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.naming.MemberNaming.Signature;
import com.android.tools.r8.naming.MethodNameMinifier.MethodRenaming;
import com.android.tools.r8.naming.Minifier.MinificationPackageNamingStrategy;
-import com.android.tools.r8.naming.NamingState.InternalState;
import com.android.tools.r8.position.Position;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.Reporter;
@@ -241,7 +239,16 @@
}
@Override
- public DexString next(DexReference reference, InternalState internalState) {
+ public DexString next(DexMethod method, MethodNamingState.InternalState internalState) {
+ return next(method);
+ }
+
+ @Override
+ public DexString next(DexField field, FieldNamingState.InternalState internalState) {
+ return next(field);
+ }
+
+ private DexString next(DexReference reference) {
if (mappedNames.containsKey(reference)) {
return factory.createString(mappedNames.get(reference).getRenamedName());
} else if (reference.isDexMethod()) {
diff --git a/src/main/java/com/android/tools/r8/naming/ReservedFieldNamingState.java b/src/main/java/com/android/tools/r8/naming/ReservedFieldNamingState.java
new file mode 100644
index 0000000..11b17bf
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/naming/ReservedFieldNamingState.java
@@ -0,0 +1,76 @@
+// 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.naming;
+
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.naming.ReservedFieldNamingState.InternalState;
+import com.google.common.collect.Sets;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.Set;
+
+public class ReservedFieldNamingState extends FieldNamingStateBase<InternalState> {
+
+ public ReservedFieldNamingState(AppView<? extends AppInfo> appView) {
+ super(appView, new IdentityHashMap<>());
+ }
+
+ public boolean isReserved(DexString name, DexType type) {
+ InternalState internalState = getInternalState(type);
+ return internalState != null && internalState.isReserved(name);
+ }
+
+ public void markReservedDirectly(DexField field) {
+ markReservedDirectly(field.name, field.type);
+ }
+
+ public void markReservedDirectly(DexString name, DexType type) {
+ getOrCreateInternalState(type).markReservedDirectly(name);
+ }
+
+ public void includeReservations(ReservedFieldNamingState reservedNames) {
+ for (Map.Entry<DexType, InternalState> entry : reservedNames.internalStates.entrySet()) {
+ getOrCreateInternalState(entry.getKey()).includeReservations(entry.getValue());
+ }
+ }
+
+ public void includeReservationsFromBelow(ReservedFieldNamingState reservedNames) {
+ for (Map.Entry<DexType, InternalState> entry : reservedNames.internalStates.entrySet()) {
+ getOrCreateInternalState(entry.getKey()).includeReservationsFromBelow(entry.getValue());
+ }
+ }
+
+ @Override
+ InternalState createInternalState() {
+ return new InternalState();
+ }
+
+ static class InternalState {
+
+ private Set<DexString> reservedNamesDirect = Sets.newIdentityHashSet();
+ private Set<DexString> reservedNamesBelow = Sets.newIdentityHashSet();
+
+ public boolean isReserved(DexString name) {
+ return reservedNamesDirect.contains(name) || reservedNamesBelow.contains(name);
+ }
+
+ public void markReservedDirectly(DexString name) {
+ reservedNamesDirect.add(name);
+ }
+
+ public void includeReservations(InternalState state) {
+ reservedNamesDirect.addAll(state.reservedNamesDirect);
+ }
+
+ public void includeReservationsFromBelow(InternalState state) {
+ reservedNamesBelow.addAll(state.reservedNamesDirect);
+ reservedNamesBelow.addAll(state.reservedNamesBelow);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/InterfaceFieldMinificationTest.java b/src/test/java/com/android/tools/r8/naming/InterfaceFieldMinificationTest.java
index d5f32e7..2041fac 100644
--- a/src/test/java/com/android/tools/r8/naming/InterfaceFieldMinificationTest.java
+++ b/src/test/java/com/android/tools/r8/naming/InterfaceFieldMinificationTest.java
@@ -4,13 +4,12 @@
package com.android.tools.r8.naming;
-import static org.hamcrest.CoreMatchers.containsString;
-
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NeverMerge;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.naming.testclasses.Greeting;
+import com.android.tools.r8.utils.StringUtils;
import org.junit.Test;
/** Regression test for b/128600647. */
@@ -18,6 +17,7 @@
@Test
public void test() throws Exception {
+ String expectedOutput = StringUtils.lines("Greeter: Hello world!");
testForR8(Backend.DEX)
.addProgramClasses(
TestClass.class, Greeter.class, Greeting.class, Greeting.getGreetingBase(), Tag.class)
@@ -27,7 +27,7 @@
.enableInliningAnnotations()
.enableMergeAnnotations()
.run(TestClass.class)
- .assertFailureWithErrorThatMatches(containsString("IncompatibleClassChangeError"));
+ .assertSuccessWithOutput(expectedOutput);
}
static class TestClass {
diff --git a/src/test/java/com/android/tools/r8/naming/OverloadedReservedFieldNamingTest.java b/src/test/java/com/android/tools/r8/naming/OverloadedReservedFieldNamingTest.java
new file mode 100644
index 0000000..2d0ea1c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/OverloadedReservedFieldNamingTest.java
@@ -0,0 +1,93 @@
+// 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.naming;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+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 com.android.tools.r8.utils.codeinspector.FieldSubject;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class OverloadedReservedFieldNamingTest extends TestBase {
+
+ private final boolean overloadAggressively;
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{1}, overload aggressively: {0}")
+ public static List<Object[]> data() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ }
+
+ public OverloadedReservedFieldNamingTest(
+ boolean overloadAggressively, TestParameters parameters) {
+ this.overloadAggressively = overloadAggressively;
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(OverloadedReservedFieldNamingTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepRules(
+ "-keep class " + A.class.getTypeName() + " { boolean a; }",
+ overloadAggressively ? "-overloadaggressively" : "")
+ .enableMergeAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::verifyAggressiveOverloading)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(StringUtils.lines("Hello world!"));
+ }
+
+ private void verifyAggressiveOverloading(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(A.class);
+ assertThat(classSubject, isPresent());
+
+ FieldSubject fieldSubject = classSubject.uniqueFieldWithName("a");
+ assertThat(fieldSubject, isPresent());
+ assertThat(fieldSubject, not(isRenamed()));
+
+ FieldSubject helloFieldSubject = classSubject.uniqueFieldWithName("hello");
+ assertThat(helloFieldSubject, isPresent());
+ assertEquals(overloadAggressively ? "a" : "b", helloFieldSubject.getFinalName());
+
+ FieldSubject worldFieldSubject = classSubject.uniqueFieldWithName("world");
+ assertThat(worldFieldSubject, isPresent());
+ assertEquals(overloadAggressively ? "a" : "c", worldFieldSubject.getFinalName());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(new A());
+ }
+ }
+
+ static class A {
+
+ static final boolean a = System.currentTimeMillis() >= 0;
+ static final String hello = System.currentTimeMillis() >= 0 ? "Hello" : null;
+ static final Object world = System.currentTimeMillis() >= 0 ? " world!" : null;
+
+ @Override
+ public String toString() {
+ return a ? (hello + world) : null;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInInterfacesWithCommonSubClassTest.java b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInInterfacesWithCommonSubClassTest.java
index 535e95f..e93b0fa 100644
--- a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInInterfacesWithCommonSubClassTest.java
+++ b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInInterfacesWithCommonSubClassTest.java
@@ -5,7 +5,10 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.AnyOf.anyOf;
+import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
import com.android.tools.r8.NeverMerge;
import com.android.tools.r8.TestBase;
@@ -73,13 +76,12 @@
assertThat(aFieldSubject, isPresent());
if (reserveName) {
- assertEquals("a", f1FieldSubject.getFinalName());
- // TODO(b/128973195): J.a should not be renamed to the same as I.f1.
+ assertEquals("b", f1FieldSubject.getFinalName());
assertEquals("a", aFieldSubject.getFinalName());
} else {
- // TODO(b/128973195): I.f1 Should not be renamed to the same as J.a.
- assertEquals("a", f1FieldSubject.getFinalName());
- assertEquals("a", aFieldSubject.getFinalName());
+ assertThat(f1FieldSubject.getFinalName(), anyOf(is("a"), is("b")));
+ assertThat(aFieldSubject.getFinalName(), anyOf(is("a"), is("b")));
+ assertNotEquals(aFieldSubject.getFinalName(), f1FieldSubject.getFinalName());
}
}
diff --git a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSubClassTest.java b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSubClassTest.java
index 5009360..77190ce 100644
--- a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSubClassTest.java
+++ b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSubClassTest.java
@@ -99,51 +99,41 @@
assertThat(aFieldSubject, isPresent());
if (reserveName) {
- // TODO(b/128973195): A.f1 should be renamed to e instead of a.
assertThat(f1FieldSubject, isRenamed());
- assertEquals("a", f1FieldSubject.getFinalName());
+ assertEquals("e", f1FieldSubject.getFinalName());
- // TODO(b/128973195): B.f2 should be renamed to f instead of b.
assertThat(f2FieldSubject, isRenamed());
- assertEquals("b", f2FieldSubject.getFinalName());
+ assertEquals("f", f2FieldSubject.getFinalName());
- // TODO(b/128973195): I.f3 should be renamed to b instead of a.
assertThat(f3FieldSubject, isRenamed());
- assertEquals("a", f3FieldSubject.getFinalName());
+ assertEquals("b", f3FieldSubject.getFinalName());
- // TODO(b/128973195): J.f4 should be renamed to c instead of b.
assertThat(f4FieldSubject, isRenamed());
- assertEquals("b", f4FieldSubject.getFinalName());
+ assertEquals("c", f4FieldSubject.getFinalName());
- // TODO(b/128973195): K.f5 should be renamed to d instead of a.
assertThat(f5FieldSubject, isRenamed());
- assertEquals("a", f5FieldSubject.getFinalName());
+ assertEquals("d", f5FieldSubject.getFinalName());
// B.a should not be renamed because it is not allowed to be minified.
assertThat(aFieldSubject, not(isRenamed()));
} else {
- // TODO(b/128973195): A.f1 should be renamed to d instead of a.
assertThat(f1FieldSubject, isRenamed());
- assertEquals("a", f1FieldSubject.getFinalName());
+ assertEquals("d", f1FieldSubject.getFinalName());
- // TODO(b/128973195): B.f2 should be renamed to e instead of b.
assertThat(f2FieldSubject, isRenamed());
- assertEquals("b", f2FieldSubject.getFinalName());
+ assertEquals("e", f2FieldSubject.getFinalName());
- // TODO(b/128973195): I.f3 should be renamed to a instead of a.
assertThat(f3FieldSubject, isRenamed());
assertEquals("a", f3FieldSubject.getFinalName());
assertThat(f4FieldSubject, isRenamed());
assertEquals("b", f4FieldSubject.getFinalName());
- // TODO(b/128973195): K.f5 should be renamed to c instead of a.
assertThat(f5FieldSubject, isRenamed());
- assertEquals("a", f5FieldSubject.getFinalName());
+ assertEquals("c", f5FieldSubject.getFinalName());
- // TODO(b/128973195): C.a should be renamed to f instead of c.
assertThat(aFieldSubject, isRenamed());
- assertEquals("c", aFieldSubject.getFinalName());
+ assertEquals("f", aFieldSubject.getFinalName());
}
}
diff --git a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSubInterfaceTest.java b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSubInterfaceTest.java
index a60537d..057ba0c 100644
--- a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSubInterfaceTest.java
+++ b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSubInterfaceTest.java
@@ -77,8 +77,7 @@
// Interface fields are visited/renamed before fields on classes. Thus, the interface field
// I.f1 will be visited first and assigned the name b (since the name a is reserved).
- // TODO(b/128973195): Should be renamed to b instead of a.
- assertEquals("a", f1FieldSubject.getFinalName());
+ assertEquals("b", f1FieldSubject.getFinalName());
} else {
// Interface fields are visited/renamed before fields on classes. Thus, the interface field
@@ -91,7 +90,7 @@
assertEquals("b", aFieldSubject.getFinalName());
}
- inspect(inspector);
+ inspect(inspector, "c");
}
@Test
@@ -110,10 +109,10 @@
testForD8().addProgramClasses(I.class, J.class).compile().writeToZip())
.run(TestClass.class)
.assertSuccessWithOutput(expectedOutput)
- .inspect(this::inspect);
+ .inspect(inspector -> inspect(inspector, "b"));
}
- private void inspect(CodeInspector inspector) {
+ private void inspect(CodeInspector inspector, String expectedNameForF2) {
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
@@ -121,8 +120,7 @@
assertThat(f2FieldSubject, isPresent());
assertThat(f2FieldSubject, isRenamed());
- // TODO(b/128973195): A.f2 should be renamed to c.
- assertEquals("a", f2FieldSubject.getFinalName());
+ assertEquals(expectedNameForF2, f2FieldSubject.getFinalName());
}
@NeverMerge
diff --git a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSuperInterfaceTest.java b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSuperInterfaceTest.java
index f19a00d..c349aed 100644
--- a/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSuperInterfaceTest.java
+++ b/src/test/java/com/android/tools/r8/naming/ReservedFieldNameInSuperInterfaceTest.java
@@ -103,9 +103,7 @@
FieldSubject f2FieldSubject = aClassSubject.uniqueFieldWithName("f2");
assertThat(f2FieldSubject, isPresent());
assertThat(f2FieldSubject, isRenamed());
-
- // TODO(b/128973195): A.f2 should be renamed to c instead of a.
- assertEquals("a", f2FieldSubject.getFinalName());
+ assertEquals("c", f2FieldSubject.getFinalName());
}
interface I {
diff --git a/src/test/java/com/android/tools/r8/naming/applymapping/sourcelibrary/MemberResolutionTest.java b/src/test/java/com/android/tools/r8/naming/applymapping/sourcelibrary/MemberResolutionTest.java
index 99fc246..dc1fe84 100644
--- a/src/test/java/com/android/tools/r8/naming/applymapping/sourcelibrary/MemberResolutionTest.java
+++ b/src/test/java/com/android/tools/r8/naming/applymapping/sourcelibrary/MemberResolutionTest.java
@@ -5,15 +5,15 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-import com.android.tools.r8.R8Command;
+import com.android.tools.r8.NeverMerge;
import com.android.tools.r8.TestBase;
-import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.origin.Origin;
-import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.utils.FileUtils;
+import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.FieldSubject;
@@ -26,6 +26,7 @@
import org.junit.runners.Parameterized;
// AbstractChecker -> X:
+@NeverMerge
abstract class AbstractChecker {
// String tag -> p
private String tag = "PrivateInitialTag_AbstractChecker";
@@ -74,19 +75,16 @@
@RunWith(Parameterized.class)
public class MemberResolutionTest extends TestBase {
- private static final List<Class<?>> CLASSES =
- ImmutableList.of(
- AbstractChecker.class, ConcreteChecker.class, MemberResolutionTestMain.class);
- private Backend backend;
+ private final TestParameters parameters;
- @Parameterized.Parameters(name = "Backend: {0}")
- public static Backend[] data() {
- return ToolHelper.getBackends();
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
}
- public MemberResolutionTest(Backend backend) {
- this.backend = backend;
+ public MemberResolutionTest(TestParameters parameters) {
+ this.parameters = parameters;
}
@Test
@@ -105,31 +103,33 @@
" void foo() -> a");
FileUtils.writeTextFile(mapPath, pgMap);
- AndroidApp app = readClasses(CLASSES);
- R8Command.Builder builder = ToolHelper.prepareR8CommandBuilder(app, emptyConsumer(backend));
- builder
- .addProguardConfiguration(
- ImmutableList.of(
- keepMainProguardConfiguration(MemberResolutionTestMain.class),
- // Do not turn on -allowaccessmodification
- "-applymapping " + mapPath,
- "-dontobfuscate"), // to use the renamed names in test-mapping.txt
- Origin.unknown())
- .addLibraryFiles(runtimeJar(backend));
- AndroidApp processedApp =
- ToolHelper.runR8(
- builder.build(),
- options -> {
- options.enableInlining = false;
- options.enableVerticalClassMerging = false;
- });
+ String expectedOutput =
+ StringUtils.lines(
+ "AbstractChecker#check:PrivateInitialTag_AbstractChecker",
+ "ConcreteChecker#check:NewTag");
- String outputBefore = runOnJava(MemberResolutionTestMain.class);
- String outputAfter = runOnVM(processedApp, MemberResolutionTestMain.class, backend);
- assertEquals(outputBefore, outputAfter);
+ if (parameters.getBackend() == Backend.CF) {
+ testForJvm()
+ .addTestClasspath()
+ .run(parameters.getRuntime(), MemberResolutionTestMain.class)
+ .assertSuccessWithOutput(expectedOutput);
+ }
- CodeInspector codeInspector = new CodeInspector(processedApp, mapPath);
- ClassSubject base = codeInspector.clazz(AbstractChecker.class);
+ CodeInspector inspector =
+ testForR8(parameters.getBackend())
+ .addProgramClasses(
+ AbstractChecker.class, ConcreteChecker.class, MemberResolutionTestMain.class)
+ .addKeepMainRule(MemberResolutionTestMain.class)
+ .addKeepRules("-applymapping " + mapPath)
+ .enableMergeAnnotations()
+ .noMinification()
+ .addOptionsModification(options -> options.enableInlining = false)
+ .setMinApi(parameters.getRuntime())
+ .run(parameters.getRuntime(), MemberResolutionTestMain.class)
+ .assertSuccessWithOutput(expectedOutput)
+ .inspector();
+
+ ClassSubject base = inspector.clazz(AbstractChecker.class);
assertThat(base, isPresent());
FieldSubject p = base.field("java.lang.String", "tag");
assertThat(p, isPresent());
@@ -140,7 +140,7 @@
assertThat(x, isRenamed());
assertEquals("x", x.getFinalName());
- ClassSubject sub = codeInspector.clazz(ConcreteChecker.class);
+ ClassSubject sub = inspector.clazz(ConcreteChecker.class);
assertThat(sub, isPresent());
FieldSubject q = sub.field("java.lang.String", "tag");
assertThat(q, isPresent());
diff --git a/src/test/java/com/android/tools/r8/naming/b128656974/B128656974.java b/src/test/java/com/android/tools/r8/naming/b128656974/B128656974.java
index 100b359..9a0696b 100644
--- a/src/test/java/com/android/tools/r8/naming/b128656974/B128656974.java
+++ b/src/test/java/com/android/tools/r8/naming/b128656974/B128656974.java
@@ -5,7 +5,6 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
-import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertThat;
@@ -18,12 +17,10 @@
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.FieldSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import org.junit.Ignore;
import org.junit.Test;
public class B128656974 extends TestBase {
- @Ignore("b/128656974")
@Test
public void testField() throws Exception {
Class<?> main = TestClassMainForField.class;
@@ -125,5 +122,4 @@
instance.foo();
}
}
-
}
diff --git a/src/test/java/com/android/tools/r8/shaking/TreeShakingSpecificTest.java b/src/test/java/com/android/tools/r8/shaking/TreeShakingSpecificTest.java
index e17f713..c8b757c 100644
--- a/src/test/java/com/android/tools/r8/shaking/TreeShakingSpecificTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/TreeShakingSpecificTest.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.OutputMode;
import com.android.tools.r8.R8;
import com.android.tools.r8.R8Command;
+import com.android.tools.r8.TestBase.Backend;
import com.android.tools.r8.ToolHelper;
import java.io.BufferedReader;
import java.io.IOException;
@@ -36,10 +37,6 @@
@RunWith(Parameterized.class)
public class TreeShakingSpecificTest {
- enum Backend {
- DEX,
- CF
- }
private Backend backend;
