Version 1.4.92
Cherry pick: Extend member pool collection to library types
CL: https://r8-review.googlesource.com/c/r8/+/36640
Cherry pick: Fix missing edge in member pool collection
CL: https://r8-review.googlesource.com/c/r8/+/36700
Cherry pick: Allow archiving when triggering cls
CL: https://r8-review.googlesource.com/c/r8/+/37522
Bug: 129794426, 65810338
Change-Id: If63127dde393c5f7cef4051e0e989be7dbe156fc
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 4b23ef3..0182e69 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -484,14 +484,14 @@
if (options.enableUninstantiatedTypeOptimization) {
timing.begin("UninstantiatedTypeOptimization");
appView.setGraphLense(
- new UninstantiatedTypeOptimization(appViewWithLiveness, options)
- .run(new MethodPoolCollection(application), executorService, timing));
+ new UninstantiatedTypeOptimization(appViewWithLiveness)
+ .run(new MethodPoolCollection(appView), executorService, timing));
application = application.asDirect().rewrittenWithLense(appView.graphLense());
- timing.end();
appViewWithLiveness.setAppInfo(
appViewWithLiveness
.appInfo()
.rewrittenWithLense(application.asDirect(), appView.graphLense()));
+ timing.end();
}
}
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index bb835e0..4b0f4b8 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.4.91";
+ public static final String LABEL = "1.4.92";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index 084ad80..f6851fd 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -8,7 +8,7 @@
import com.android.tools.r8.shaking.VerticalClassMerger.VerticallyMergedClasses;
import com.android.tools.r8.utils.InternalOptions;
-public class AppView<T extends AppInfo> {
+public class AppView<T extends AppInfo> implements DexDefinitionSupplier {
private T appInfo;
private AppServices appServices;
@@ -40,6 +40,27 @@
this.appServices = appServices;
}
+ @Override
+ public final DexDefinition definitionFor(DexReference reference) {
+ return appInfo().definitionFor(reference);
+ }
+
+ @Override
+ public final DexEncodedField definitionFor(DexField field) {
+ return appInfo().definitionFor(field);
+ }
+
+ @Override
+ public final DexEncodedMethod definitionFor(DexMethod method) {
+ return appInfo().definitionFor(method);
+ }
+
+ @Override
+ public final DexClass definitionFor(DexType type) {
+ return appInfo().definitionFor(type);
+ }
+
+ @Override
public DexItemFactory dexItemFactory() {
return dexItemFactory;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexDefinitionSupplier.java b/src/main/java/com/android/tools/r8/graph/DexDefinitionSupplier.java
new file mode 100644
index 0000000..7d9888e
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/DexDefinitionSupplier.java
@@ -0,0 +1,18 @@
+// 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.graph;
+
+public interface DexDefinitionSupplier {
+
+ DexDefinition definitionFor(DexReference reference);
+
+ DexEncodedField definitionFor(DexField field);
+
+ DexEncodedMethod definitionFor(DexMethod method);
+
+ DexClass definitionFor(DexType type);
+
+ DexItemFactory dexItemFactory();
+}
diff --git a/src/main/java/com/android/tools/r8/graph/TopDownClassHierarchyTraversal.java b/src/main/java/com/android/tools/r8/graph/TopDownClassHierarchyTraversal.java
index e8d6a4f..14c98e4 100644
--- a/src/main/java/com/android/tools/r8/graph/TopDownClassHierarchyTraversal.java
+++ b/src/main/java/com/android/tools/r8/graph/TopDownClassHierarchyTraversal.java
@@ -11,59 +11,93 @@
import java.util.Set;
import java.util.function.Consumer;
-public class TopDownClassHierarchyTraversal {
+public class TopDownClassHierarchyTraversal<T extends DexClass> {
- public static void visit(
- AppView<? extends AppInfo> appView,
- Iterable<DexProgramClass> classes,
- Consumer<DexProgramClass> visitor) {
- Deque<DexProgramClass> worklist = new ArrayDeque<>();
- Set<DexProgramClass> visited = new HashSet<>();
+ private enum Scope {
+ ALL_CLASSES,
+ ONLY_PROGRAM_CLASSES
+ }
- Iterator<DexProgramClass> classIterator = classes.iterator();
+ private final DexDefinitionSupplier definitions;
+ private final Scope scope;
+
+ private final Set<T> visited = new HashSet<>();
+ private final Deque<T> worklist = new ArrayDeque<>();
+
+ private TopDownClassHierarchyTraversal(DexDefinitionSupplier definitions, Scope scope) {
+ this.definitions = definitions;
+ this.scope = scope;
+ }
+
+ /**
+ * Returns a visitor that can be used to visit all the classes (including class path and library
+ * classes) that are reachable from a given set of sources.
+ */
+ public static TopDownClassHierarchyTraversal<DexClass> forAllClasses(
+ DexDefinitionSupplier definitions) {
+ return new TopDownClassHierarchyTraversal<>(definitions, Scope.ALL_CLASSES);
+ }
+
+ /**
+ * Returns a visitor that can be used to visit all the program classes that are reachable from a
+ * given set of sources.
+ */
+ public static TopDownClassHierarchyTraversal<DexProgramClass> forProgramClasses(
+ DexDefinitionSupplier definitions) {
+ return new TopDownClassHierarchyTraversal<>(definitions, Scope.ONLY_PROGRAM_CLASSES);
+ }
+
+ public void visit(Iterable<DexProgramClass> sources, Consumer<T> visitor) {
+ Iterator<DexProgramClass> sourceIterator = sources.iterator();
// Visit the program classes in a top-down order according to the class hierarchy.
- while (classIterator.hasNext() || !worklist.isEmpty()) {
+ while (sourceIterator.hasNext() || !worklist.isEmpty()) {
if (worklist.isEmpty()) {
- // Add the ancestors of this class (including the class itself) to the worklist in such a
- // way that all super types of the class come before the class itself.
- addAncestorsToWorklist(classIterator.next(), worklist, visited, appView);
+ // Add the ancestors of the next source (including the source itself) to the worklist in
+ // such a way that all super types of the source class come before the class itself.
+ addAncestorsToWorklist(sourceIterator.next());
if (worklist.isEmpty()) {
continue;
}
}
- DexProgramClass clazz = worklist.removeFirst();
+ T clazz = worklist.removeFirst();
if (visited.add(clazz)) {
+ assert scope != Scope.ONLY_PROGRAM_CLASSES || clazz.isProgramClass();
visitor.accept(clazz);
}
}
+
+ visited.clear();
}
- private static void addAncestorsToWorklist(
- DexProgramClass clazz,
- Deque<DexProgramClass> worklist,
- Set<DexProgramClass> visited,
- AppView<? extends AppInfo> appView) {
- if (visited.contains(clazz)) {
+ private void addAncestorsToWorklist(DexClass clazz) {
+ @SuppressWarnings("unchecked")
+ T clazzWithTypeT = (T) clazz;
+
+ if (visited.contains(clazzWithTypeT)) {
return;
}
- worklist.addFirst(clazz);
+ worklist.addFirst(clazzWithTypeT);
// Add super classes to worklist.
if (clazz.superType != null) {
- DexClass definition = appView.appInfo().definitionFor(clazz.superType);
- if (definition != null && definition.isProgramClass()) {
- addAncestorsToWorklist(definition.asProgramClass(), worklist, visited, appView);
+ DexClass definition = definitions.definitionFor(clazz.superType);
+ if (definition != null) {
+ if (scope != Scope.ONLY_PROGRAM_CLASSES || definition.isProgramClass()) {
+ addAncestorsToWorklist(definition);
+ }
}
}
// Add super interfaces to worklist.
for (DexType interfaceType : clazz.interfaces.values) {
- DexClass definition = appView.appInfo().definitionFor(interfaceType);
- if (definition != null && definition.isProgramClass()) {
- addAncestorsToWorklist(definition.asProgramClass(), worklist, visited, appView);
+ DexClass definition = definitions.definitionFor(interfaceType);
+ if (definition != null) {
+ if (scope != Scope.ONLY_PROGRAM_CLASSES || definition.isProgramClass()) {
+ addAncestorsToWorklist(definition);
+ }
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 32fc687..2adc697 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -194,7 +194,7 @@
if (enableWholeProgramOptimizations) {
assert appInfo.hasLiveness();
AppInfoWithLiveness appInfoWithLiveness = appInfo.withLiveness();
- AppView<? extends AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness();
+ AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness();
assert rootSet != null;
this.nonNullTracker = new NonNullTracker(
appInfoWithLiveness, libraryMethodsReturningNonNull(appInfo.dexItemFactory));
@@ -213,7 +213,7 @@
options.enableDevirtualization ? new Devirtualizer(appViewWithLiveness) : null;
this.uninstantiatedTypeOptimization =
options.enableUninstantiatedTypeOptimization
- ? new UninstantiatedTypeOptimization(appViewWithLiveness, options)
+ ? new UninstantiatedTypeOptimization(appViewWithLiveness)
: null;
this.typeChecker = new TypeChecker(appView);
} else {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MethodPoolCollection.java b/src/main/java/com/android/tools/r8/ir/optimize/MethodPoolCollection.java
index 26dee92..52de259 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MethodPoolCollection.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MethodPoolCollection.java
@@ -4,7 +4,8 @@
package com.android.tools.r8.ir.optimize;
-import com.android.tools.r8.graph.DexApplication;
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
@@ -34,19 +35,18 @@
private static final Equivalence<DexMethod> equivalence = MethodSignatureEquivalence.get();
- private final DexApplication application;
+ private final AppView<? extends AppInfo> appView;
private final Map<DexClass, MethodPool> methodPools = new ConcurrentHashMap<>();
- public MethodPoolCollection(DexApplication application) {
- this.application = application;
+ public MethodPoolCollection(AppView<? extends AppInfo> appView) {
+ this.appView = appView;
}
public void buildAll(ExecutorService executorService, Timing timing) throws ExecutionException {
timing.begin("Building method pool collection");
try {
List<Future<?>> futures = new ArrayList<>();
- @SuppressWarnings("unchecked")
- List<DexClass> classes = (List) application.classes();
+ Iterable<? extends DexClass> classes = appView.appInfo().classes();
submitAll(classes, futures, executorService);
ThreadUtils.awaitFutures(futures);
} finally {
@@ -85,7 +85,9 @@
}
private void submitAll(
- Iterable<DexClass> classes, List<Future<?>> futures, ExecutorService executorService) {
+ Iterable<? extends DexClass> classes,
+ List<Future<?>> futures,
+ ExecutorService executorService) {
for (DexClass clazz : classes) {
futures.add(executorService.submit(computeMethodPoolPerClass(clazz)));
}
@@ -102,7 +104,7 @@
}
});
if (clazz.superType != null) {
- DexClass superClazz = application.definitionFor(clazz.superType);
+ DexClass superClazz = appView.definitionFor(clazz.superType);
if (superClazz != null) {
MethodPool superPool = methodPools.computeIfAbsent(superClazz, k -> new MethodPool());
superPool.linkSubtype(methodPool);
@@ -111,9 +113,10 @@
}
if (clazz.isInterface()) {
for (DexType subtype : clazz.type.allImmediateSubtypes()) {
- DexClass subClazz = application.definitionFor(subtype);
+ DexClass subClazz = appView.definitionFor(subtype);
if (subClazz != null) {
MethodPool childPool = methodPools.computeIfAbsent(subClazz, k -> new MethodPool());
+ methodPool.linkSubtype(childPool);
childPool.linkInterface(methodPool);
}
}
@@ -133,10 +136,10 @@
}
if (superTypes.add(clazz)) {
if (clazz.superType != null) {
- addNonNull(worklist, application.definitionFor(clazz.superType));
+ addNonNull(worklist, appView.definitionFor(clazz.superType));
}
for (DexType interfaceType : clazz.interfaces.values) {
- addNonNull(worklist, application.definitionFor(interfaceType));
+ addNonNull(worklist, appView.definitionFor(interfaceType));
}
}
}
@@ -147,20 +150,18 @@
DexClass subject, Predicate<DexClass> stoppingCriterion) {
Set<DexClass> subTypes = new HashSet<>();
Deque<DexClass> worklist = new ArrayDeque<>();
- subject.type.forAllExtendsSubtypes(
- type -> addNonNull(worklist, application.definitionFor(type)));
+ subject.type.forAllExtendsSubtypes(type -> addNonNull(worklist, appView.definitionFor(type)));
subject.type.forAllImplementsSubtypes(
- type -> addNonNull(worklist, application.definitionFor(type)));
+ type -> addNonNull(worklist, appView.definitionFor(type)));
while (!worklist.isEmpty()) {
DexClass clazz = worklist.pop();
if (stoppingCriterion.test(clazz)) {
continue;
}
if (subTypes.add(clazz)) {
- clazz.type.forAllExtendsSubtypes(
- type -> addNonNull(worklist, application.definitionFor(type)));
+ clazz.type.forAllExtendsSubtypes(type -> addNonNull(worklist, appView.definitionFor(type)));
clazz.type.forAllImplementsSubtypes(
- type -> addNonNull(worklist, application.definitionFor(type)));
+ type -> addNonNull(worklist, appView.definitionFor(type)));
}
}
return subTypes;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
index 7385237..c0d0266 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLense;
@@ -39,7 +40,6 @@
import com.android.tools.r8.ir.optimize.MethodPoolCollection.MethodPool;
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
-import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.MethodSignatureEquivalence;
import com.android.tools.r8.utils.Timing;
import com.google.common.base.Equivalence.Wrapper;
@@ -104,29 +104,20 @@
}
}
+ private static final MethodSignatureEquivalence equivalence = MethodSignatureEquivalence.get();
+
private final AppView<? extends AppInfoWithLiveness> appView;
- private final DexItemFactory dexItemFactory;
- private final InternalOptions options;
private int numberOfInstanceGetOrInstancePutWithNullReceiver = 0;
private int numberOfInvokesWithNullArgument = 0;
private int numberOfInvokesWithNullReceiver = 0;
- public UninstantiatedTypeOptimization(
- AppView<? extends AppInfoWithLiveness> appView, InternalOptions options) {
+ public UninstantiatedTypeOptimization(AppView<AppInfoWithLiveness> appView) {
this.appView = appView;
- this.dexItemFactory = appView.dexItemFactory();
- this.options = options;
}
public GraphLense run(
MethodPoolCollection methodPoolCollection, ExecutorService executorService, Timing timing) {
- BiMap<DexMethod, DexMethod> methodMapping = HashBiMap.create();
- Map<DexMethod, RemovedArgumentsInfo> removedArgumentsInfoPerMethod = new IdentityHashMap<>();
-
- MethodSignatureEquivalence equivalence = MethodSignatureEquivalence.get();
-
- Map<Wrapper<DexMethod>, Set<DexType>> changedVirtualMethods = new HashMap<>();
try {
methodPoolCollection.buildAll(executorService, timing);
@@ -134,135 +125,21 @@
throw new RuntimeException(e);
}
- TopDownClassHierarchyTraversal.visit(
- appView,
- appView.appInfo().classes(),
- clazz -> {
- MethodPool methodPool = methodPoolCollection.get(clazz);
+ Map<Wrapper<DexMethod>, Set<DexType>> changedVirtualMethods = new HashMap<>();
+ BiMap<DexMethod, DexMethod> methodMapping = HashBiMap.create();
+ Map<DexMethod, RemovedArgumentsInfo> removedArgumentsInfoPerMethod = new IdentityHashMap<>();
- if (clazz.isInterface()) {
- // Do not allow changing the prototype of methods that override an interface method.
- // This achieved by faking that there is already a method with the given signature.
- for (DexEncodedMethod virtualMethod : clazz.virtualMethods()) {
- RewrittenPrototypeDescription prototypeChanges =
- new RewrittenPrototypeDescription(
- isAlwaysNull(virtualMethod.method.proto.returnType),
- getRemovedArgumentsInfo(virtualMethod, ALLOW_ARGUMENT_REMOVAL));
- if (!prototypeChanges.isEmpty()) {
- DexMethod newMethod = getNewMethodSignature(virtualMethod, prototypeChanges);
- Wrapper<DexMethod> wrapper = equivalence.wrap(newMethod);
- if (!methodPool.hasSeenDirectly(wrapper)) {
- methodPool.seen(wrapper);
- }
- }
- }
- return;
- }
+ TopDownClassHierarchyTraversal.forProgramClasses(appView)
+ .visit(
+ appView.appInfo().classes(),
+ clazz ->
+ processClass(
+ clazz,
+ changedVirtualMethods,
+ methodMapping,
+ methodPoolCollection,
+ removedArgumentsInfoPerMethod));
- Map<DexEncodedMethod, RewrittenPrototypeDescription> prototypeChangesPerMethod =
- new IdentityHashMap<>();
- for (DexEncodedMethod directMethod : clazz.directMethods()) {
- RewrittenPrototypeDescription prototypeChanges =
- getPrototypeChanges(directMethod, ALLOW_ARGUMENT_REMOVAL);
- if (!prototypeChanges.isEmpty()) {
- prototypeChangesPerMethod.put(directMethod, prototypeChanges);
- }
- }
-
- // Reserve all signatures which are known to not be touched below.
- Set<Wrapper<DexMethod>> usedSignatures = new HashSet<>();
- for (DexEncodedMethod method : clazz.methods()) {
- if (!prototypeChangesPerMethod.containsKey(method)) {
- usedSignatures.add(equivalence.wrap(method.method));
- }
- }
-
- // Change the return type of direct methods that return an uninstantiated type to void.
- List<DexEncodedMethod> directMethods = clazz.directMethods();
- for (int i = 0; i < directMethods.size(); ++i) {
- DexEncodedMethod encodedMethod = directMethods.get(i);
- DexMethod method = encodedMethod.method;
- RewrittenPrototypeDescription prototypeChanges =
- prototypeChangesPerMethod.getOrDefault(
- encodedMethod, RewrittenPrototypeDescription.none());
- RemovedArgumentsInfo removedArgumentsInfo = prototypeChanges.getRemovedArgumentsInfo();
- DexMethod newMethod = getNewMethodSignature(encodedMethod, prototypeChanges);
- if (newMethod != method) {
- Wrapper<DexMethod> wrapper = equivalence.wrap(newMethod);
-
- // TODO(b/110806787): Can be extended to handle collisions by renaming the given
- // method.
- if (usedSignatures.add(wrapper)) {
- clazz.setDirectMethod(i, encodedMethod.toTypeSubstitutedMethod(newMethod));
- methodMapping.put(method, newMethod);
- if (removedArgumentsInfo.hasRemovedArguments()) {
- removedArgumentsInfoPerMethod.put(newMethod, removedArgumentsInfo);
- }
- }
- }
- }
-
- // Change the return type of virtual methods that return an uninstantiated type to void.
- // This is done in two steps. First we change the return type of all methods that override
- // a method whose return type has already been changed to void previously. Note that
- // all supertypes of the current class are always visited prior to the current class.
- // This is important to ensure that a method that used to override a method in its super
- // class will continue to do so after this optimization.
- List<DexEncodedMethod> virtualMethods = clazz.virtualMethods();
- for (int i = 0; i < virtualMethods.size(); ++i) {
- DexEncodedMethod encodedMethod = virtualMethods.get(i);
- DexMethod method = encodedMethod.method;
- RewrittenPrototypeDescription prototypeChanges =
- getPrototypeChanges(encodedMethod, DISALLOW_ARGUMENT_REMOVAL);
- DexMethod newMethod = getNewMethodSignature(encodedMethod, prototypeChanges);
- if (newMethod != method) {
- Wrapper<DexMethod> wrapper = equivalence.wrap(newMethod);
-
- boolean isOverrideOfPreviouslyChangedMethodInSuperClass =
- changedVirtualMethods.getOrDefault(equivalence.wrap(method), ImmutableSet.of())
- .stream()
- .anyMatch(other -> clazz.type.isSubtypeOf(other, appView.appInfo()));
- if (isOverrideOfPreviouslyChangedMethodInSuperClass) {
- assert methodPool.hasSeen(wrapper);
-
- boolean signatureIsAvailable = usedSignatures.add(wrapper);
- assert signatureIsAvailable;
-
- clazz.setVirtualMethod(i, encodedMethod.toTypeSubstitutedMethod(newMethod));
- methodMapping.put(method, newMethod);
- }
- }
- }
- for (int i = 0; i < virtualMethods.size(); ++i) {
- DexEncodedMethod encodedMethod = virtualMethods.get(i);
- DexMethod method = encodedMethod.method;
- RewrittenPrototypeDescription prototypeChanges =
- getPrototypeChanges(encodedMethod, DISALLOW_ARGUMENT_REMOVAL);
- DexMethod newMethod = getNewMethodSignature(encodedMethod, prototypeChanges);
- if (newMethod != method) {
- Wrapper<DexMethod> wrapper = equivalence.wrap(newMethod);
-
- // TODO(b/110806787): Can be extended to handle collisions by renaming the given
- // method. Note that this also requires renaming all of the methods that override this
- // method, though.
- if (!methodPool.hasSeen(wrapper) && usedSignatures.add(wrapper)) {
- methodPool.seen(wrapper);
-
- clazz.setVirtualMethod(i, encodedMethod.toTypeSubstitutedMethod(newMethod));
- methodMapping.put(method, newMethod);
-
- boolean added =
- changedVirtualMethods
- .computeIfAbsent(equivalence.wrap(method), key -> Sets.newIdentityHashSet())
- .add(clazz.type);
- assert added;
- }
- }
- }
- });
-
- // TODO(christofferqa): There is no need to do anything at the call site when the graph lense
- // is unchanged!
if (!methodMapping.isEmpty()) {
return new UninstantiatedTypeOptimizationGraphLense(
methodMapping, removedArgumentsInfoPerMethod, appView);
@@ -270,6 +147,134 @@
return appView.graphLense();
}
+ private void processClass(
+ DexProgramClass clazz,
+ Map<Wrapper<DexMethod>, Set<DexType>> changedVirtualMethods,
+ BiMap<DexMethod, DexMethod> methodMapping,
+ MethodPoolCollection methodPoolCollection,
+ Map<DexMethod, RemovedArgumentsInfo> removedArgumentsInfoPerMethod) {
+ MethodPool methodPool = methodPoolCollection.get(clazz);
+
+ if (clazz.isInterface()) {
+ // Do not allow changing the prototype of methods that override an interface method.
+ // This achieved by faking that there is already a method with the given signature.
+ for (DexEncodedMethod virtualMethod : clazz.virtualMethods()) {
+ RewrittenPrototypeDescription prototypeChanges =
+ new RewrittenPrototypeDescription(
+ isAlwaysNull(virtualMethod.method.proto.returnType),
+ getRemovedArgumentsInfo(virtualMethod, ALLOW_ARGUMENT_REMOVAL));
+ if (!prototypeChanges.isEmpty()) {
+ DexMethod newMethod = getNewMethodSignature(virtualMethod, prototypeChanges);
+ Wrapper<DexMethod> wrapper = equivalence.wrap(newMethod);
+ if (!methodPool.hasSeenDirectly(wrapper)) {
+ methodPool.seen(wrapper);
+ }
+ }
+ }
+ return;
+ }
+
+ Map<DexEncodedMethod, RewrittenPrototypeDescription> prototypeChangesPerMethod =
+ new IdentityHashMap<>();
+ for (DexEncodedMethod directMethod : clazz.directMethods()) {
+ RewrittenPrototypeDescription prototypeChanges =
+ getPrototypeChanges(directMethod, ALLOW_ARGUMENT_REMOVAL);
+ if (!prototypeChanges.isEmpty()) {
+ prototypeChangesPerMethod.put(directMethod, prototypeChanges);
+ }
+ }
+
+ // Reserve all signatures which are known to not be touched below.
+ Set<Wrapper<DexMethod>> usedSignatures = new HashSet<>();
+ for (DexEncodedMethod method : clazz.methods()) {
+ if (!prototypeChangesPerMethod.containsKey(method)) {
+ usedSignatures.add(equivalence.wrap(method.method));
+ }
+ }
+
+ // Change the return type of direct methods that return an uninstantiated type to void.
+ List<DexEncodedMethod> directMethods = clazz.directMethods();
+ for (int i = 0; i < directMethods.size(); ++i) {
+ DexEncodedMethod encodedMethod = directMethods.get(i);
+ DexMethod method = encodedMethod.method;
+ RewrittenPrototypeDescription prototypeChanges =
+ prototypeChangesPerMethod.getOrDefault(
+ encodedMethod, RewrittenPrototypeDescription.none());
+ RemovedArgumentsInfo removedArgumentsInfo = prototypeChanges.getRemovedArgumentsInfo();
+ DexMethod newMethod = getNewMethodSignature(encodedMethod, prototypeChanges);
+ if (newMethod != method) {
+ Wrapper<DexMethod> wrapper = equivalence.wrap(newMethod);
+
+ // TODO(b/110806787): Can be extended to handle collisions by renaming the given
+ // method.
+ if (usedSignatures.add(wrapper)) {
+ clazz.setDirectMethod(i, encodedMethod.toTypeSubstitutedMethod(newMethod));
+ methodMapping.put(method, newMethod);
+ if (removedArgumentsInfo.hasRemovedArguments()) {
+ removedArgumentsInfoPerMethod.put(newMethod, removedArgumentsInfo);
+ }
+ }
+ }
+ }
+
+ // Change the return type of virtual methods that return an uninstantiated type to void.
+ // This is done in two steps. First we change the return type of all methods that override
+ // a method whose return type has already been changed to void previously. Note that
+ // all supertypes of the current class are always visited prior to the current class.
+ // This is important to ensure that a method that used to override a method in its super
+ // class will continue to do so after this optimization.
+ List<DexEncodedMethod> virtualMethods = clazz.virtualMethods();
+ for (int i = 0; i < virtualMethods.size(); ++i) {
+ DexEncodedMethod encodedMethod = virtualMethods.get(i);
+ DexMethod method = encodedMethod.method;
+ RewrittenPrototypeDescription prototypeChanges =
+ getPrototypeChanges(encodedMethod, DISALLOW_ARGUMENT_REMOVAL);
+ DexMethod newMethod = getNewMethodSignature(encodedMethod, prototypeChanges);
+ if (newMethod != method) {
+ Wrapper<DexMethod> wrapper = equivalence.wrap(newMethod);
+
+ boolean isOverrideOfPreviouslyChangedMethodInSuperClass =
+ changedVirtualMethods.getOrDefault(equivalence.wrap(method), ImmutableSet.of()).stream()
+ .anyMatch(other -> clazz.type.isSubtypeOf(other, appView.appInfo()));
+ if (isOverrideOfPreviouslyChangedMethodInSuperClass) {
+ assert methodPool.hasSeen(wrapper);
+
+ boolean signatureIsAvailable = usedSignatures.add(wrapper);
+ assert signatureIsAvailable;
+
+ clazz.setVirtualMethod(i, encodedMethod.toTypeSubstitutedMethod(newMethod));
+ methodMapping.put(method, newMethod);
+ }
+ }
+ }
+ for (int i = 0; i < virtualMethods.size(); ++i) {
+ DexEncodedMethod encodedMethod = virtualMethods.get(i);
+ DexMethod method = encodedMethod.method;
+ RewrittenPrototypeDescription prototypeChanges =
+ getPrototypeChanges(encodedMethod, DISALLOW_ARGUMENT_REMOVAL);
+ DexMethod newMethod = getNewMethodSignature(encodedMethod, prototypeChanges);
+ if (newMethod != method) {
+ Wrapper<DexMethod> wrapper = equivalence.wrap(newMethod);
+
+ // TODO(b/110806787): Can be extended to handle collisions by renaming the given
+ // method. Note that this also requires renaming all of the methods that override this
+ // method, though.
+ if (!methodPool.hasSeen(wrapper) && usedSignatures.add(wrapper)) {
+ methodPool.seen(wrapper);
+
+ clazz.setVirtualMethod(i, encodedMethod.toTypeSubstitutedMethod(newMethod));
+ methodMapping.put(method, newMethod);
+
+ boolean added =
+ changedVirtualMethods
+ .computeIfAbsent(equivalence.wrap(method), key -> Sets.newIdentityHashSet())
+ .add(clazz.type);
+ assert added;
+ }
+ }
+ }
+ }
+
private RewrittenPrototypeDescription getPrototypeChanges(
DexEncodedMethod encodedMethod, Strategy strategy) {
if (ArgumentRemovalUtils.isPinned(encodedMethod, appView)
@@ -311,6 +316,8 @@
private DexMethod getNewMethodSignature(
DexEncodedMethod encodedMethod, RewrittenPrototypeDescription prototypeChanges) {
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+
DexMethod method = encodedMethod.method;
RemovedArgumentsInfo removedArgumentsInfo = prototypeChanges.getRemovedArgumentsInfo();
@@ -477,7 +484,7 @@
TypeLatticeElement.fromDexType(fieldType, true, appView.appInfo());
if (!value.getTypeLattice().lessThanOrEqual(fieldLatticeType, appView.appInfo())) {
// Broken type hierarchy. See FieldTypeTest#test_brokenTypeHierarchy.
- assert options.testing.allowTypeErrors;
+ assert appView.options().testing.allowTypeErrors;
return;
}
@@ -554,7 +561,8 @@
Value nullValue = new Value(code.valueNumberGenerator.next(), TypeLatticeElement.NULL, null);
ConstNumber constNumberInstruction = new ConstNumber(nullValue, 0);
// Note that we only keep position info for throwing instructions in release mode.
- constNumberInstruction.setPosition(options.debug ? instruction.getPosition() : Position.none());
+ constNumberInstruction.setPosition(
+ appView.options().debug ? instruction.getPosition() : Position.none());
instructionIterator.add(constNumberInstruction);
instructionIterator.next();
@@ -580,8 +588,8 @@
// target.
return;
}
- if (guard != dexItemFactory.catchAllType
- && !dexItemFactory.npeType.isSubtypeOf(guard, appView.appInfo())) {
+ if (guard != DexItemFactory.catchAllType
+ && !appView.dexItemFactory().npeType.isSubtypeOf(guard, appView.appInfo())) {
// TODO(christofferqa): Consider updating previous dominator tree instead of
// rebuilding it from scratch.
DominatorTree dominatorTree = new DominatorTree(code, MAY_HAVE_UNREACHABLE_BLOCKS);
diff --git a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
index 3b3c125..a08f5d9 100644
--- a/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
+++ b/src/main/java/com/android/tools/r8/optimize/ClassAndMemberPublicizer.java
@@ -35,7 +35,7 @@
RootSet rootSet) {
this.application = application;
this.appView = appView;
- this.methodPoolCollection = new MethodPoolCollection(application);
+ this.methodPoolCollection = new MethodPoolCollection(appView);
this.rootSet = rootSet;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 965d563..116a9e3 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -202,9 +202,7 @@
private final AppInfoWithLiveness appInfo;
private final AppView<? extends AppInfoWithLiveness> appView;
private final ExecutorService executorService;
- private final GraphLense graphLense;
private final MethodPoolCollection methodPoolCollection;
- private final InternalOptions options;
private final Timing timing;
private Collection<DexMethod> invokes;
@@ -239,9 +237,7 @@
this.appInfo = appView.appInfo();
this.appView = appView;
this.executorService = executorService;
- this.graphLense = appView.graphLense();
- this.methodPoolCollection = new MethodPoolCollection(application);
- this.options = options;
+ this.methodPoolCollection = new MethodPoolCollection(appView);
this.renamedMembersLense = new VerticalClassMergerGraphLense.Builder();
this.timing = timing;
this.mainDexClasses = mainDexClasses;
@@ -607,7 +603,7 @@
public GraphLense run() {
timing.begin("merge");
- GraphLense mergingGraphLense = mergeClasses(graphLense);
+ GraphLense mergingGraphLense = mergeClasses();
timing.end();
timing.begin("fixup");
GraphLense result = new TreeFixer().fixupTypeReferences(mergingGraphLense);
@@ -683,13 +679,14 @@
return true;
}
- private GraphLense mergeClasses(GraphLense graphLense) {
+ private GraphLense mergeClasses() {
// Visit the program classes in a top-down order according to the class hierarchy.
- TopDownClassHierarchyTraversal.visit(appView, mergeCandidates, this::mergeClassIfPossible);
+ TopDownClassHierarchyTraversal.forProgramClasses(appView)
+ .visit(mergeCandidates, this::mergeClassIfPossible);
if (Log.ENABLED) {
Log.debug(getClass(), "Merged %d classes.", mergedClasses.size());
}
- return renamedMembersLense.build(graphLense, mergedClasses, appInfo);
+ return renamedMembersLense.build(appView.graphLense(), mergedClasses, appInfo);
}
private boolean methodResolutionMayChange(DexClass source, DexClass target) {
@@ -1189,7 +1186,7 @@
SynthesizedBridgeCode code =
new SynthesizedBridgeCode(
newMethod,
- graphLense.getOriginalMethodSignature(method.method),
+ appView.graphLense().getOriginalMethodSignature(method.method),
invocationTarget.method,
invocationTarget.isPrivateMethod() ? DIRECT : STATIC);
@@ -1625,7 +1622,7 @@
}
private boolean disallowInlining(DexEncodedMethod method, DexType invocationContext) {
- if (options.enableInlining) {
+ if (appView.options().enableInlining) {
if (method.getCode().isJarCode()) {
JarCode jarCode = method.getCode().asJarCode();
ConstraintWithTarget constraint =
@@ -1686,7 +1683,7 @@
public GraphLenseLookupResult lookupMethod(DexMethod method, DexMethod context, Type type) {
// First look up the method using the existing graph lense (for example, the type will have
// changed if the method was publicized by ClassAndMemberPublicizer).
- GraphLenseLookupResult lookup = graphLense.lookupMethod(method, context, type);
+ GraphLenseLookupResult lookup = appView.graphLense().lookupMethod(method, context, type);
DexMethod previousMethod = lookup.getMethod();
Type previousType = lookup.getType();
// Then check if there is a renaming due to the vertical class merger.
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/CollisionWithLibraryMethodsTest.java b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/CollisionWithLibraryMethodsTest.java
new file mode 100644
index 0000000..28c5b2f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/CollisionWithLibraryMethodsTest.java
@@ -0,0 +1,75 @@
+// 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.ir.optimize.unusedarguments;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+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 CollisionWithLibraryMethodsTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] params() {
+ return Backend.values();
+ }
+
+ public CollisionWithLibraryMethodsTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput = StringUtils.lines("Hello world!");
+ testForR8(backend)
+ .addInnerClasses(CollisionWithLibraryMethodsTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableClassInliningAnnotations()
+ .enableInliningAnnotations()
+ .compile()
+ .inspect(this::verify)
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
+ }
+
+ private void verify(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(Greeting.class);
+ assertThat(classSubject, isPresent());
+
+ MethodSubject methodSubject = classSubject.uniqueMethodWithName("toString");
+ assertThat(methodSubject, isPresent());
+ assertEquals("java.lang.Object", methodSubject.getMethod().method.proto.parameters.toString());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(new Greeting().toString(null));
+ }
+ }
+
+ @NeverClassInline
+ static class Greeting {
+
+ @NeverInline
+ public String toString(Object unused) {
+ return "Hello world!";
+ }
+ }
+}
diff --git a/tools/archive.py b/tools/archive.py
index c37b304..e9233c7 100755
--- a/tools/archive.py
+++ b/tools/archive.py
@@ -47,6 +47,10 @@
def IsMaster(version):
branches = subprocess.check_output(['git', 'branch', '-r', '--contains',
'HEAD'])
+ # CL runs from gerrit does not have a branch, we always treat them as master
+ # commits to archive these to the hash based location
+ if len(branches) == 0:
+ return True
if not version.endswith('-dev'):
# Sanity check, we don't want to archive on top of release builds EVER
# Note that even though we branch, we never push the bots to build the same
