Unify reservation of field and method names in strategy
This is the final refactoring before adding rewriting the
ProguardMapMinifier to support applymapping.
Change-Id: I7be689851e7a419d1361e2ec07043cbbb1e948bd
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 aeac240..633a179 100644
--- a/src/main/java/com/android/tools/r8/naming/FieldNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/FieldNameMinifier.java
@@ -85,7 +85,7 @@
for (DexClass clazz : appView.appInfo().app().asDirect().allClasses()) {
ReservedFieldNamingState reservedNames = null;
for (DexEncodedField field : clazz.fields()) {
- if (shouldReserveName(clazz, field)) {
+ if (strategy.isReserved(field, clazz)) {
if (reservedNames == null) {
reservedNames = getOrCreateReservedFieldNamingState(clazz.type);
}
@@ -109,17 +109,6 @@
propagateReservedFieldNamesUpwards();
}
- private boolean shouldReserveName(DexClass clazz, DexEncodedField field) {
- if (clazz.isLibraryClass()) {
- return true;
- }
- if (!appView.options().getProguardConfiguration().hasApplyMappingFile()
- && appView.rootSet().mayNotBeMinified(field.field, appView)) {
- return true;
- }
- return false;
- }
-
private void propagateReservedFieldNamesUpwards() {
BottomUpClassHierarchyTraversal.forProgramClasses(appView)
.visit(
diff --git a/src/main/java/com/android/tools/r8/naming/FieldNamingState.java b/src/main/java/com/android/tools/r8/naming/FieldNamingState.java
index d654558..67298e2 100644
--- a/src/main/java/com/android/tools/r8/naming/FieldNamingState.java
+++ b/src/main/java/com/android/tools/r8/naming/FieldNamingState.java
@@ -50,11 +50,7 @@
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().mayNotBeMinified(encodedField.field, appView)) {
+ if (clazz == null || strategy.isReserved(encodedField, clazz)) {
return field.name;
}
}
diff --git a/src/main/java/com/android/tools/r8/naming/MemberNamingStrategy.java b/src/main/java/com/android/tools/r8/naming/MemberNamingStrategy.java
index f9e8ed5..6510d6c 100644
--- a/src/main/java/com/android/tools/r8/naming/MemberNamingStrategy.java
+++ b/src/main/java/com/android/tools/r8/naming/MemberNamingStrategy.java
@@ -4,6 +4,9 @@
package com.android.tools.r8.naming;
+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.DexField;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexReference;
@@ -17,5 +20,9 @@
boolean breakOnNotAvailable(DexReference source, DexString name);
- boolean noObfuscation(DexReference reference);
+ boolean isReserved(DexEncodedMethod method, DexClass holder);
+
+ boolean isReserved(DexEncodedField field, DexClass holder);
+
+ boolean allowMemberRenaming(DexClass holder);
}
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 029f65a..b5b32b2 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
@@ -139,11 +139,6 @@
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.
@@ -207,7 +202,7 @@
//
// A simple way to ensure this is to process virtual methods first and then direct methods.
DexClass holder = appView.definitionFor(type);
- boolean shouldAssignName = holder != null && !alwaysReserveMemberNames(holder);
+ boolean shouldAssignName = holder != null && strategy.allowMemberRenaming(holder);
if (shouldAssignName) {
MethodNamingState<?> state =
computeStateIfAbsent(type, k -> minifierState.getState(holder.superType).createChild());
@@ -218,7 +213,7 @@
assignNameToMethod(method, state);
}
}
- appView.appInfo().forAllExtendsSubtypes(type, subtype -> assignNamesToClassesMethods(subtype));
+ appView.appInfo().forAllExtendsSubtypes(type, this::assignNamesToClassesMethods);
}
private void assignNameToMethod(DexEncodedMethod encodedMethod, MethodNamingState<?> state) {
@@ -276,13 +271,8 @@
DexClass holder = appView.definitionFor(type);
if (holder != null) {
- boolean keepAll = alwaysReserveMemberNames(holder) || holder.accessFlags.isAnnotation();
for (DexEncodedMethod method : shuffleMethods(holder.methods(), appView.options())) {
- // TODO(christofferqa): Wouldn't it be sufficient only to reserve names for non-private
- // methods?
- if (keepAll
- || method.accessFlags.isConstructor()
- || strategy.noObfuscation(method.method)) {
+ if (strategy.isReserved(method, holder)) {
reserveNamesForMethod(method.method, state);
}
}
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 d2424fe..c6ffef4 100644
--- a/src/main/java/com/android/tools/r8/naming/Minifier.java
+++ b/src/main/java/com/android/tools/r8/naming/Minifier.java
@@ -8,6 +8,7 @@
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.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
@@ -167,6 +168,7 @@
@Override
public DexString next(DexMethod method, InternalNamingState internalState) {
+ assert checkAllowMemberRenaming(method.holder);
DexEncodedMethod encodedMethod = appView.definitionFor(method);
boolean isDirectOrStatic = encodedMethod.isDirectMethod() || encodedMethod.isStatic();
return getNextName(internalState, isDirectOrStatic);
@@ -174,6 +176,7 @@
@Override
public DexString next(DexField field, InternalNamingState internalState) {
+ assert checkAllowMemberRenaming(field.holder);
return getNextName(internalState, false);
}
@@ -187,8 +190,29 @@
}
@Override
- public boolean noObfuscation(DexReference reference) {
- return appView.rootSet().mayNotBeMinified(reference, appView);
+ public boolean isReserved(DexEncodedMethod method, DexClass holder) {
+ if (!allowMemberRenaming(holder)
+ || holder.accessFlags.isAnnotation()
+ || method.accessFlags.isConstructor()) {
+ return true;
+ }
+ return appView.rootSet().mayNotBeMinified(method.method, appView);
+ }
+
+ @Override
+ public boolean isReserved(DexEncodedField field, DexClass holder) {
+ return holder.isLibraryClass() || appView.rootSet().mayNotBeMinified(field.field, appView);
+ }
+
+ @Override
+ public boolean allowMemberRenaming(DexClass holder) {
+ return holder.isProgramClass();
+ }
+
+ public boolean checkAllowMemberRenaming(DexType holder) {
+ DexClass clazz = appView.definitionFor(holder);
+ assert clazz != null && allowMemberRenaming(clazz);
+ return true;
}
}
}
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 90a6760..784dc8b 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
@@ -7,6 +7,8 @@
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.DexEncodedField;
+import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
@@ -269,9 +271,18 @@
}
@Override
- public boolean noObfuscation(DexReference reference) {
- // We have an explicit mapping from the proguard map thus everything might have to be renamed.
+ public boolean isReserved(DexEncodedMethod method, DexClass holder) {
return false;
}
+
+ @Override
+ public boolean isReserved(DexEncodedField field, DexClass holder) {
+ return false;
+ }
+
+ @Override
+ public boolean allowMemberRenaming(DexClass holder) {
+ return true;
+ }
}
}
