Merge "Revert "Ensure lookupDescriptor do not introduce duplicate minified types""
diff --git a/src/main/java/com/android/tools/r8/D8CommandParser.java b/src/main/java/com/android/tools/r8/D8CommandParser.java
index 00157f5..3985cd7 100644
--- a/src/main/java/com/android/tools/r8/D8CommandParser.java
+++ b/src/main/java/com/android/tools/r8/D8CommandParser.java
@@ -5,10 +5,12 @@
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.origin.PathOrigin;
+import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.FlagFile;
import com.android.tools.r8.utils.StringDiagnostic;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import java.io.IOException;
import java.nio.file.Files;
@@ -21,6 +23,15 @@
public class D8CommandParser extends BaseCompilerCommandParser {
+ private static final Set<String> OPTIONS_WITH_PARAMETER =
+ ImmutableSet.of(
+ "--output",
+ "--lib",
+ "--classpath",
+ "--min-api",
+ "--main-dex-list",
+ "--main-dex-list-output");
+
private static final String APK_EXTENSION = ".apk";
private static final String JAR_EXTENSION = ".jar";
private static final String ZIP_EXTENSION = ".zip";
@@ -106,13 +117,16 @@
" # <file> must be an existing directory or a zip file.",
" --lib <file> # Add <file> as a library resource.",
" --classpath <file> # Add <file> as a classpath resource.",
- " --min-api # Minimum Android API level compatibility",
+ " --min-api <number> # Minimum Android API level compatibility, default: "
+ + AndroidApiLevel.getDefault().getLevel()
+ + ".",
" --intermediate # Compile an intermediate result intended for later",
" # merging.",
" --file-per-class # Produce a separate dex file per input class",
" --no-desugaring # Force disable desugaring.",
" --main-dex-list <file> # List of classes to place in the primary dex file.",
- " --main-dex-list-output <file> # Output resulting main dex list in <file>.",
+ " --main-dex-list-output <file>",
+ " # Output resulting main dex list in <file>.",
" --version # Print the version of d8.",
" --help # Print this message."));
@@ -153,6 +167,16 @@
String[] expandedArgs = FlagFile.expandFlagFiles(args, builder);
for (int i = 0; i < expandedArgs.length; i++) {
String arg = expandedArgs[i].trim();
+ String nextArg = null;
+ if (OPTIONS_WITH_PARAMETER.contains(arg)) {
+ if (++i < expandedArgs.length) {
+ nextArg = expandedArgs[i];
+ } else {
+ builder.error(
+ new StringDiagnostic("Missing parameter for " + expandedArgs[i - 1] + ".", origin));
+ break;
+ }
+ }
if (arg.length() == 0) {
continue;
} else if (arg.equals("--help")) {
@@ -176,19 +200,18 @@
} else if (arg.equals("--file-per-class")) {
outputMode = OutputMode.DexFilePerClass;
} else if (arg.equals("--output")) {
- String output = expandedArgs[++i];
if (outputPath != null) {
builder.error(
new StringDiagnostic(
- "Cannot output both to '" + outputPath.toString() + "' and '" + output + "'",
+ "Cannot output both to '" + outputPath.toString() + "' and '" + nextArg + "'",
origin));
continue;
}
- outputPath = Paths.get(output);
+ outputPath = Paths.get(nextArg);
} else if (arg.equals("--lib")) {
- builder.addLibraryFiles(Paths.get(expandedArgs[++i]));
+ builder.addLibraryFiles(Paths.get(nextArg));
} else if (arg.equals("--classpath")) {
- Path file = Paths.get(expandedArgs[++i]);
+ Path file = Paths.get(nextArg);
try {
if (!Files.exists(file)) {
throw new NoSuchFileException(file.toString());
@@ -206,17 +229,16 @@
builder.error(new ExceptionDiagnostic(e, new PathOrigin(file)));
}
} else if (arg.equals("--main-dex-list")) {
- builder.addMainDexListFiles(Paths.get(expandedArgs[++i]));
+ builder.addMainDexListFiles(Paths.get(nextArg));
} else if (arg.equals("--main-dex-list-output")) {
- builder.setMainDexListOutputPath(Paths.get(expandedArgs[++i]));
+ builder.setMainDexListOutputPath(Paths.get(nextArg));
} else if (arg.equals("--optimize-multidex-for-linearalloc")) {
builder.setOptimizeMultidexForLinearAlloc(true);
} else if (arg.equals("--min-api")) {
- String minApiString = expandedArgs[++i];
if (hasDefinedApiLevel) {
builder.error(new StringDiagnostic("Cannot set multiple --min-api options", origin));
} else {
- parseMinApi(builder, minApiString, origin);
+ parseMinApi(builder, nextArg, origin);
hasDefinedApiLevel = true;
}
} else if (arg.equals("--intermediate")) {
diff --git a/src/main/java/com/android/tools/r8/DexIndexedConsumer.java b/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
index e235e67..8731d2a 100644
--- a/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
+++ b/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.utils.OutputBuilder;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.ZipUtils;
+import com.google.common.collect.ImmutableList;
import com.google.common.io.ByteStreams;
import com.google.common.io.Closer;
import java.io.IOException;
@@ -181,6 +182,12 @@
public static void writeResources(Path archive, List<ProgramResource> resources)
throws IOException, ResourceException {
+ writeResources(archive, resources, ImmutableList.of());
+ }
+
+ public static void writeResources(
+ Path archive, List<ProgramResource> resources, List<DataEntryResource> dataResources)
+ throws IOException, ResourceException {
OpenOption[] options =
new OpenOption[] {StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING};
try (Closer closer = Closer.create()) {
@@ -191,6 +198,11 @@
byte[] bytes = ByteStreams.toByteArray(closer.register(resource.getByteStream()));
ZipUtils.writeToZipStream(out, entryName, bytes, ZipEntry.STORED);
}
+ for (DataEntryResource dataResource : dataResources) {
+ String entryName = dataResource.getName();
+ byte[] bytes = ByteStreams.toByteArray(closer.register(dataResource.getByteStream()));
+ ZipUtils.writeToZipStream(out, entryName, bytes, ZipEntry.STORED);
+ }
}
}
}
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 734e421..4f472c6 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -63,7 +63,6 @@
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.InternalOptions.LineNumberOptimization;
import com.android.tools.r8.utils.LineNumberOptimizer;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.SelfRetraceTest;
@@ -634,12 +633,7 @@
// When line number optimization is turned off the identity mapping for line numbers is
// used. We still run the line number optimizer to collect line numbers and inline frame
// information for the mapping file.
- ClassNameMapper classNameMapper =
- LineNumberOptimizer.run(
- application,
- appView.graphLense(),
- namingLens,
- options.lineNumberOptimization == LineNumberOptimization.OFF);
+ ClassNameMapper classNameMapper = LineNumberOptimizer.run(appView, application, namingLens);
timing.end();
proguardMapSupplier = ProguardMapSupplier.fromClassNameMapper(classNameMapper, options);
diff --git a/src/main/java/com/android/tools/r8/R8CommandParser.java b/src/main/java/com/android/tools/r8/R8CommandParser.java
index 580b068..b91739a 100644
--- a/src/main/java/com/android/tools/r8/R8CommandParser.java
+++ b/src/main/java/com/android/tools/r8/R8CommandParser.java
@@ -4,14 +4,28 @@
package com.android.tools.r8;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.FlagFile;
import com.android.tools.r8.utils.StringDiagnostic;
+import com.google.common.collect.ImmutableSet;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
+import java.util.Set;
public class R8CommandParser extends BaseCompilerCommandParser {
+ private static final Set<String> OPTIONS_WITH_PARAMETER =
+ ImmutableSet.of(
+ "--output",
+ "--lib",
+ "--min-api",
+ "--main-dex-rules",
+ "--main-dex-list",
+ "--main-dex-list-output",
+ "--pg-conf",
+ "--pg-map-output");
+
public static void main(String[] args) throws CompilationFailedException {
R8Command command = parse(args, Origin.root()).build();
if (command.isPrintHelp()) {
@@ -45,7 +59,9 @@
" --output <file> # Output result in <file>.",
" # <file> must be an existing directory or a zip file.",
" --lib <file> # Add <file> as a library resource.",
- " --min-api # Minimum Android API level compatibility.",
+ " --min-api <number> # Minimum Android API level compatibility, default: "
+ + AndroidApiLevel.getDefault().getLevel()
+ + ".",
" --pg-conf <file> # Proguard configuration <file>.",
" --pg-map-output <file> # Output the resulting name and line mapping to <file>.",
" --no-tree-shaking # Force disable tree shaking of unreachable classes.",
@@ -55,7 +71,8 @@
" --main-dex-rules <file> # Proguard keep rules for classes to place in the",
" # primary dex file.",
" --main-dex-list <file> # List of classes to place in the primary dex file.",
- " --main-dex-list-output <file> # Output the full main-dex list in <file>.",
+ " --main-dex-list-output <file> ",
+ " # Output the full main-dex list in <file>.",
" --version # Print the version of r8.",
" --help # Print this message."));
/**
@@ -102,6 +119,17 @@
String[] expandedArgs = FlagFile.expandFlagFiles(args, builder);
for (int i = 0; i < expandedArgs.length; i++) {
String arg = expandedArgs[i].trim();
+ String nextArg = null;
+ if (OPTIONS_WITH_PARAMETER.contains(arg)) {
+ if (++i < expandedArgs.length) {
+ nextArg = expandedArgs[i];
+ } else {
+ builder.error(
+ new StringDiagnostic(
+ "Missing parameter for " + expandedArgs[i - 1] + ".", argsOrigin));
+ break;
+ }
+ }
if (arg.length() == 0) {
continue;
} else if (arg.equals("--help")) {
@@ -137,26 +165,24 @@
}
state.outputMode = OutputMode.ClassFile;
} else if (arg.equals("--output")) {
- String outputPath = expandedArgs[++i];
if (state.outputPath != null) {
builder.error(
new StringDiagnostic(
"Cannot output both to '"
+ state.outputPath.toString()
+ "' and '"
- + outputPath
+ + nextArg
+ "'",
argsOrigin));
}
- state.outputPath = Paths.get(outputPath);
+ state.outputPath = Paths.get(nextArg);
} else if (arg.equals("--lib")) {
- builder.addLibraryFiles(Paths.get(expandedArgs[++i]));
+ builder.addLibraryFiles(Paths.get(nextArg));
} else if (arg.equals("--min-api")) {
- String minApiString = expandedArgs[++i];
if (state.hasDefinedApiLevel) {
builder.error(new StringDiagnostic("Cannot set multiple --min-api options", argsOrigin));
} else {
- parseMinApi(builder, minApiString, argsOrigin);
+ parseMinApi(builder, nextArg, argsOrigin);
state.hasDefinedApiLevel = true;
}
} else if (arg.equals("--no-tree-shaking")) {
@@ -166,17 +192,17 @@
} else if (arg.equals("--no-desugaring")) {
builder.setDisableDesugaring(true);
} else if (arg.equals("--main-dex-rules")) {
- builder.addMainDexRulesFiles(Paths.get(expandedArgs[++i]));
+ builder.addMainDexRulesFiles(Paths.get(nextArg));
} else if (arg.equals("--main-dex-list")) {
- builder.addMainDexListFiles(Paths.get(expandedArgs[++i]));
+ builder.addMainDexListFiles(Paths.get(nextArg));
} else if (arg.equals("--main-dex-list-output")) {
- builder.setMainDexListOutputPath(Paths.get(expandedArgs[++i]));
+ builder.setMainDexListOutputPath(Paths.get(nextArg));
} else if (arg.equals("--optimize-multidex-for-linearalloc")) {
builder.setOptimizeMultidexForLinearAlloc(true);
} else if (arg.equals("--pg-conf")) {
- builder.addProguardConfigurationFiles(Paths.get(expandedArgs[++i]));
+ builder.addProguardConfigurationFiles(Paths.get(nextArg));
} else if (arg.equals("--pg-map-output")) {
- builder.setProguardMapOutputPath(Paths.get(expandedArgs[++i]));
+ builder.setProguardMapOutputPath(Paths.get(nextArg));
} else if (arg.equals("--no-data-resources")) {
state.includeDataResources = false;
} else {
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 936f967..2ebccfe 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions.OutlineOptions;
import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
@@ -269,25 +270,23 @@
* language, where interfaces "extend" their superinterface.
*/
public void forAllImplementsSubtypes(Consumer<DexType> f) {
- if (hierarchyLevel != INTERFACE_LEVEL) {
- return;
+ allImplementsSubtypes().forEach(f);
+ }
+
+ public Iterable<DexType> allImplementsSubtypes() {
+ if (hierarchyLevel == INTERFACE_LEVEL) {
+ return Iterables.filter(directSubtypes, subtype -> !subtype.isInterface());
}
- for (DexType subtype : directSubtypes) {
- // Filter out other interfaces.
- if (subtype.hierarchyLevel != INTERFACE_LEVEL) {
- f.accept(subtype);
- }
- }
+ return ImmutableList.of();
+ }
+
+ public static Iterable<DexType> allInterfaces(DexItemFactory dexItemFactory) {
+ assert dexItemFactory.objectType.hierarchyLevel == ROOT_LEVEL;
+ return Iterables.filter(dexItemFactory.objectType.directSubtypes, DexType::isInterface);
}
public static void forAllInterfaces(DexItemFactory factory, Consumer<DexType> f) {
- DexType object = factory.objectType;
- assert object.hierarchyLevel == ROOT_LEVEL;
- for (DexType subtype : object.directSubtypes) {
- if (subtype.isInterface()) {
- f.accept(subtype);
- }
- }
+ allInterfaces(factory).forEach(f);
}
/**
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index e53f959..bc81538 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -96,7 +96,7 @@
// Initialize top-level naming state.
topLevelState = new Namespace(
getPackageBinaryNameFromJavaType(options.getProguardConfiguration().getPackagePrefix()));
- states.computeIfAbsent("", k -> topLevelState);
+ states.put("", topLevelState);
}
static class ClassRenaming {
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 90dbbdb..be339b6 100644
--- a/src/main/java/com/android/tools/r8/naming/FieldNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/FieldNameMinifier.java
@@ -27,7 +27,7 @@
Function<DexType, ?> getKeyTransform() {
if (overloadAggressively) {
// Use the type as the key, hence reuse names per type.
- return a -> a;
+ return Function.identity();
} else {
// Always use the same key, hence do not reuse names per type.
return a -> Void.class;
@@ -97,7 +97,7 @@
if (clazz == null) {
return;
}
- NamingState<DexType, ?> state = getState(clazz.type);
+ NamingState<DexType, ?> state = minifierState.getState(clazz.type);
assert state != null;
clazz.forEachField(field -> renameField(field, state));
type.forAllExtendsSubtypes(this::renameFieldsInSubtypes);
@@ -106,9 +106,7 @@
private void renameField(DexEncodedField encodedField, NamingState<DexType, ?> state) {
DexField field = encodedField.field;
if (!state.isReserved(field.name, field.type)) {
- renaming.put(
- field,
- state.assignNewNameFor(field.name, field.type, useUniqueMemberNames));
+ renaming.put(field, state.assignNewNameFor(field.name, field.type, useUniqueMemberNames));
}
}
diff --git a/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
new file mode 100644
index 0000000..b643be3
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/naming/InterfaceMethodNameMinifier.java
@@ -0,0 +1,400 @@
+// 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.MethodNameMinifier.shuffleMethods;
+
+import com.android.tools.r8.graph.DexCallSite;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedMethod;
+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.MethodNameMinifier.FrontierState;
+import com.android.tools.r8.naming.MethodNameMinifier.MethodNamingState;
+import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
+import com.android.tools.r8.utils.InternalOptions;
+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.ImmutableSet;
+import com.google.common.collect.Sets;
+import java.io.PrintStream;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+public class InterfaceMethodNameMinifier {
+
+ private final AppInfoWithLiveness appInfo;
+ private final Set<DexCallSite> desugaredCallSites;
+ private final Equivalence<DexMethod> equivalence;
+ private final FrontierState frontierState;
+ private final MemberNameMinifier<DexMethod, DexProto>.State minifierState;
+ private final InternalOptions options;
+
+ 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<>();
+
+ /** 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<>();
+
+ /**
+ * A map from DexMethods to all the definitions seen. Needed as the Wrapper equalizes them all.
+ */
+ private final Map<Wrapper<DexMethod>, Set<DexMethod>> sourceMethodsMap = new HashMap<>();
+
+ InterfaceMethodNameMinifier(
+ AppInfoWithLiveness appInfo,
+ Set<DexCallSite> desugaredCallSites,
+ Equivalence<DexMethod> equivalence,
+ FrontierState frontierState,
+ MemberNameMinifier<DexMethod, DexProto>.State minifierState,
+ InternalOptions options) {
+ this.appInfo = appInfo;
+ this.desugaredCallSites = desugaredCallSites;
+ this.equivalence = equivalence;
+ this.frontierState = frontierState;
+ this.minifierState = minifierState;
+ this.options = options;
+ }
+
+ public Comparator<Wrapper<DexMethod>> createDefaultInterfaceMethodOrdering() {
+ return (a, b) -> globalStateMap.get(b).size() - globalStateMap.get(a).size();
+ }
+
+ Map<DexCallSite, DexString> getCallSiteRenamings() {
+ return callSiteRenamings;
+ }
+
+ private void reserveNamesInInterfaces() {
+ for (DexType type : DexType.allInterfaces(appInfo.dexItemFactory)) {
+ assert type.isInterface();
+ frontierState.allocateNamingStateAndReserve(type, type, null);
+ }
+ }
+
+ void assignNamesToInterfaceMethods(Timing timing) {
+ // Reserve all the names that are required for interfaces.
+ reserveNamesInInterfaces();
+
+ // First compute a map from method signatures to a set of naming states for interfaces and
+ // frontier states of classes that implement them. We add the frontier states so that we can
+ // reserve the names for later method naming.
+ timing.begin("Compute map");
+ for (DexType type : DexType.allInterfaces(appInfo.dexItemFactory)) {
+ assert type.isInterface();
+ DexClass clazz = appInfo.definitionFor(type);
+ if (clazz != null) {
+ assert clazz.isInterface();
+ Set<NamingState<DexProto, ?>> collectedStates = getReachableStates(type);
+ for (DexEncodedMethod method : shuffleMethods(clazz.methods(), options)) {
+ addStatesToGlobalMapForMethod(method.method, collectedStates, type);
+ }
+ }
+ }
+
+ // Collect the live call sites for multi-interface lambda expression renaming. For code with
+ // desugared lambdas this is a conservative estimate, as we don't track if the generated
+ // lambda classes survive into the output. As multi-interface lambda expressions are rare
+ // this is not a big deal.
+ Set<DexCallSite> liveCallSites = Sets.union(desugaredCallSites, appInfo.callSites);
+ // If the input program contains a multi-interface lambda expression that implements
+ // interface methods with different protos, we need to make sure that
+ // the implemented lambda methods are renamed to the same name.
+ // To achieve this, we map each DexCallSite that corresponds to a lambda expression to one of
+ // the DexMethods it implements, and we unify the DexMethods that need to be renamed together.
+ Map<DexCallSite, DexMethod> callSites = new IdentityHashMap<>();
+ // Union-find structure to keep track of methods that must be renamed together.
+ // Note that if the input does not use multi-interface lambdas,
+ // unificationParent will remain empty.
+ Map<Wrapper<DexMethod>, Wrapper<DexMethod>> unificationParent = new HashMap<>();
+ liveCallSites.forEach(
+ callSite -> {
+ Set<Wrapper<DexMethod>> callSiteMethods = new HashSet<>();
+ // Don't report errors, as the set of call sites is a conservative estimate, and can
+ // refer to interfaces which has been removed.
+ Set<DexEncodedMethod> implementedMethods =
+ appInfo.lookupLambdaImplementedMethods(callSite);
+ if (implementedMethods.isEmpty()) {
+ return;
+ }
+ callSites.put(callSite, implementedMethods.iterator().next().method);
+ for (DexEncodedMethod method : implementedMethods) {
+ DexType iface = method.method.holder;
+ assert iface.isInterface();
+ Set<NamingState<DexProto, ?>> collectedStates = getReachableStates(iface);
+ addStatesToGlobalMapForMethod(method.method, collectedStates, iface);
+ callSiteMethods.add(equivalence.wrap(method.method));
+ }
+ if (callSiteMethods.size() > 1) {
+ // Implemented interfaces have different return types. Unify them.
+ Wrapper<DexMethod> mainKey = callSiteMethods.iterator().next();
+ mainKey = unificationParent.getOrDefault(mainKey, mainKey);
+ for (Wrapper<DexMethod> key : callSiteMethods) {
+ unificationParent.put(key, mainKey);
+ }
+ }
+ });
+ Map<Wrapper<DexMethod>, Set<Wrapper<DexMethod>>> unification = new HashMap<>();
+ for (Wrapper<DexMethod> key : unificationParent.keySet()) {
+ // Find root with path-compression.
+ Wrapper<DexMethod> root = unificationParent.get(key);
+ while (unificationParent.get(root) != root) {
+ Wrapper<DexMethod> k = unificationParent.get(unificationParent.get(root));
+ unificationParent.put(root, k);
+ root = k;
+ }
+ unification.computeIfAbsent(root, k -> new HashSet<>()).add(key);
+ }
+ timing.end();
+ // Go over every method and assign a name.
+ timing.begin("Allocate names");
+
+ // Sort the methods by the number of dependent states, so that we use short names for methods
+ // that are referenced in many places.
+ List<Wrapper<DexMethod>> interfaceMethods =
+ globalStateMap.keySet().stream()
+ .filter(wrapper -> unificationParent.getOrDefault(wrapper, wrapper).equals(wrapper))
+ .sorted(options.testing.minifier.createInterfaceMethodOrdering(this))
+ .collect(Collectors.toList());
+
+ // Propagate reserved names to all states.
+ List<Wrapper<DexMethod>> reservedInterfaceMethods =
+ interfaceMethods.stream()
+ .filter(wrapper -> anyIsReserved(wrapper, unification))
+ .collect(Collectors.toList());
+ for (Wrapper<DexMethod> key : reservedInterfaceMethods) {
+ propagateReservedNames(key, unification);
+ }
+
+ // Verify that there is no more to propagate.
+ assert reservedInterfaceMethods.stream()
+ .noneMatch(key -> propagateReservedNames(key, unification));
+
+ // Assign names to unreserved interface methods.
+ for (Wrapper<DexMethod> key : interfaceMethods) {
+ if (!reservedInterfaceMethods.contains(key)) {
+ assignNameToInterfaceMethod(key, unification);
+ }
+ }
+
+ for (Entry<DexCallSite, DexMethod> entry : callSites.entrySet()) {
+ DexMethod method = entry.getValue();
+ DexString renamed = minifierState.getRenaming(method);
+ if (originStates.get(equivalence.wrap(method)).isReserved(method.name, method.proto)) {
+ assert renamed == null;
+ callSiteRenamings.put(entry.getKey(), method.name);
+ } else {
+ assert renamed != null;
+ callSiteRenamings.put(entry.getKey(), renamed);
+ }
+ }
+ timing.end();
+ }
+
+ private boolean propagateReservedNames(
+ Wrapper<DexMethod> key, Map<Wrapper<DexMethod>, Set<Wrapper<DexMethod>>> unification) {
+ Set<Wrapper<DexMethod>> unifiedMethods =
+ unification.getOrDefault(key, Collections.singleton(key));
+ boolean changed = false;
+ for (Wrapper<DexMethod> wrapper : unifiedMethods) {
+ DexMethod unifiedMethod = wrapper.get();
+ assert unifiedMethod != null;
+ for (NamingState<DexProto, ?> namingState : globalStateMap.get(wrapper)) {
+ if (!namingState.isReserved(unifiedMethod.name, unifiedMethod.proto)) {
+ namingState.reserveName(unifiedMethod.name, unifiedMethod.proto);
+ changed = true;
+ }
+ }
+ }
+ return changed;
+ }
+
+ private void assignNameToInterfaceMethod(
+ Wrapper<DexMethod> key, Map<Wrapper<DexMethod>, Set<Wrapper<DexMethod>>> unification) {
+ List<MethodNamingState> 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)) {
+ collectedStates.add(
+ new MethodNamingState(namingState, unifiedMethod.name, unifiedMethod.proto));
+ }
+ }
+
+ DexMethod method = key.get();
+ assert method != null;
+
+ Set<String> loggingFilter = options.extensiveInterfaceMethodMinifierLoggingFilter;
+ if (!loggingFilter.isEmpty()) {
+ if (sourceMethods.stream().map(DexMethod::toSourceString).anyMatch(loggingFilter::contains)) {
+ print(method, sourceMethods, collectedStates, System.out);
+ }
+ }
+
+ MethodNamingState originState =
+ new MethodNamingState(originStates.get(key), method.name, method.proto);
+ assignNameForInterfaceMethodInAllStates(collectedStates, sourceMethods, originState);
+ }
+
+ private void assignNameForInterfaceMethodInAllStates(
+ List<MethodNamingState> collectedStates,
+ Set<DexMethod> sourceMethods,
+ MethodNamingState originState) {
+ assert !anyIsReserved(collectedStates);
+
+ // We use the origin state to allocate a name here so that we can reuse names between different
+ // unrelated interfaces. This saves some space. The alternative would be to use a global state
+ // for allocating names, which would save the work to search here.
+ DexString previousCandidate = null;
+ DexString candidate;
+ do {
+ candidate = originState.assignNewName();
+
+ // If the state returns the same candidate for two consecutive trials, it should be this case:
+ // 1) an interface method with the same signature (name, param) but different return type
+ // has been already renamed; and 2) -useuniqueclassmembernames is set.
+ // The option forces the naming state to return the same renamed name for the same signature.
+ // So, here we break the loop in an ad-hoc manner.
+ if (candidate != null && candidate == previousCandidate) {
+ assert minifierState.useUniqueMemberNames();
+ break;
+ }
+ for (MethodNamingState state : collectedStates) {
+ if (!state.isAvailable(candidate)) {
+ previousCandidate = candidate;
+ candidate = null;
+ break;
+ }
+ }
+ } while (candidate == null);
+ for (MethodNamingState state : collectedStates) {
+ state.addRenaming(candidate);
+ }
+ // Rename all methods in interfaces that gave rise to this renaming.
+ for (DexMethod sourceMethod : sourceMethods) {
+ minifierState.putRenaming(sourceMethod, candidate);
+ }
+ }
+
+ private void addStatesToGlobalMapForMethod(
+ DexMethod method, Set<NamingState<DexProto, ?>> collectedStates, DexType originInterface) {
+ Wrapper<DexMethod> key = equivalence.wrap(method);
+ globalStateMap.computeIfAbsent(key, k -> new HashSet<>()).addAll(collectedStates);
+ sourceMethodsMap.computeIfAbsent(key, k -> new HashSet<>()).add(method);
+ originStates.putIfAbsent(key, minifierState.getState(originInterface));
+ }
+
+ private boolean anyIsReserved(
+ Wrapper<DexMethod> key, Map<Wrapper<DexMethod>, Set<Wrapper<DexMethod>>> unification) {
+ Set<Wrapper<DexMethod>> unifiedMethods =
+ unification.getOrDefault(key, Collections.singleton(key));
+ for (Wrapper<DexMethod> wrapper : unifiedMethods) {
+ DexMethod unifiedMethod = wrapper.get();
+ assert unifiedMethod != null;
+
+ for (NamingState<DexProto, ?> namingState : globalStateMap.get(wrapper)) {
+ if (namingState.isReserved(unifiedMethod.name, unifiedMethod.proto)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private boolean anyIsReserved(List<MethodNamingState> collectedStates) {
+ DexString name = collectedStates.get(0).getName();
+ Map<DexProto, Boolean> globalStateCache = new IdentityHashMap<>();
+ for (MethodNamingState state : collectedStates) {
+ assert state.getName() == name;
+ boolean isReservedInGlobalState =
+ globalStateCache.computeIfAbsent(
+ state.getProto(), proto -> minifierState.isReservedInGlobalState(name, proto));
+ // TODO(christofferqa): Should this be using logical OR instead?
+ if (isReservedInGlobalState && state.isReserved()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private Set<NamingState<DexProto, ?>> getReachableStates(DexType type) {
+ if (minifierState.useUniqueMemberNames()) {
+ return ImmutableSet.of(minifierState.globalState());
+ }
+
+ Set<DexType> reachableInterfaces = Sets.newIdentityHashSet();
+ reachableInterfaces.add(type);
+ collectSuperInterfaces(type, reachableInterfaces);
+ collectSubInterfaces(type, reachableInterfaces);
+
+ Set<NamingState<DexProto, ?>> reachableStates = new HashSet<>();
+ for (DexType reachableInterface : reachableInterfaces) {
+ // Add the interface itself.
+ reachableStates.add(minifierState.getState(reachableInterface));
+
+ // And the frontiers that correspond to the classes that implement the interface.
+ for (DexType frontier : reachableInterface.allImplementsSubtypes()) {
+ NamingState<DexProto, ?> state = minifierState.getState(frontierState.get(frontier));
+ assert state != null;
+ reachableStates.add(state);
+ }
+ }
+ return reachableStates;
+ }
+
+ private void collectSuperInterfaces(DexType iface, Set<DexType> interfaces) {
+ DexClass clazz = appInfo.definitionFor(iface);
+ // In cases where we lack the interface's definition, we can at least look at subtypes and
+ // tie those up to get proper naming.
+ if (clazz != null) {
+ for (DexType type : clazz.interfaces.values) {
+ if (interfaces.add(type)) {
+ collectSuperInterfaces(type, interfaces);
+ }
+ }
+ }
+ }
+
+ private void collectSubInterfaces(DexType iface, Set<DexType> interfaces) {
+ for (DexType subtype : iface.allExtendsSubtypes()) {
+ assert subtype.isInterface();
+ if (interfaces.add(subtype)) {
+ collectSubInterfaces(subtype, interfaces);
+ }
+ }
+ }
+
+ private void print(
+ DexMethod method,
+ Set<DexMethod> sourceMethods,
+ List<MethodNamingState> collectedStates,
+ PrintStream out) {
+ out.println("-----------------------------------------------------------------------");
+ out.println("assignNameToInterfaceMethod(`" + method.toSourceString() + "`)");
+ out.println("-----------------------------------------------------------------------");
+ out.println("Source methods:");
+ for (DexMethod sourceMethod : sourceMethods) {
+ out.println(" " + sourceMethod.toSourceString());
+ }
+ out.println("States:");
+ collectedStates.forEach(state -> state.print(" ", minifierState::getStateKey, out));
+ out.println();
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
index 7f28744..419bfbb 100644
--- a/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
@@ -9,6 +9,8 @@
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
import com.android.tools.r8.utils.InternalOptions;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
@@ -22,11 +24,16 @@
protected final List<String> dictionary;
protected final Map<MemberType, DexString> renaming = new IdentityHashMap<>();
- protected final Map<DexType, NamingState<StateType, ?>> states = new IdentityHashMap<>();
protected final NamingState<StateType, ?> globalState;
protected final boolean useUniqueMemberNames;
protected final boolean overloadAggressively;
+ 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(AppInfoWithLiveness appInfo, RootSet rootSet, InternalOptions options) {
this.appInfo = appInfo;
this.rootSet = rootSet;
@@ -46,7 +53,36 @@
return useUniqueMemberNames ? globalState : states.computeIfAbsent(type, f);
}
- protected NamingState<StateType, ?> getState(DexType type) {
- return useUniqueMemberNames ? globalState : states.get(type);
+ // 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 useUniqueMemberNames ? globalState : states.get(type);
+ }
+
+ DexType getStateKey(NamingState<StateType, ?> state) {
+ return states.inverse().get(state);
+ }
+
+ NamingState<StateType, ?> globalState() {
+ return globalState;
+ }
+
+ boolean isReservedInGlobalState(DexString name, StateType state) {
+ return globalState.isReserved(name, state);
+ }
+
+ boolean useUniqueMemberNames() {
+ return useUniqueMemberNames;
+ }
}
}
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 21e6bf9..8864271 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
@@ -19,15 +19,10 @@
import com.google.common.base.Equivalence;
import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Sets;
-import java.util.ArrayList;
-import java.util.Collections;
+import java.io.PrintStream;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.IdentityHashMap;
-import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Set;
import java.util.function.Function;
@@ -92,20 +87,15 @@
*/
class MethodNameMinifier extends MemberNameMinifier<DexMethod, DexProto> {
- private final Set<DexCallSite> desugaredCallSites;
-
private final Equivalence<DexMethod> equivalence;
- private final Map<DexCallSite, DexString> callSiteRenaming = new IdentityHashMap<>();
- private final Map<DexType, DexType> frontierMap = new IdentityHashMap<>();
+ private final FrontierState frontierState = new FrontierState();
MethodNameMinifier(
AppInfoWithLiveness appInfo,
RootSet rootSet,
- Set<DexCallSite> desugaredCallSites,
InternalOptions options) {
super(appInfo, rootSet, options);
- this.desugaredCallSites = desugaredCallSites;
equivalence =
overloadAggressively
? MethodSignatureEquivalence.get()
@@ -139,32 +129,31 @@
}
}
- MethodRenaming computeRenaming(Timing timing) {
+ MethodRenaming computeRenaming(Set<DexCallSite> desugaredCallSites, Timing timing) {
// Phase 1: Reserve all the names that need to be kept and allocate linked state in the
// library part.
timing.begin("Phase 1");
- reserveNamesInClasses(
- appInfo.dexItemFactory.objectType, appInfo.dexItemFactory.objectType, null);
+ reserveNamesInClasses();
timing.end();
- // Phase 2: Reserve all the names that are required for interfaces.
+ // Phase 2: Reserve all the names that are required for interfaces, and then assign names to
+ // interface methods. These are assigned by finding a name that is free in all naming
+ // states that may hold an implementation
timing.begin("Phase 2");
- DexType.forAllInterfaces(appInfo.dexItemFactory, this::reserveNamesInInterfaces);
+ InterfaceMethodNameMinifier interfaceMethodNameMinifier =
+ new InterfaceMethodNameMinifier(
+ appInfo, desugaredCallSites, equivalence, frontierState, minifierState, options);
+ interfaceMethodNameMinifier.assignNamesToInterfaceMethods(timing);
timing.end();
- // Phase 3: Assign names to interface methods. These are assigned by finding a name that is
- // free in all naming states that may hold an implementation.
+ // Phase 3: Assign names top-down by traversing the subtype hierarchy.
timing.begin("Phase 3");
- assignNamesToInterfaceMethods(timing);
- timing.end();
- // Phase 4: Assign names top-down by traversing the subtype hierarchy.
- timing.begin("Phase 4");
assignNamesToClassesMethods(appInfo.dexItemFactory.objectType, false);
timing.end();
// Phase 4: Do the same for private methods.
- timing.begin("Phase 5");
+ timing.begin("Phase 4");
assignNamesToClassesMethods(appInfo.dexItemFactory.objectType, true);
timing.end();
- return new MethodRenaming(renaming, callSiteRenaming);
+ return new MethodRenaming(renaming, interfaceMethodNameMinifier.getCallSiteRenamings());
}
private void assignNamesToClassesMethods(DexType type, boolean doPrivates) {
@@ -172,7 +161,7 @@
if (holder != null && !holder.isLibraryClass()) {
Map<Wrapper<DexMethod>, DexString> renamingAtThisLevel = new HashMap<>();
NamingState<DexProto, ?> state =
- computeStateIfAbsent(type, k -> getState(holder.superType).createChild());
+ computeStateIfAbsent(type, k -> minifierState.getState(holder.superType).createChild());
for (DexEncodedMethod method : holder.allMethodsSorted()) {
assignNameToMethod(method, state, renamingAtThisLevel, doPrivates);
}
@@ -206,292 +195,77 @@
}
}
- private Set<NamingState<DexProto, ?>> getReachableStates(DexType type) {
- Set<DexType> interfaces = Sets.newIdentityHashSet();
- interfaces.add(type);
- collectSuperInterfaces(type, interfaces);
- collectSubInterfaces(type, interfaces);
- Set<NamingState<DexProto, ?>> reachableStates = new HashSet<>();
- for (DexType iface : interfaces) {
- // Add the interface itself
- reachableStates.add(getState(iface));
- // And the frontiers that correspond to the classes that implement the interface.
- iface.forAllImplementsSubtypes(t -> {
- NamingState<DexProto, ?> state = getState(frontierMap.get(t));
- assert state != null;
- reachableStates.add(state);
- });
- }
- return reachableStates;
- }
-
- private void assignNamesToInterfaceMethods(Timing timing) {
- // First compute a map from method signatures to a set of naming states for interfaces and
- // frontier states of classes that implement them. We add the frontier states so that we can
- // reserve the names for later method naming.
- timing.begin("Compute map");
- // A map from DexMethods to all the states linked to interfaces they appear in.
- Map<Wrapper<DexMethod>, Set<NamingState<DexProto, ?>>> globalStateMap = new HashMap<>();
- // A map from DexMethods to all the definitions seen. Needed as the Wrapper equalizes them all.
- Map<Wrapper<DexMethod>, Set<DexMethod>> sourceMethodsMap = new HashMap<>();
- // A map from DexMethods to the first interface state it was seen in. Used to pick good names.
- Map<Wrapper<DexMethod>, NamingState<DexProto, ?>> originStates = new HashMap<>();
- DexType.forAllInterfaces(
- appInfo.dexItemFactory,
- iface -> {
- assert iface.isInterface();
- DexClass clazz = appInfo.definitionFor(iface);
- if (clazz != null) {
- assert clazz.isInterface();
- Set<NamingState<DexProto, ?>> collectedStates = getReachableStates(iface);
- for (DexEncodedMethod method : shuffleMethods(clazz.methods())) {
- addStatesToGlobalMapForMethod(
- method, collectedStates, globalStateMap, sourceMethodsMap, originStates, iface);
- }
- }
- });
- // Collect the live call sites for multi-interface lambda expression renaming. For code with
- // desugared lambdas this is a conservative estimate, as we don't track if the generated
- // lambda classes survive into the output. As multi-interface lambda expressions are rare
- // this is not a big deal.
- Set<DexCallSite> liveCallSites = Sets.union(desugaredCallSites, appInfo.callSites);
- // If the input program contains a multi-interface lambda expression that implements
- // interface methods with different protos, we need to make sure that
- // the implemented lambda methods are renamed to the same name.
- // To achieve this, we map each DexCallSite that corresponds to a lambda expression to one of
- // the DexMethods it implements, and we unify the DexMethods that need to be renamed together.
- Map<DexCallSite, DexMethod> callSites = new IdentityHashMap<>();
- // Union-find structure to keep track of methods that must be renamed together.
- // Note that if the input does not use multi-interface lambdas,
- // unificationParent will remain empty.
- Map<Wrapper<DexMethod>, Wrapper<DexMethod>> unificationParent = new HashMap<>();
- liveCallSites.forEach(
- callSite -> {
- Set<Wrapper<DexMethod>> callSiteMethods = new HashSet<>();
- // Don't report errors, as the set of call sites is a conservative estimate, and can
- // refer to interfaces which has been removed.
- Set<DexEncodedMethod> implementedMethods =
- appInfo.lookupLambdaImplementedMethods(callSite);
- if (implementedMethods.isEmpty()) {
- return;
- }
- callSites.put(callSite, implementedMethods.iterator().next().method);
- for (DexEncodedMethod method : implementedMethods) {
- DexType iface = method.method.holder;
- assert iface.isInterface();
- Set<NamingState<DexProto, ?>> collectedStates = getReachableStates(iface);
- addStatesToGlobalMapForMethod(
- method, collectedStates, globalStateMap, sourceMethodsMap, originStates, iface);
- callSiteMethods.add(equivalence.wrap(method.method));
- }
- if (callSiteMethods.size() > 1) {
- // Implemented interfaces have different return types. Unify them.
- Wrapper<DexMethod> mainKey = callSiteMethods.iterator().next();
- mainKey = unificationParent.getOrDefault(mainKey, mainKey);
- for (Wrapper<DexMethod> key : callSiteMethods) {
- unificationParent.put(key, mainKey);
- }
- }
- });
- Map<Wrapper<DexMethod>, Set<Wrapper<DexMethod>>> unification = new HashMap<>();
- for (Wrapper<DexMethod> key : unificationParent.keySet()) {
- // Find root with path-compression.
- Wrapper<DexMethod> root = unificationParent.get(key);
- while (unificationParent.get(root) != root) {
- Wrapper<DexMethod> k = unificationParent.get(unificationParent.get(root));
- unificationParent.put(root, k);
- root = k;
- }
- unification.computeIfAbsent(root, k -> new HashSet<>()).add(key);
- }
- timing.end();
- // Go over every method and assign a name.
- timing.begin("Allocate names");
- // Sort the methods by the number of dependent states, so that we use short names for methods
- // references in many places.
- List<Wrapper<DexMethod>> methods = new ArrayList<>(globalStateMap.keySet());
- methods.sort((a, b) -> globalStateMap.get(b).size() - globalStateMap.get(a).size());
- for (Wrapper<DexMethod> key : methods) {
- if (!unificationParent.getOrDefault(key, key).equals(key)) {
- continue;
- }
- List<MethodNamingState> 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)) {
- collectedStates.add(
- new MethodNamingState(namingState, unifiedMethod.name, unifiedMethod.proto));
- }
- }
- DexMethod method = key.get();
- assert method != null;
- MethodNamingState originState =
- new MethodNamingState(originStates.get(key), method.name, method.proto);
- assignNameForInterfaceMethodInAllStates(collectedStates, sourceMethods, originState);
- }
- for (Entry<DexCallSite, DexMethod> entry : callSites.entrySet()) {
- DexMethod method = entry.getValue();
- DexString renamed = renaming.get(method);
- if (originStates.get(equivalence.wrap(method)).isReserved(method.name, method.proto)) {
- assert renamed == null;
- callSiteRenaming.put(entry.getKey(), method.name);
- } else {
- assert renamed != null;
- callSiteRenaming.put(entry.getKey(), renamed);
- }
- }
- timing.end();
- }
-
- private void collectSuperInterfaces(DexType iface, Set<DexType> interfaces) {
- DexClass clazz = appInfo.definitionFor(iface);
- // In cases where we lack the interface's definition, we can at least look at subtypes and
- // tie those up to get proper naming.
- if (clazz != null) {
- for (DexType type : clazz.interfaces.values) {
- if (interfaces.add(type)) {
- collectSuperInterfaces(type, interfaces);
- }
- }
- }
- }
-
- private void collectSubInterfaces(DexType iface, Set<DexType> interfaces) {
- iface.forAllExtendsSubtypes(subtype -> {
- assert subtype.isInterface();
- if (interfaces.add(subtype)) {
- collectSubInterfaces(subtype, interfaces);
- }
- });
- }
-
- private void addStatesToGlobalMapForMethod(
- DexEncodedMethod method,
- Set<NamingState<DexProto, ?>> collectedStates,
- Map<Wrapper<DexMethod>, Set<NamingState<DexProto, ?>>> globalStateMap,
- Map<Wrapper<DexMethod>, Set<DexMethod>> sourceMethodsMap,
- Map<Wrapper<DexMethod>, NamingState<DexProto, ?>> originStates,
- DexType originInterface) {
- Wrapper<DexMethod> key = equivalence.wrap(method.method);
- Set<NamingState<DexProto, ?>> stateSet =
- globalStateMap.computeIfAbsent(key, k -> new HashSet<>());
- stateSet.addAll(collectedStates);
- sourceMethodsMap.computeIfAbsent(key, k -> new HashSet<>()).add(method.method);
- originStates.putIfAbsent(key, getState(originInterface));
- }
-
- private void assignNameForInterfaceMethodInAllStates(
- List<MethodNamingState> collectedStates,
- Set<DexMethod> sourceMethods,
- MethodNamingState originState) {
- if (anyIsReserved(collectedStates)) {
- // This method's name is reserved in at least one naming state, so reserve it everywhere.
- for (MethodNamingState state : collectedStates) {
- state.reserveName();
- }
- return;
- }
- // We use the origin state to allocate a name here so that we can reuse names between different
- // unrelated interfaces. This saves some space. The alternative would be to use a global state
- // for allocating names, which would save the work to search here.
- DexString previousCandidate = null;
- DexString candidate;
- do {
- candidate = originState.assignNewNameFor(false);
- // If the state returns the same candidate for two consecutive trials, it should be this case:
- // 1) an interface method with the same signature (name, param) but different return type
- // has been already renamed; and 2) -useuniqueclassmembernames is set.
- // The option forces the naming state to return the same renamed name for the same signature.
- // So, here we break the loop in an ad-hoc manner.
- if (candidate != null && candidate == previousCandidate) {
- assert useUniqueMemberNames;
- break;
- }
- for (MethodNamingState state : collectedStates) {
- if (!state.isAvailable(candidate)) {
- previousCandidate = candidate;
- candidate = null;
- break;
- }
- }
- } while (candidate == null);
- for (MethodNamingState state : collectedStates) {
- state.addRenaming(candidate);
- }
- // Rename all methods in interfaces that gave rise to this renaming.
- for (DexMethod sourceMethod : sourceMethods) {
- renaming.put(sourceMethod, candidate);
- }
- }
-
- private boolean anyIsReserved(List<MethodNamingState> collectedStates) {
- DexString name = collectedStates.get(0).getName();
- Map<DexProto, Boolean> globalStateCache = new HashMap<>();
- for (MethodNamingState state : collectedStates) {
- assert state.getName() == name;
- if (globalStateCache.computeIfAbsent(
- state.getProto(), proto -> globalState.isReserved(name, proto))
- && state.isReserved()) {
- return true;
- }
- }
- return false;
+ private void reserveNamesInClasses() {
+ reserveNamesInClasses(
+ appInfo.dexItemFactory.objectType, appInfo.dexItemFactory.objectType, null);
}
private void reserveNamesInClasses(
DexType type, DexType libraryFrontier, NamingState<DexProto, ?> parent) {
assert !type.isInterface();
- DexClass holder = appInfo.definitionFor(type);
NamingState<DexProto, ?> state =
- allocateNamingStateAndReserve(holder, type, libraryFrontier, parent);
+ frontierState.allocateNamingStateAndReserve(type, libraryFrontier, parent);
+
// If this is a library class (or effectively a library class as it is missing) move the
// frontier forward.
- type.forAllExtendsSubtypes(
- subtype -> {
- assert !subtype.isInterface();
- reserveNamesInClasses(
- subtype,
- holder == null || holder.isLibraryClass() ? subtype : libraryFrontier,
- state);
- });
- }
-
- private void reserveNamesInInterfaces(DexType type) {
- assert type.isInterface();
- frontierMap.put(type, type);
DexClass holder = appInfo.definitionFor(type);
- allocateNamingStateAndReserve(holder, type, type, null);
- }
-
- private NamingState<DexProto, ?> allocateNamingStateAndReserve(
- DexClass holder, DexType type, DexType libraryFrontier, NamingState<DexProto, ?> parent) {
- frontierMap.put(type, libraryFrontier);
- NamingState<DexProto, ?> state =
- computeStateIfAbsent(
- libraryFrontier,
- ignore -> parent == null
- ? NamingState.createRoot(
- appInfo.dexItemFactory, dictionary, getKeyTransform(), useUniqueMemberNames)
- : parent.createChild());
- if (holder != null) {
- boolean keepAll = holder.isLibraryClass() || holder.accessFlags.isAnnotation();
- for (DexEncodedMethod method : shuffleMethods(holder.methods())) {
- reserveNamesForMethod(method, keepAll, state);
- }
+ for (DexType subtype : type.allExtendsSubtypes()) {
+ assert !subtype.isInterface();
+ reserveNamesInClasses(
+ subtype, holder == null || holder.isLibraryClass() ? subtype : libraryFrontier, state);
}
- return state;
}
- private void reserveNamesForMethod(
- DexEncodedMethod encodedMethod, boolean keepAll, NamingState<DexProto, ?> state) {
- DexMethod method = encodedMethod.method;
- if (keepAll || rootSet.noObfuscation.contains(method)) {
+ class FrontierState {
+
+ private final Map<DexType, DexType> frontiers = new IdentityHashMap<>();
+
+ NamingState<DexProto, ?> allocateNamingStateAndReserve(
+ DexType type, DexType frontier, NamingState<DexProto, ?> parent) {
+ if (frontier != type) {
+ frontiers.put(type, frontier);
+ }
+
+ NamingState<DexProto, ?> state =
+ computeStateIfAbsent(
+ frontier,
+ ignore ->
+ parent == null
+ ? NamingState.createRoot(
+ appInfo.dexItemFactory,
+ dictionary,
+ getKeyTransform(),
+ useUniqueMemberNames)
+ : parent.createChild());
+
+ DexClass holder = appInfo.definitionFor(type);
+ if (holder != null) {
+ boolean keepAll = holder.isLibraryClass() || holder.accessFlags.isAnnotation();
+ for (DexEncodedMethod method : shuffleMethods(holder.methods(), options)) {
+ // TODO(christofferqa): Wouldn't it be sufficient only to reserve names for non-private
+ // methods?
+ if (keepAll || rootSet.noObfuscation.contains(method.method)) {
+ reserveNamesForMethod(method.method, state);
+ }
+ }
+ }
+
+ return state;
+ }
+
+ private void reserveNamesForMethod(DexMethod method, NamingState<DexProto, ?> state) {
state.reserveName(method.name, method.proto);
globalState.reserveName(method.name, method.proto);
}
+
+ public DexType get(DexType type) {
+ return frontiers.getOrDefault(type, type);
+ }
+
+ public DexType put(DexType type, DexType frontier) {
+ assert frontier != type;
+ return frontiers.put(type, frontier);
+ }
}
/**
@@ -499,7 +273,7 @@
* 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.
*/
- private static class MethodNamingState {
+ static class MethodNamingState {
private final NamingState<DexProto, ?> parent;
private final DexString name;
@@ -512,8 +286,8 @@
this.proto = proto;
}
- DexString assignNewNameFor(boolean markAsUsed) {
- return parent.assignNewNameFor(name, proto, markAsUsed);
+ DexString assignNewName() {
+ return parent.assignNewNameFor(name, proto, false);
}
void reserveName() {
@@ -539,11 +313,25 @@
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, indentation + " ", out);
+ }
}
// Shuffles the given methods if assertions are enabled and deterministic debugging is disabled.
// Used to ensure that the generated output is deterministic.
- private Iterable<DexEncodedMethod> shuffleMethods(Iterable<DexEncodedMethod> methods) {
+ static Iterable<DexEncodedMethod> shuffleMethods(
+ Iterable<DexEncodedMethod> methods, InternalOptions options) {
return options.testing.irOrdering.order(methods);
}
}
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 becc643..5333125 100644
--- a/src/main/java/com/android/tools/r8/naming/Minifier.java
+++ b/src/main/java/com/android/tools/r8/naming/Minifier.java
@@ -64,8 +64,8 @@
timing.begin("MinifyMethods");
MethodRenaming methodRenaming =
- new MethodNameMinifier(appInfo, rootSet, desugaredCallSites, options)
- .computeRenaming(timing);
+ new MethodNameMinifier(appInfo, rootSet, options)
+ .computeRenaming(desugaredCallSites, timing);
timing.end();
assert new MinifiedRenaming(classRenaming, methodRenaming, FieldRenaming.empty(), appInfo)
diff --git a/src/main/java/com/android/tools/r8/naming/NamingState.java b/src/main/java/com/android/tools/r8/naming/NamingState.java
index 999579a..d414b3b 100644
--- a/src/main/java/com/android/tools/r8/naming/NamingState.java
+++ b/src/main/java/com/android/tools/r8/naming/NamingState.java
@@ -11,10 +11,12 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.collect.Table;
+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;
@@ -129,7 +131,19 @@
state.addRenaming(original, key, newName);
}
- private class InternalState {
+ void printState(ProtoType proto, String indentation, PrintStream out) {
+ KeyType key = keyTransform.apply(proto);
+ InternalState state = findInternalStateFor(key);
+ if (state != null) {
+ state.printReservedNames(indentation, out);
+ state.printRenamings(indentation, out);
+ } else {
+ out.print(indentation);
+ out.println("<NO STATE>");
+ }
+ }
+
+ class InternalState {
private static final int INITIAL_NAME_COUNT = 1;
private final char[] EMPTY_CHAR_ARRAY = new char[0];
@@ -229,5 +243,49 @@
return StringUtils.numberToIdentifier(EMPTY_CHAR_ARRAY, nameCount++, false);
}
}
+
+ void printReservedNames(String indentation, PrintStream out) {
+ out.print(indentation);
+ out.print("Reserved names:");
+ if (reservedNames == null || reservedNames.isEmpty()) {
+ out.print(" <NO RESERVED NAMES>");
+ } else {
+ for (DexString reservedName : reservedNames) {
+ out.print(System.lineSeparator());
+ out.print(indentation);
+ out.print(" ");
+ out.print(reservedName.toSourceString());
+ }
+ }
+ out.println();
+ if (parentInternalState != null) {
+ parentInternalState.printReservedNames(indentation + " ", out);
+ }
+ }
+
+ void printRenamings(String indentation, PrintStream out) {
+ out.print(indentation);
+ out.print("Renamings:");
+ if (renamings == null || renamings.isEmpty()) {
+ out.print(" <NO RENAMINGS>");
+ } else {
+ for (DexString original : renamings.rowKeySet()) {
+ Map<KeyType, DexString> row = renamings.row(original);
+ for (Entry<KeyType, DexString> entry : row.entrySet()) {
+ out.print(System.lineSeparator());
+ out.print(indentation);
+ out.print(" ");
+ out.print(original.toSourceString());
+ out.print(entry.getKey().toString());
+ out.print(" -> ");
+ out.print(entry.getValue().toSourceString());
+ }
+ }
+ }
+ out.println();
+ if (parentInternalState != null) {
+ parentInternalState.printRenamings(indentation + " ", out);
+ }
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/references/Reference.java b/src/main/java/com/android/tools/r8/references/Reference.java
index 0e3d988..decd262 100644
--- a/src/main/java/com/android/tools/r8/references/Reference.java
+++ b/src/main/java/com/android/tools/r8/references/Reference.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.utils.DescriptorUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.MapMaker;
+import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.List;
@@ -150,6 +151,17 @@
returnType == Void.TYPE ? null : typeFromClass(returnType));
}
+ /** Get a method reference from a Java reflection constructor. */
+ public static MethodReference methodFromMethod(Constructor<?> method) {
+ Class<?> holderClass = method.getDeclaringClass();
+ Class<?>[] parameterTypes = method.getParameterTypes();
+ ImmutableList.Builder<TypeReference> builder = ImmutableList.builder();
+ for (Class<?> parameterType : parameterTypes) {
+ builder.add(typeFromClass(parameterType));
+ }
+ return method(classFromClass(holderClass), "<init>", builder.build(), null);
+ }
+
/** Get a field reference from its full reference specification. */
public static FieldReference field(
ClassReference holderClass, String fieldName, TypeReference fieldType) {
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index f011931..1a19a39 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -179,7 +179,7 @@
private final Set<DexType> liveTypes = Sets.newIdentityHashSet();
/** Set of annotation types that are instantiated. */
- private final SetWithReason<DexAnnotation> instantiatedAnnotations =
+ private final SetWithReason<DexAnnotation> liveAnnotations =
new SetWithReason<>(this::registerAnnotation);
/** Set of types that are actually instantiated. These cannot be abstract. */
private final SetWithReason<DexType> instantiatedTypes = new SetWithReason<>(this::registerType);
@@ -307,6 +307,10 @@
this.options = options;
}
+ private static <T> SetWithReason<T> newSetWithoutReasonReporter() {
+ return new SetWithReason<>((f, r) -> {});
+ }
+
private void enqueueRootItems(Map<DexReference, Set<ProguardKeepRule>> items) {
items.entrySet().forEach(this::enqueueRootItem);
}
@@ -824,9 +828,13 @@
}
// If this type has deferred annotations, we have to process those now, too.
Set<DexAnnotation> annotations = deferredAnnotations.remove(type);
- if (annotations != null) {
- annotations.forEach(annotation -> handleAnnotationOfLiveType(holder, annotation));
+ if (annotations != null && !annotations.isEmpty()) {
+ assert !holder.accessFlags.isAnnotation()
+ || annotations.stream().allMatch(a -> a.annotation.type == holder.type);
+ annotations.forEach(annotation -> handleAnnotation(holder, annotation));
}
+ } else {
+ assert deferredAnnotations.get(holder) == null;
}
Map<DexReference, Set<ProguardKeepRule>> dependentItems = rootSet.getDependentItems(holder);
@@ -836,10 +844,6 @@
}
}
- private void handleAnnotationOfLiveType(DexDefinition holder, DexAnnotation annotation) {
- handleAnnotation(holder, annotation, true);
- }
-
private void processAnnotations(DexDefinition holder, DexAnnotation[] annotations) {
for (DexAnnotation annotation : annotations) {
processAnnotation(holder, annotation);
@@ -847,19 +851,23 @@
}
private void processAnnotation(DexDefinition holder, DexAnnotation annotation) {
- handleAnnotation(holder, annotation, false);
+ handleAnnotation(holder, annotation);
}
- private void handleAnnotation(DexDefinition holder, DexAnnotation annotation, boolean forceLive) {
+ private void handleAnnotation(DexDefinition holder, DexAnnotation annotation) {
assert !holder.isDexClass() || !holder.asDexClass().isLibraryClass();
DexType type = annotation.annotation.type;
- boolean isLive = forceLive || liveTypes.contains(type);
+ boolean annotationTypeIsLibraryClass =
+ appInfo.definitionFor(type) == null || appInfo.definitionFor(type).isLibraryClass();
+ boolean isLive = annotationTypeIsLibraryClass || liveTypes.contains(type);
if (!shouldKeepAnnotation(annotation, isLive, appInfo.dexItemFactory, options)) {
// Remember this annotation for later.
- deferredAnnotations.computeIfAbsent(type, ignore -> new HashSet<>()).add(annotation);
+ if (!annotationTypeIsLibraryClass) {
+ deferredAnnotations.computeIfAbsent(type, ignore -> new HashSet<>()).add(annotation);
+ }
return;
}
- instantiatedAnnotations.add(annotation, KeepReason.annotatedOn(holder));
+ liveAnnotations.add(annotation, KeepReason.annotatedOn(holder));
AnnotationReferenceMarker referenceMarker =
new AnnotationReferenceMarker(annotation.annotation.type, appInfo.dexItemFactory);
annotation.annotation.collectIndexedItems(referenceMarker);
@@ -1097,6 +1105,7 @@
SetWithReason<DexEncodedField> reachableFields = reachableInstanceFields.get(type);
if (reachableFields != null) {
for (DexEncodedField field : reachableFields.getItems()) {
+ // TODO(b/120959039): Should the reason this field is reachable come from the set?
markInstanceFieldAsLive(field, KeepReason.reachableFromLiveType(type));
}
}
@@ -1241,14 +1250,14 @@
if (encodedField.accessFlags.isStatic()) {
markStaticFieldAsLive(encodedField.field, reason);
} else {
- SetWithReason<DexEncodedField> reachable =
- reachableInstanceFields.computeIfAbsent(
- encodedField.field.clazz, ignore -> new SetWithReason<>((f, r) -> {}));
- // TODO(b/120959039): The reachable.add test might be hiding other paths to the field.
- if (reachable.add(encodedField, reason)
- && isInstantiatedOrHasInstantiatedSubtype(encodedField.field.clazz)) {
+ if (isInstantiatedOrHasInstantiatedSubtype(encodedField.field.clazz)) {
// We have at least one live subtype, so mark it as live.
markInstanceFieldAsLive(encodedField, reason);
+ } else {
+ // Add the field to the reachable set if the type later becomes instantiated.
+ reachableInstanceFields
+ .computeIfAbsent(encodedField.field.clazz, ignore -> newSetWithoutReasonReporter())
+ .add(encodedField, reason);
}
}
}
@@ -1292,7 +1301,7 @@
// TODO(b/120959039): The reachable.add test might be hiding other paths to the method.
SetWithReason<DexEncodedMethod> reachable =
reachableVirtualMethods.computeIfAbsent(
- encodedMethod.method.holder, (ignore) -> new SetWithReason<>((m, r) -> {}));
+ encodedMethod.method.holder, ignore -> newSetWithoutReasonReporter());
if (reachable.add(encodedMethod, reason)) {
// Abstract methods cannot be live.
if (!encodedMethod.accessFlags.isAbstract()) {
@@ -1856,10 +1865,8 @@
* for these.
*/
public final SortedSet<DexType> liveTypes;
- /**
- * Set of annotation types that are instantiated.
- */
- final SortedSet<DexType> instantiatedAnnotations;
+ /** Set of annotation types that are instantiated. */
+ final SortedSet<DexType> instantiatedAnnotationTypes;
/**
* Set of types that are actually instantiated. These cannot be abstract.
*/
@@ -2020,9 +2027,8 @@
PresortedComparable<DexType>::slowCompareTo, enqueuer.liveTypes);
ImmutableSortedSet.Builder<DexType> builder =
ImmutableSortedSet.orderedBy(PresortedComparable<DexType>::slowCompareTo);
- enqueuer.instantiatedAnnotations.items.forEach(
- annotation -> builder.add(annotation.annotation.type));
- this.instantiatedAnnotations = builder.build();
+ enqueuer.liveAnnotations.items.forEach(annotation -> builder.add(annotation.annotation.type));
+ this.instantiatedAnnotationTypes = builder.build();
this.instantiatedTypes = ImmutableSortedSet.copyOf(
PresortedComparable<DexType>::slowCompareTo, enqueuer.instantiatedTypes.getItems());
this.instantiatedLambdas =
@@ -2080,7 +2086,7 @@
Collection<DexType> removedClasses) {
super(application);
this.liveTypes = previous.liveTypes;
- this.instantiatedAnnotations = previous.instantiatedAnnotations;
+ this.instantiatedAnnotationTypes = previous.instantiatedAnnotationTypes;
this.instantiatedTypes = previous.instantiatedTypes;
this.instantiatedLambdas = previous.instantiatedLambdas;
this.targetedMethods = previous.targetedMethods;
@@ -2128,8 +2134,8 @@
GraphLense lense) {
super(application, lense);
this.liveTypes = rewriteItems(previous.liveTypes, lense::lookupType);
- this.instantiatedAnnotations =
- rewriteItems(previous.instantiatedAnnotations, lense::lookupType);
+ this.instantiatedAnnotationTypes =
+ rewriteItems(previous.instantiatedAnnotationTypes, lense::lookupType);
this.instantiatedTypes = rewriteItems(previous.instantiatedTypes, lense::lookupType);
this.instantiatedLambdas = rewriteItems(previous.instantiatedLambdas, lense::lookupType);
this.targetedMethods = lense.rewriteMethodsConservatively(previous.targetedMethods);
@@ -2207,7 +2213,7 @@
Map<DexType, Reference2IntMap<DexField>> ordinalsMaps) {
super(previous);
this.liveTypes = previous.liveTypes;
- this.instantiatedAnnotations = previous.instantiatedAnnotations;
+ this.instantiatedAnnotationTypes = previous.instantiatedAnnotationTypes;
this.instantiatedTypes = previous.instantiatedTypes;
this.instantiatedLambdas = previous.instantiatedLambdas;
this.targetedMethods = previous.targetedMethods;
@@ -2272,7 +2278,7 @@
assert type.isClassType();
return instantiatedTypes.contains(type)
|| instantiatedLambdas.contains(type)
- || instantiatedAnnotations.contains(type);
+ || instantiatedAnnotationTypes.contains(type);
}
public boolean isInstantiatedIndirectly(DexType type) {
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index e3a8273..5acd2a8 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -1433,11 +1433,27 @@
}
private IdentifierPatternWithWildcards acceptIdentifierWithBackreference(IdentifierType kind) {
+ IdentifierPatternWithWildcardsAndNegation pattern =
+ acceptIdentifierWithBackreference(kind, false);
+ if (pattern == null) {
+ return null;
+ }
+ assert !pattern.negated;
+ return pattern.patternWithWildcards;
+ }
+
+ private IdentifierPatternWithWildcardsAndNegation acceptIdentifierWithBackreference(
+ IdentifierType kind, boolean allowNegation) {
ImmutableList.Builder<ProguardWildcard> wildcardsCollector = ImmutableList.builder();
StringBuilder currentAsterisks = null;
int asteriskCount = 0;
StringBuilder currentBackreference = null;
skipWhitespace();
+
+ final char quote = acceptQuoteIfPresent();
+ final boolean quoted = isQuote(quote);
+ final boolean negated = allowNegation ? acceptChar('!') : false;
+
int start = position;
int end = position;
while (!eof(end)) {
@@ -1516,9 +1532,17 @@
currentBackreference = new StringBuilder();
end += Character.charCount(current);
} else {
+ if (quoted && quote != current) {
+ throw reporter.fatalError(
+ new StringDiagnostic(
+ "Invalid character '" + (char) current + "', expected end-quote.",
+ origin,
+ getPosition()));
+ }
break;
}
}
+ position = quoted ? end + 1 : end;
if (currentAsterisks != null) {
wildcardsCollector.add(new ProguardWildcard.Pattern(currentAsterisks.toString()));
}
@@ -1530,10 +1554,8 @@
if (start == end) {
return null;
}
- position = end;
- return new IdentifierPatternWithWildcards(
- contents.substring(start, end),
- wildcardsCollector.build());
+ return new IdentifierPatternWithWildcardsAndNegation(
+ contents.substring(start, end), wildcardsCollector.build(), negated);
}
private String acceptFieldNameOrIntegerForReturn() {
@@ -1627,20 +1649,20 @@
}
}
+ private void parseClassNameAddToBuilder(ProguardClassNameList.Builder builder)
+ throws ProguardRuleParserException {
+ IdentifierPatternWithWildcardsAndNegation name = parseClassName(true);
+ builder.addClassName(
+ name.negated,
+ ProguardTypeMatcher.create(name.patternWithWildcards, ClassOrType.CLASS, dexItemFactory));
+ skipWhitespace();
+ }
+
private ProguardClassNameList parseClassNames() throws ProguardRuleParserException {
ProguardClassNameList.Builder builder = ProguardClassNameList.builder();
- skipWhitespace();
- boolean negated = acceptChar('!');
- builder.addClassName(negated,
- ProguardTypeMatcher.create(parseClassName(), ClassOrType.CLASS, dexItemFactory));
- skipWhitespace();
- while (acceptChar(',')) {
- skipWhitespace();
- negated = acceptChar('!');
- builder.addClassName(negated,
- ProguardTypeMatcher.create(parseClassName(), ClassOrType.CLASS, dexItemFactory));
- skipWhitespace();
- }
+ do {
+ parseClassNameAddToBuilder(builder);
+ } while (acceptChar(','));
return builder.build();
}
@@ -1650,8 +1672,15 @@
}
private IdentifierPatternWithWildcards parseClassName() throws ProguardRuleParserException {
- IdentifierPatternWithWildcards name =
- acceptIdentifierWithBackreference(IdentifierType.CLASS_NAME);
+ IdentifierPatternWithWildcardsAndNegation name = parseClassName(false);
+ assert !name.negated;
+ return name.patternWithWildcards;
+ }
+
+ private IdentifierPatternWithWildcardsAndNegation parseClassName(boolean allowNegation)
+ throws ProguardRuleParserException {
+ IdentifierPatternWithWildcardsAndNegation name =
+ acceptIdentifierWithBackreference(IdentifierType.CLASS_NAME, allowNegation);
if (name == null) {
throw parseError("Class name expected");
}
@@ -1838,4 +1867,15 @@
return false;
}
}
+
+ static class IdentifierPatternWithWildcardsAndNegation {
+ final IdentifierPatternWithWildcards patternWithWildcards;
+ final boolean negated;
+
+ IdentifierPatternWithWildcardsAndNegation(
+ String pattern, List<ProguardWildcard> wildcards, boolean negated) {
+ patternWithWildcards = new IdentifierPatternWithWildcards(pattern, wildcards);
+ this.negated = negated;
+ }
+ }
}
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidApp.java b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
index 28d1628..369367e 100644
--- a/src/main/java/com/android/tools/r8/utils/AndroidApp.java
+++ b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.DataEntryResource;
import com.android.tools.r8.DataResource;
import com.android.tools.r8.DataResourceProvider;
+import com.android.tools.r8.DataResourceProvider.Visitor;
import com.android.tools.r8.DexFilePerClassFileConsumer;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.DirectoryClassFileProvider;
@@ -213,6 +214,29 @@
}
}
+ public List<DataEntryResource> getDataEntryResourcesForTesting() throws ResourceException {
+ List<DataEntryResource> out = new ArrayList<>();
+ for (ProgramResourceProvider programResourceProvider : getProgramResourceProviders()) {
+ DataResourceProvider dataResourceProvider = programResourceProvider.getDataResourceProvider();
+ if (dataResourceProvider != null) {
+ dataResourceProvider.accept(
+ new Visitor() {
+
+ @Override
+ public void visit(DataDirectoryResource directory) {
+ // Ignore.
+ }
+
+ @Override
+ public void visit(DataEntryResource file) {
+ out.add(file);
+ }
+ });
+ }
+ }
+ return out;
+ }
+
/** Get program resource providers. */
public List<ProgramResourceProvider> getProgramResourceProviders() {
return programResourceProviders;
@@ -320,14 +344,12 @@
}
}
- /**
- * Write the dex program resources to @code{archive} and the proguard resource as its sibling.
- */
+ /** Write the dex program resources to @code{archive}. */
public void writeToZip(Path archive, OutputMode outputMode) throws IOException {
try {
if (outputMode == OutputMode.DexIndexed) {
- List<ProgramResource> resources = getDexProgramResourcesForTesting();
- DexIndexedConsumer.ArchiveConsumer.writeResources(archive, resources);
+ DexIndexedConsumer.ArchiveConsumer.writeResources(
+ archive, getDexProgramResourcesForTesting(), getDataEntryResourcesForTesting());
} else if (outputMode == OutputMode.DexFilePerClassFile) {
List<ProgramResource> resources = getDexProgramResourcesForTesting();
DexFilePerClassFileConsumer.ArchiveConsumer.writeResources(
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidAppConsumers.java b/src/main/java/com/android/tools/r8/utils/AndroidAppConsumers.java
index 4371edd..c7078ec 100644
--- a/src/main/java/com/android/tools/r8/utils/AndroidAppConsumers.java
+++ b/src/main/java/com/android/tools/r8/utils/AndroidAppConsumers.java
@@ -6,6 +6,9 @@
import com.android.tools.r8.BaseCompilerCommand;
import com.android.tools.r8.ByteDataView;
import com.android.tools.r8.ClassFileConsumer;
+import com.android.tools.r8.DataDirectoryResource;
+import com.android.tools.r8.DataEntryResource;
+import com.android.tools.r8.DataResourceConsumer;
import com.android.tools.r8.DexFilePerClassFileConsumer;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.DexIndexedConsumer.ForwardingConsumer;
@@ -105,6 +108,29 @@
}
}
+ @Override
+ public DataResourceConsumer getDataResourceConsumer() {
+ assert consumer.getDataResourceConsumer() == null;
+ return new DataResourceConsumer() {
+
+ @Override
+ public void accept(
+ DataDirectoryResource directory, DiagnosticsHandler diagnosticsHandler) {
+ // Ignore.
+ }
+
+ @Override
+ public void accept(DataEntryResource file, DiagnosticsHandler diagnosticsHandler) {
+ builder.addDataResource(file);
+ }
+
+ @Override
+ public void finished(DiagnosticsHandler handler) {
+ // Ignore.
+ }
+ };
+ }
+
synchronized void addDexFile(int fileIndex, byte[] data, Set<String> descriptors) {
files.put(fileIndex, new DescriptorsWithContents(descriptors, data));
}
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index a5854b6..85c4876 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -21,15 +21,18 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.optimize.Inliner;
+import com.android.tools.r8.naming.InterfaceMethodNameMinifier;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.ProguardConfiguration;
import com.android.tools.r8.shaking.ProguardConfigurationRule;
import com.android.tools.r8.utils.IROrdering.IdentityIROrdering;
import com.android.tools.r8.utils.IROrdering.NondeterministicIROrdering;
+import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -223,6 +226,9 @@
}
public Set<String> extensiveLoggingFilter = getExtensiveLoggingFilter();
+ public Set<String> extensiveInterfaceMethodMinifierLoggingFilter =
+ getExtensiveInterfaceMethodMinifierLoggingFilter();
+
public List<String> methodsFilter = ImmutableList.of();
public int minApiLevel = AndroidApiLevel.getDefault().getLevel();
// Skipping min_api check and compiling an intermediate result intended for later merging.
@@ -298,6 +304,19 @@
return ImmutableSet.of();
}
+ private static Set<String> getExtensiveInterfaceMethodMinifierLoggingFilter() {
+ String property =
+ System.getProperty("com.android.tools.r8.extensiveInterfaceMethodMinifierLoggingFilter");
+ if (property != null) {
+ ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+ for (String method : property.split(";")) {
+ builder.add(method);
+ }
+ return builder.build();
+ }
+ return ImmutableSet.of();
+ }
+
public static class InvalidParameterAnnotationInfo {
final DexMethod method;
@@ -543,6 +562,21 @@
public boolean forceNameReflectionOptimization = false;
public boolean disallowLoadStoreOptimization = false;
public Consumer<IRCode> irModifier = null;
+
+ public MinifierTestingOptions minifier = new MinifierTestingOptions();
+
+ public static class MinifierTestingOptions {
+
+ public Comparator<DexMethod> interfaceMethodOrdering = null;
+
+ public Comparator<Wrapper<DexMethod>> createInterfaceMethodOrdering(
+ InterfaceMethodNameMinifier minifier) {
+ if (interfaceMethodOrdering != null) {
+ return (a, b) -> interfaceMethodOrdering.compare(a.get(), b.get());
+ }
+ return minifier.createDefaultInterfaceMethodOrdering();
+ }
+ }
}
private boolean hasMinApi(AndroidApiLevel level) {
diff --git a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
index f5a4b81..427341f 100644
--- a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
+++ b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
@@ -5,6 +5,8 @@
import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.cf.code.CfPosition;
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.CfCode;
import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexApplication;
@@ -39,6 +41,7 @@
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.naming.Range;
+import com.android.tools.r8.utils.InternalOptions.LineNumberOptimization;
import com.google.common.base.Suppliers;
import java.util.ArrayList;
import java.util.IdentityHashMap;
@@ -66,13 +69,33 @@
}
private static class OptimizingPositionRemapper implements PositionRemapper {
- private int nextLineNumber = 1;
+ private final int maxLineDelta;
+ private DexMethod previousMethod = null;
+ private int previousSourceLine = -1;
+ private int nextOptimizedLineNumber = 1;
+
+ OptimizingPositionRemapper(InternalOptions options) {
+ // TODO(113198295): For dex using "Constants.DBG_LINE_RANGE + Constants.DBG_LINE_BASE"
+ // instead of 1 creates a ~30% smaller map file but the dex files gets larger due to reduced
+ // debug info canonicalization.
+ maxLineDelta = options.isGeneratingClassFiles() ? Integer.MAX_VALUE : 1;
+ }
@Override
public Position createRemappedPosition(
int line, DexString file, DexMethod method, Position callerPosition) {
- Position newPosition = new Position(nextLineNumber, file, method, null);
- ++nextLineNumber;
+ assert method != null;
+ if (previousMethod == method) {
+ assert previousSourceLine >= 0;
+ if (line > previousSourceLine && line - previousSourceLine <= maxLineDelta) {
+ nextOptimizedLineNumber += (line - previousSourceLine) - 1;
+ }
+ }
+
+ Position newPosition = new Position(nextOptimizedLineNumber, file, method, null);
+ ++nextOptimizedLineNumber;
+ previousSourceLine = line;
+ previousMethod = method;
return newPosition;
}
}
@@ -138,10 +161,9 @@
}
public static ClassNameMapper run(
+ AppView<AppInfoWithSubtyping> appView,
DexApplication application,
- GraphLense graphLense,
- NamingLens namingLens,
- boolean identityMapping) {
+ NamingLens namingLens) {
ClassNameMapper.Builder classNameMapperBuilder = ClassNameMapper.builder();
// Collect which files contain which classes that need to have their line numbers optimized.
for (DexProgramClass clazz : application.classes()) {
@@ -151,7 +173,7 @@
// At this point we don't know if we really need to add this class to the builder.
// It depends on whether any methods/fields are renamed or some methods contain positions.
// Create a supplier which creates a new, cached ClassNaming.Builder on-demand.
- DexType originalType = graphLense.getOriginalType(clazz.type);
+ DexType originalType = appView.graphLense().getOriginalType(clazz.type);
DexString renamedClassName = namingLens.lookupDescriptor(clazz.getType());
Supplier<ClassNaming.Builder> onDemandClassNamingBuilder =
Suppliers.memoize(
@@ -164,7 +186,7 @@
addClassToClassNaming(originalType, renamedClassName, onDemandClassNamingBuilder);
// First transfer renamed fields to classNamingBuilder.
- addFieldsToClassNaming(graphLense, namingLens, clazz, onDemandClassNamingBuilder);
+ addFieldsToClassNaming(appView.graphLense(), namingLens, clazz, onDemandClassNamingBuilder);
// Then process the methods, ordered by renamed name.
List<DexString> renamedMethodNames = new ArrayList<>(methodsByRenamedName.keySet());
@@ -179,8 +201,12 @@
sortMethods(methods);
}
+ boolean identityMapping =
+ appView.options().lineNumberOptimization == LineNumberOptimization.OFF;
PositionRemapper positionRemapper =
- identityMapping ? new IdentityPositionRemapper() : new OptimizingPositionRemapper();
+ identityMapping
+ ? new IdentityPositionRemapper()
+ : new OptimizingPositionRemapper(appView.options());
for (DexEncodedMethod method : methods) {
List<MappedPosition> mappedPositions = new ArrayList<>();
@@ -194,7 +220,7 @@
}
}
- DexMethod originalMethod = graphLense.getOriginalMethodSignature(method.method);
+ DexMethod originalMethod = appView.graphLense().getOriginalMethodSignature(method.method);
MethodSignature originalSignature =
MethodSignature.fromDexMethod(originalMethod, originalMethod.holder != clazz.type);
@@ -217,7 +243,7 @@
signatures.put(originalMethod, originalSignature);
Function<DexMethod, MethodSignature> getOriginalMethodSignature =
m -> {
- DexMethod original = graphLense.getOriginalMethodSignature(m);
+ DexMethod original = appView.graphLense().getOriginalMethodSignature(m);
return signatures.computeIfAbsent(
original,
key ->
diff --git a/src/main/java/com/android/tools/r8/utils/StringUtils.java b/src/main/java/com/android/tools/r8/utils/StringUtils.java
index 244639e..c91c288 100644
--- a/src/main/java/com/android/tools/r8/utils/StringUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/StringUtils.java
@@ -141,7 +141,7 @@
return builder.toString();
}
- public static String lines(String... lines) {
+ public static String lines(List<String> lines) {
StringBuilder builder = new StringBuilder();
for (String line : lines) {
builder.append(line).append(LINE_SEPARATOR);
@@ -149,6 +149,10 @@
return builder.toString();
}
+ public static String lines(String... lines) {
+ return lines(Arrays.asList(lines));
+ }
+
public static String withNativeLineSeparator(String s) {
s = s.replace("\r\n", "\n");
if (LINE_SEPARATOR.equals("\r\n")) {
diff --git a/src/test/java/com/android/tools/r8/D8CommandTest.java b/src/test/java/com/android/tools/r8/D8CommandTest.java
index 6b51909..c191d9a 100644
--- a/src/test/java/com/android/tools/r8/D8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/D8CommandTest.java
@@ -507,6 +507,12 @@
assertEquals(0, new ZipFile(emptyZip.toFile(), StandardCharsets.UTF_8).size());
}
+ @Test(expected = CompilationFailedException.class)
+ public void missingParameterForLastOption() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "Missing parameter", handler -> parse(handler, "--output"));
+ }
+
private D8Command parse(String... args) throws CompilationFailedException {
return D8Command.parse(args, EmbeddedOrigin.INSTANCE).build();
}
diff --git a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
index 9dfb30d..245a35e 100644
--- a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
@@ -131,6 +131,11 @@
}
@Override
+ public ExternalR8TestBuilder addDataEntryResources(DataEntryResource... resources) {
+ throw new Unimplemented("No support for adding data entry resources");
+ }
+
+ @Override
public ExternalR8TestBuilder addProgramClasses(Collection<Class<?>> classes) {
// Adding a collection of classes will build a jar of exactly those classes so that no other
// classes are made available via a too broad classpath directory.
diff --git a/src/test/java/com/android/tools/r8/ProguardTestBuilder.java b/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
index 0fd0354..3b4647f 100644
--- a/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
@@ -181,6 +181,11 @@
}
@Override
+ public ProguardTestBuilder addDataEntryResources(DataEntryResource... resources) {
+ throw new Unimplemented("No support for adding data entry resources");
+ }
+
+ @Override
public ProguardTestBuilder addProgramClasses(Collection<Class<?>> classes) {
// Adding a collection of classes will build a jar of exactly those classes so that no other
// classes are made available via a too broad classpath directory.
diff --git a/src/test/java/com/android/tools/r8/R8CommandTest.java b/src/test/java/com/android/tools/r8/R8CommandTest.java
index 5331ca9..980e495 100644
--- a/src/test/java/com/android/tools/r8/R8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/R8CommandTest.java
@@ -619,6 +619,12 @@
runCustomResourceProcessing(false, false, 0);
}
+ @Test(expected = CompilationFailedException.class)
+ public void missingParameterForLastOption() throws CompilationFailedException {
+ DiagnosticsChecker.checkErrorsContains(
+ "Missing parameter", handler -> parse(handler, "--output"));
+ }
+
private R8Command parse(String... args) throws CompilationFailedException {
return R8Command.parse(args, EmbeddedOrigin.INSTANCE).build();
}
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index daf5568..e8555d7 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -97,6 +97,11 @@
}
@Override
+ public R8TestBuilder addDataEntryResources(DataEntryResource... resources) {
+ return addDataResources(Arrays.asList(resources));
+ }
+
+ @Override
public R8TestBuilder addKeepRuleFiles(List<Path> files) {
builder.addProguardConfigurationFiles(files);
return self();
diff --git a/src/test/java/com/android/tools/r8/R8TestRunResult.java b/src/test/java/com/android/tools/r8/R8TestRunResult.java
index 55730eb..ecf247c 100644
--- a/src/test/java/com/android/tools/r8/R8TestRunResult.java
+++ b/src/test/java/com/android/tools/r8/R8TestRunResult.java
@@ -57,7 +57,6 @@
return self();
}
-
public String proguardMap() {
return proguardMap;
}
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index 9d8e03c..a17ab09 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -46,6 +46,8 @@
return minification(false);
}
+ public abstract T addDataEntryResources(DataEntryResource... resources);
+
public abstract T addKeepRuleFiles(List<Path> files);
public T addKeepRuleFiles(Path... files) throws IOException {
@@ -62,6 +64,10 @@
return addKeepRules("-keep class ** { *; }");
}
+ public T addKeepAllInterfacesRule() {
+ return addKeepRules("-keep interface ** { *; }");
+ }
+
public T addKeepClassRules(Class<?>... classes) {
for (Class<?> clazz : classes) {
addKeepRules("-keep class " + clazz.getTypeName());
diff --git a/src/test/java/com/android/tools/r8/naming/InterfaceMethodNameMinifierTest.java b/src/test/java/com/android/tools/r8/naming/InterfaceMethodNameMinifierTest.java
new file mode 100644
index 0000000..f2e1a60
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/InterfaceMethodNameMinifierTest.java
@@ -0,0 +1,69 @@
+// 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.TestBase;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.utils.FileUtils;
+import java.nio.file.Path;
+import org.junit.Test;
+
+/** Regression test for b/123730537. */
+public class InterfaceMethodNameMinifierTest extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ Path dictionary = temp.getRoot().toPath().resolve("dictionary.txt");
+ FileUtils.writeTextFile(dictionary, "a", "b", "c");
+
+ testForR8(Backend.DEX)
+ .addInnerClasses(InterfaceMethodNameMinifierTest.class)
+ .addKeepRules(
+ "-keep,allowobfuscation interface * { <methods>; }",
+ "-keep,allowobfuscation class * { <methods>; }",
+ "-keep interface " + L.class.getTypeName() + " { <methods>; }",
+ "-obfuscationdictionary " + dictionary.toString())
+ // Minify the interface methods in alphabetic order.
+ .addOptionsModification(
+ options -> options.testing.minifier.interfaceMethodOrdering = DexMethod::slowCompareTo)
+ .compile();
+ }
+
+ interface I {}
+
+ interface J extends I {
+
+ // Will be renamed first, to a().
+ void a();
+
+ // Will be renamed secondly. Should be renamed to c(), and not b(), because `void K.b()` will
+ // be renamed to b() because `void L.b()` is reserved.
+ void c();
+ }
+
+ interface K extends I {
+
+ // Will be renamed thirdly, to b(), because `void L.b()` is reserved.
+ void b();
+ }
+
+ interface L {
+
+ // Reserved. Will be renamed together with `void K.b()`.
+ void b();
+ }
+
+ static class Implementation implements J, K {
+
+ @Override
+ public void a() {}
+
+ @Override
+ public void b() {}
+
+ @Override
+ public void c() {}
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/b124357885/B124357885Test.java b/src/test/java/com/android/tools/r8/naming/b124357885/B124357885Test.java
new file mode 100644
index 0000000..3112ae7
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/b124357885/B124357885Test.java
@@ -0,0 +1,97 @@
+// 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.b124357885;
+
+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.allOf;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.graph.DexAnnotationElement;
+import com.android.tools.r8.graph.DexValue;
+import com.android.tools.r8.graph.DexValue.DexValueArray;
+import com.android.tools.r8.graph.DexValue.DexValueString;
+import com.android.tools.r8.utils.DescriptorUtils;
+import com.android.tools.r8.utils.codeinspector.AnnotationSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
+import org.junit.Test;
+
+public class B124357885Test extends TestBase {
+
+ private void checkSignatureAnnotation(AnnotationSubject signature) {
+ DexAnnotationElement[] elements = signature.getAnnotation().elements;
+ assertEquals(1, elements.length);
+ assertEquals("value", elements[0].name.toString());
+ assertTrue(elements[0].value instanceof DexValueArray);
+ DexValueArray array = (DexValueArray) elements[0].value;
+ StringBuilder builder = new StringBuilder();
+ for (DexValue value : array.getValues()) {
+ assertTrue(value instanceof DexValueString);
+ builder.append(((DexValueString) value).value);
+ }
+ // TODO(124357885): This should be the minified name for FooImpl instead of Foo.
+ String fooDescriptor = DescriptorUtils.javaTypeToDescriptor(Foo.class.getTypeName());
+ StringBuilder expected =
+ new StringBuilder()
+ .append("()")
+ .append(fooDescriptor.substring(0, fooDescriptor.length() - 1)) // Remove the ;.
+ .append("<Ljava/lang/String;>")
+ .append(";"); // Add the ; here.
+ assertEquals(expected.toString(), builder.toString());
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(Backend.DEX)
+ .addProgramClasses(Main.class, Service.class, Foo.class, FooImpl.class)
+ .addKeepMainRule(Main.class)
+ .addKeepRules("-keepattributes Signature,InnerClasses,EnclosingMethod")
+ .compile()
+ .inspect(inspector -> {
+ assertThat(inspector.clazz(Main.class), allOf(isPresent(), not(isRenamed())));
+ assertThat(inspector.clazz(Service.class), allOf(isPresent(), isRenamed()));
+ assertThat(inspector.clazz(Foo.class), not(isPresent()));
+ assertThat(inspector.clazz(FooImpl.class), allOf(isPresent(), isRenamed()));
+ // TODO(124477502): Using uniqueMethodWithName("fooList") does not work.
+ assertEquals(1, inspector.clazz(Service.class).allMethods().size());
+ MethodSubject fooList = inspector.clazz(Service.class).allMethods().get(0);
+ AnnotationSubject signature = fooList.annotation("dalvik.annotation.Signature");
+ checkSignatureAnnotation(signature);
+ })
+ .run(Main.class)
+ .assertFailureWithErrorThatMatches(
+ containsString(
+ "java.lang.ClassNotFoundException: "
+ + "Didn't find class \"com.android.tools.r8.naming.b124357885.Foo\""));
+ }
+}
+
+class Main {
+ public static void main(String... args) throws Exception {
+ Method method = Service.class.getMethod("fooList");
+ ParameterizedType type = (ParameterizedType) method.getGenericReturnType();
+ Class<?> rawType = (Class<?>) type.getRawType();
+ System.out.println(rawType.getName());
+
+ // Convince R8 we only use subtypes to get class merging of Foo into FooImpl.
+ Foo<String> foo = new FooImpl<>();
+ System.out.println(foo);
+ }
+}
+
+interface Service {
+ Foo<String> fooList();
+}
+
+interface Foo<T> {}
+
+class FooImpl<T> implements Foo<T> {}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/LineDeltaTest.java b/src/test/java/com/android/tools/r8/naming/retrace/LineDeltaTest.java
new file mode 100644
index 0000000..4e64e7e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/retrace/LineDeltaTest.java
@@ -0,0 +1,102 @@
+// 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.retrace;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.ForceInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+
+public class LineDeltaTest extends TestBase {
+ public String runTest(Backend backend) throws Exception {
+ return testForR8(backend)
+ .enableInliningAnnotations()
+ .addProgramClasses(LineDeltaTestClass.class)
+ .addKeepMainRule(LineDeltaTestClass.class)
+ .addKeepRules("-keepattributes LineNumberTable")
+ .run(LineDeltaTestClass.class)
+ .assertSuccessWithOutput(StringUtils.lines(
+ "In test1() - 1",
+ "In test1() - 2",
+ "In test1() - 3",
+ "In test1() - 4",
+ "In test2() - 1",
+ "In test2() - 2",
+ "In test2() - 3",
+ "In test2() - 4"
+ ))
+ .proguardMap();
+ }
+
+ private long mapLines(String map) {
+ return StringUtils.splitLines(map).stream().filter(line -> !line.startsWith("#")).count();
+ }
+
+ @Test
+ public void testDex() throws Exception {
+ assertEquals(17, mapLines(runTest(Backend.DEX)));
+ }
+
+ @Test
+ public void testCf() throws Exception {
+ assertEquals(5, mapLines(runTest(Backend.CF)));
+ }
+}
+
+class LineDeltaTestClass {
+ @ForceInline
+ static void test1() {
+ System.out.println("In test1() - 1");
+ // One line comment.
+ System.out.println("In test1() - 2");
+ // Two line comments.
+ //
+ System.out.println("In test1() - 3");
+ // Four line comments.
+ //
+ //
+ //
+ System.out.println("In test1() - 4");
+ }
+
+ @ForceInline
+ static void test2() {
+ System.out.println("In test2() - 1");
+ // Seven line comments.
+ //
+ //
+ //
+ //
+ //
+ //
+ System.out.println("In test2() - 2");
+ // Eight line comments.
+ //
+ //
+ //
+ //
+ //
+ //
+ //
+ System.out.println("In test2() - 3");
+ // Nine line comments.
+ //
+ //
+ //
+ //
+ //
+ //
+ //
+ //
+ System.out.println("In test2() - 4");
+ }
+
+ public static void main(String[] args) {
+ test1();
+ test2();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
index f3aba75..cac6dd3 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -2402,12 +2402,14 @@
@Test
public void b124181032() throws Exception {
+ // Test spaces and quotes in class name list.
ProguardConfigurationParser parser;
parser = new ProguardConfigurationParser(new DexItemFactory(), reporter);
parser.parse(
createConfigurationForTesting(
ImmutableList.of(
- "-keepclassmembers class a.b.c.**, !**Client, !**Interceptor {",
+ "-keepclassmembers class \"a.b.c.**\" ,"
+ + " !**d , '!**e' , \"!**f\" , g , 'h' , \"i\" { ",
"<fields>;",
"<init>();",
"}")));
@@ -2415,6 +2417,6 @@
assertEquals(1, rules.size());
ProguardConfigurationRule rule = rules.get(0);
assertEquals(ProguardKeepRuleType.KEEP_CLASS_MEMBERS.toString(), rule.typeString());
- assertEquals("a.b.c.**,!**Client,!**Interceptor", rule.getClassNames().toString());
+ assertEquals("a.b.c.**,!**d,!**e,!**f,g,h,i", rule.getClassNames().toString());
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ServiceLoaderTest.java b/src/test/java/com/android/tools/r8/shaking/ServiceLoaderTest.java
new file mode 100644
index 0000000..e005a2c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ServiceLoaderTest.java
@@ -0,0 +1,96 @@
+// 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.shaking;
+
+import com.android.tools.r8.DataEntryResource;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.Lists;
+import java.util.List;
+import java.util.ServiceLoader;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ServiceLoaderTest extends TestBase {
+
+ private final boolean includeWorldGreeter;
+
+ @Parameters(name = "Include WorldGreeter: {0}")
+ public static Boolean[] data() {
+ return BooleanUtils.values();
+ }
+
+ public ServiceLoaderTest(boolean includeWorldGreeter) {
+ this.includeWorldGreeter = includeWorldGreeter;
+ }
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput = includeWorldGreeter ? "Hello world!" : "Hello";
+
+ List<String> serviceImplementations = Lists.newArrayList(HelloGreeter.class.getTypeName());
+ if (includeWorldGreeter) {
+ serviceImplementations.add(WorldGreeter.class.getTypeName());
+ }
+
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(ServiceLoaderTest.class)
+ .addKeepMainRule(TestClass.class)
+ // TODO(b/124181030): Test should work without the following keep-all-rules.
+ .addKeepAllClassesRule()
+ .addKeepAllInterfacesRule()
+ .addDataEntryResources(
+ DataEntryResource.fromBytes(
+ StringUtils.lines(serviceImplementations).getBytes(),
+ "META-INF/services/" + Greeter.class.getTypeName(),
+ Origin.unknown()))
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput)
+ .inspector();
+
+ // TODO(b/124181030): Verify that Greeter is merged into HelloGreeter when `includeWorldGreeter`
+ // is false.
+
+ // TODO(b/124181030): Verify that META-INF/services/...WorldGreeter is removed when
+ // `includeWorldGreeter` is false.
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ for (Greeter greeter : ServiceLoader.load(Greeter.class)) {
+ System.out.print(greeter.greeting());
+ }
+ }
+ }
+
+ public interface Greeter {
+
+ String greeting();
+ }
+
+ public static class HelloGreeter implements Greeter {
+
+ @Override
+ public String greeting() {
+ return "Hello";
+ }
+ }
+
+ public static class WorldGreeter implements Greeter {
+
+ @Override
+ public String greeting() {
+ return " world!";
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
index e518181..3ba9b99 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTest.java
@@ -8,9 +8,7 @@
@Keep
public class KeptByFieldReflectionTest {
- // TODO(b/123262024): This field must be kept un-initialized. Otherwise the "-whyareyoukeeping"
- // output tested will hit the initialization in <init> and not the reflective access.
- public int foo;
+ public int foo = 42;
public static void main(String[] args) throws Exception {
// Due to b/123210548 the object cannot be created by a reflective newInstance call.
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
index 62dcebe..6ba1f08 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByFieldReflectionTestRunner.java
@@ -30,7 +30,7 @@
private static final Class<?> CLASS = KeptByFieldReflectionTest.class;
private static final Collection<Class<?>> CLASSES = Arrays.asList(CLASS);
- private final String EXPECTED_STDOUT = StringUtils.lines("got foo: 0");
+ private final String EXPECTED_STDOUT = StringUtils.lines("got foo: 42");
private final String EXPECTED_WHYAREYOUKEEPING =
StringUtils.lines(
@@ -55,6 +55,7 @@
public void test() throws Exception {
MethodReference mainMethod = methodFromMethod(CLASS.getDeclaredMethod("main", String[].class));
FieldReference fooField = fieldFromField(CLASS.getDeclaredField("foo"));
+ MethodReference fooInit = methodFromMethod(CLASS.getDeclaredConstructor());
if (backend == Backend.CF) {
testForJvm().addProgramClasses(CLASSES).run(CLASS).assertSuccessWithOutput(EXPECTED_STDOUT);
@@ -80,6 +81,11 @@
inspector.method(mainMethod).assertNotRenamed().assertKeptBy(keepMain);
+ // The field is primarily kept by the reflective lookup in main.
inspector.field(fooField).assertRenamed().assertReflectedFrom(mainMethod);
+
+ // The field is also kept by the write in Foo.<init>.
+ // We may want to change that behavior. See b/124428834.
+ inspector.field(fooField).assertRenamed().assertKeptBy(inspector.method(fooInit));
}
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
index 8e24088..87ae2ec 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
@@ -116,4 +116,9 @@
public boolean hasLocalVariableTable() {
throw new Unreachable("Cannot determine if an absent method has a local variable table");
}
+
+ @Override
+ public AnnotationSubject annotation(String name) {
+ return new AbsentAnnotationSubject();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
index 796532d..6581a47 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.CfCode;
import com.android.tools.r8.graph.Code;
+import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexCode;
import com.android.tools.r8.graph.DexDebugEvent;
import com.android.tools.r8.graph.DexDebugInfo;
@@ -283,4 +284,13 @@
public String toString() {
return dexMethod.toSourceString();
}
+
+ @Override
+ public AnnotationSubject annotation(String name) {
+ DexAnnotation annotation = codeInspector.findAnnotation(name, dexMethod.annotations);
+ return annotation == null
+ ? new AbsentAnnotationSubject()
+ : new FoundAnnotationSubject(annotation);
+ }
+
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
index 8fb4001..1575372 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
@@ -72,4 +72,6 @@
public boolean isMethodSubject() {
return true;
}
+
+ public abstract AnnotationSubject annotation(String name);
}
diff --git a/third_party/openjdk/openjdk-9.0.4/osx.tar.gz.sha1 b/third_party/openjdk/openjdk-9.0.4/osx.tar.gz.sha1
index 7db6817..231fb73 100644
--- a/third_party/openjdk/openjdk-9.0.4/osx.tar.gz.sha1
+++ b/third_party/openjdk/openjdk-9.0.4/osx.tar.gz.sha1
@@ -1 +1 @@
-a68b718365f7ce214eec2f6bb89311885940b10e
\ No newline at end of file
+7084f18a3aad664361c52188d5f899d3050e3eb6
\ No newline at end of file
diff --git a/third_party/opensource_apps.tar.gz.sha1 b/third_party/opensource_apps.tar.gz.sha1
index 9fb2bb2..4ee5113 100644
--- a/third_party/opensource_apps.tar.gz.sha1
+++ b/third_party/opensource_apps.tar.gz.sha1
@@ -1 +1 @@
-64b0689bce8f6789320b34da4e91c6dbcc1296e9
\ No newline at end of file
+efed1b4bf62340b626cf2d0addb4f334664c0727
\ No newline at end of file
diff --git a/tools/as_utils.py b/tools/as_utils.py
index 7a6528a..817f7ed 100644
--- a/tools/as_utils.py
+++ b/tools/as_utils.py
@@ -8,8 +8,6 @@
import os
import shutil
-import utils
-
def add_r8_dependency(checkout_dir, temp_dir, minified):
build_file = os.path.join(checkout_dir, 'build.gradle')
assert os.path.isfile(build_file), (
@@ -112,7 +110,7 @@
assert 'transformDexArchiveWithDexMergerFor' not in x
return 'transformClassesAndResourcesWithR8For' in x
- assert shrinker == 'proguard'
+ assert shrinker == 'pg'
return ('transformClassesAndResourcesWithProguard' in x
or 'transformClassesWithDexBuilderFor' in x
or 'transformDexArchiveWithDexMergerFor' in x)
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index 506c583..d6dd647 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -137,7 +137,7 @@
'tivi': {
'app_id': 'app.tivi',
'git_repo': 'https://github.com/sgjesse/tivi.git',
- 'revision': '7d7f591d6f39d7caeb88dd13bf476c0c06accdfb',
+ 'revision': '25c52e3593e7c98da4e537b49b29f6f67f88754d',
'min_sdk': 23,
'compile_sdk': 28,
},
@@ -728,13 +728,15 @@
if not options.no_build or options.golem:
gradle.RunGradle(['r8', 'r8lib'])
- assert os.path.isfile(utils.R8_JAR), 'Cannot build without r8.jar'
- assert os.path.isfile(utils.R8LIB_JAR), 'Cannot build without r8lib.jar'
-
# Make a copy of r8.jar and r8lib.jar such that they stay the same for
# the entire execution of this script.
- shutil.copyfile(utils.R8_JAR, os.path.join(temp_dir, 'r8.jar'))
- shutil.copyfile(utils.R8LIB_JAR, os.path.join(temp_dir, 'r8lib.jar'))
+ all_shrinkers_enabled = (options.shrinker == None)
+ if all_shrinkers_enabled or 'r8-nolib' in options.shrinker:
+ assert os.path.isfile(utils.R8_JAR), 'Cannot build without r8.jar'
+ shutil.copyfile(utils.R8_JAR, os.path.join(temp_dir, 'r8.jar'))
+ if all_shrinkers_enabled or 'r8' in options.shrinker:
+ assert os.path.isfile(utils.R8LIB_JAR), 'Cannot build without r8lib.jar'
+ shutil.copyfile(utils.R8LIB_JAR, os.path.join(temp_dir, 'r8lib.jar'))
result_per_shrinker_per_app = {}