Merge "Add peephole infrastructure to IR"
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index fd57823..bd63ae5 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -166,6 +166,7 @@
options.enableMinification = false;
options.enableInlining = false;
options.enableClassInlining = false;
+ options.enableHorizontalClassMerging = false;
options.enableVerticalClassMerging = false;
options.enableClassStaticizer = false;
options.outline.enabled = false;
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java
index 2441034..e0f330d 100644
--- a/src/main/java/com/android/tools/r8/D8Command.java
+++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -251,6 +251,8 @@
internal.enableInlining = false;
assert internal.enableClassInlining;
internal.enableClassInlining = false;
+ assert internal.enableHorizontalClassMerging;
+ internal.enableHorizontalClassMerging = false;
assert internal.enableVerticalClassMerging;
internal.enableVerticalClassMerging = false;
assert internal.enableClassStaticizer;
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index b7c538f..94921b7 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -44,6 +44,7 @@
import com.android.tools.r8.shaking.ReasonPrinter;
import com.android.tools.r8.shaking.RootSetBuilder;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
+import com.android.tools.r8.shaking.StaticClassMerger;
import com.android.tools.r8.shaking.TreePruner;
import com.android.tools.r8.shaking.VerticalClassMerger;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -353,6 +354,14 @@
timing.end();
}
appView.setGraphLense(new MemberRebindingAnalysis(appViewWithLiveness, options).run());
+ if (options.enableHorizontalClassMerging) {
+ StaticClassMerger staticClassMerger = new StaticClassMerger(appViewWithLiveness);
+ appView.setGraphLense(staticClassMerger.run());
+ appViewWithLiveness.setAppInfo(
+ appViewWithLiveness
+ .appInfo()
+ .rewrittenWithLense(application.asDirect(), appView.graphLense()));
+ }
if (options.enableVerticalClassMerging) {
timing.begin("ClassMerger");
VerticalClassMerger verticalClassMerger =
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 7b24770..b5eafcd 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -648,6 +648,7 @@
? LineNumberOptimization.ON
: LineNumberOptimization.OFF;
+ assert internal.enableHorizontalClassMerging || !proguardConfiguration.isOptimizing();
assert internal.enableVerticalClassMerging || !proguardConfiguration.isOptimizing();
if (internal.debug) {
internal.proguardConfiguration.getKeepAttributes().lineNumberTable = true;
@@ -656,6 +657,7 @@
// TODO(zerny): Should we support inlining in debug mode? b/62937285
internal.enableInlining = false;
internal.enableClassInlining = false;
+ internal.enableHorizontalClassMerging = false;
internal.enableVerticalClassMerging = false;
internal.enableClassStaticizer = false;
// TODO(zerny): Should we support outlining in debug mode? b/62937285
diff --git a/src/main/java/com/android/tools/r8/graph/AccessFlags.java b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
index 626e5cc..afbb13f 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
@@ -72,6 +72,10 @@
return visibilityOrdinal() > other.visibilityOrdinal();
}
+ public boolean isAtLeastAsVisibleAs(AccessFlags other) {
+ return visibilityOrdinal() >= other.visibilityOrdinal();
+ }
+
private int visibilityOrdinal() {
// public > protected > package > private
if (isPublic()) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 22a771b..0ae75c5 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -157,6 +157,11 @@
return compilationState != CompilationState.NOT_PROCESSED;
}
+ public boolean isInitializer() {
+ checkIfObsolete();
+ return isInstanceInitializer() || isClassInitializer();
+ }
+
public boolean isInstanceInitializer() {
checkIfObsolete();
return accessFlags.isConstructor() && !accessFlags.isStatic();
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 c7cd614..e188f29 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
@@ -34,6 +34,7 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.desugar.CovariantReturnTypeAnnotationTransformer;
import com.android.tools.r8.ir.desugar.InterfaceMethodRewriter;
@@ -732,10 +733,8 @@
printC1VisualizerHeader(method);
printMethod(code, "Initial IR (SSA)");
- DexClass holder = definitionFor(method.method.holder);
- if (method.getCode() != null && method.getCode().isJarCode()
- && holder.hasKotlinInfo()) {
- computeKotlinNotNullParamHints(feedback, holder.getKotlinInfo(), method, code);
+ if (method.getCode() != null && method.getCode().isJarCode()) {
+ computeKotlinNotNullParamHints(feedback, method, code);
}
if (options.canHaveArtStringNewInitBug()) {
@@ -907,11 +906,19 @@
}
private void computeKotlinNotNullParamHints(
- OptimizationFeedback feedback, KotlinInfo kotlinInfo, DexEncodedMethod method, IRCode code) {
+ OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ DexMethod originalSignature = graphLense().getOriginalMethodSignature(method.method);
+ DexClass originalHolder = definitionFor(originalSignature.holder);
+ if (!originalHolder.hasKotlinInfo()) {
+ return;
+ }
+
// Use non-null parameter hints in Kotlin metadata if available.
+ KotlinInfo kotlinInfo = originalHolder.getKotlinInfo();
if (kotlinInfo.hasNonNullParameterHints()) {
- BitSet hintFromMetadata = kotlinInfo.lookupNonNullParameterHint(
- method.method.name.toString(), method.method.proto.toDescriptorString());
+ BitSet hintFromMetadata =
+ kotlinInfo.lookupNonNullParameterHint(
+ originalSignature.name.toString(), originalSignature.proto.toDescriptorString());
if (hintFromMetadata != null) {
if (hintFromMetadata.length() > 0) {
feedback.setKotlinNotNullParamHints(method, hintFromMetadata);
@@ -934,12 +941,15 @@
// to the mentioned intrinsic as the first argument. We do it only for
// code coming from Java classfile, since after the method is rewritten
// by R8 this call gets inlined.
- if (!user.isInvokeStatic() ||
- user.asInvokeMethod().getInvokedMethod() != checkParameterIsNotNull ||
- user.inValues().indexOf(argument) != 0) {
+ if (!user.isInvokeStatic()) {
continue;
}
- paramsCheckedForNull.set(index);
+ InvokeMethod invoke = user.asInvokeMethod();
+ DexMethod invokedMethod =
+ appView.graphLense().getOriginalMethodSignature(invoke.getInvokedMethod());
+ if (invokedMethod == checkParameterIsNotNull && user.inValues().indexOf(argument) == 0) {
+ paramsCheckedForNull.set(index);
+ }
}
}
if (paramsCheckedForNull.length() > 0) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 2a74cad..1660374 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -75,7 +75,7 @@
}
public boolean isBlackListed(DexEncodedMethod method) {
- return blackList.contains(method.method)
+ return blackList.contains(appView.graphLense().getOriginalMethodSignature(method.method))
|| appView.appInfo().neverInline.contains(method.method)
|| TwrCloseResourceRewriter.isSynthesizedCloseResourceMethod(method.method, converter);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ClassMergingRule.java b/src/main/java/com/android/tools/r8/shaking/ClassMergingRule.java
new file mode 100644
index 0000000..43bfe4b
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/ClassMergingRule.java
@@ -0,0 +1,82 @@
+// Copyright (c) 2018, 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.errors.Unreachable;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+import java.util.List;
+
+public class ClassMergingRule extends ProguardConfigurationRule {
+
+ public enum Type {
+ NEVER
+ }
+
+ public static class Builder extends ProguardConfigurationRule.Builder<ClassMergingRule, Builder> {
+
+ private Builder() {
+ super();
+ }
+
+ Type type;
+
+ @Override
+ public Builder self() {
+ return this;
+ }
+
+ public Builder setType(Type type) {
+ this.type = type;
+ return this;
+ }
+
+ @Override
+ public ClassMergingRule build() {
+ return new ClassMergingRule(origin, getPosition(), source, classAnnotation, classAccessFlags,
+ negatedClassAccessFlags, classTypeNegated, classType, classNames, inheritanceAnnotation,
+ inheritanceClassName, inheritanceIsExtends, memberRules, type);
+ }
+ }
+
+ private final Type type;
+
+ private ClassMergingRule(
+ Origin origin,
+ Position position,
+ String source,
+ ProguardTypeMatcher classAnnotation,
+ ProguardAccessFlags classAccessFlags,
+ ProguardAccessFlags negatedClassAccessFlags,
+ boolean classTypeNegated,
+ ProguardClassType classType,
+ ProguardClassNameList classNames,
+ ProguardTypeMatcher inheritanceAnnotation,
+ ProguardTypeMatcher inheritanceClassName,
+ boolean inheritanceIsExtends,
+ List<ProguardMemberRule> memberRules,
+ Type type) {
+ super(origin, position, source, classAnnotation, classAccessFlags, negatedClassAccessFlags,
+ classTypeNegated, classType, classNames, inheritanceAnnotation, inheritanceClassName,
+ inheritanceIsExtends, memberRules);
+ this.type = type;
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ public Type getType() {
+ return type;
+ }
+
+ @Override
+ String typeString() {
+ switch (type) {
+ case NEVER:
+ return "nevermerge";
+ }
+ throw new Unreachable("Unknown class merging type " + type);
+ }
+}
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 e0cf415..8574e6d 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1813,6 +1813,10 @@
*/
public final Set<DexMethod> neverInline;
/**
+ * All types that *must* never be merged due to a configuration directive (testing only).
+ */
+ public final Set<DexType> neverMerge;
+ /**
* All items with -identifiernamestring rule.
* Bound boolean value indicates the rule is explicitly specified by users (<code>true</code>)
* or not, i.e., implicitly added by R8 (<code>false</code>).
@@ -1871,6 +1875,7 @@
this.alwaysInline = enqueuer.rootSet.alwaysInline;
this.forceInline = enqueuer.rootSet.forceInline;
this.neverInline = enqueuer.rootSet.neverInline;
+ this.neverMerge = enqueuer.rootSet.neverMerge;
this.identifierNameStrings = joinIdentifierNameStrings(
enqueuer.rootSet.identifierNameStrings, enqueuer.identifierNameStrings);
this.prunedTypes = Collections.emptySet();
@@ -1913,6 +1918,7 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = previous.forceInline;
this.neverInline = previous.neverInline;
+ this.neverMerge = previous.neverMerge;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = mergeSets(previous.prunedTypes, removedClasses);
this.switchMaps = previous.switchMaps;
@@ -1965,6 +1971,10 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = lense.rewriteMethodsWithRenamedSignature(previous.forceInline);
this.neverInline = lense.rewriteMethodsWithRenamedSignature(previous.neverInline);
+ assert lense.assertDefinitionNotModified(
+ previous.neverMerge.stream().map(this::definitionFor).filter(Objects::nonNull)
+ .collect(Collectors.toList()));
+ this.neverMerge = previous.neverMerge;
this.identifierNameStrings =
lense.rewriteReferencesConservatively(previous.identifierNameStrings);
// Switchmap classes should never be affected by renaming.
@@ -2008,6 +2018,7 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = previous.forceInline;
this.neverInline = previous.neverInline;
+ this.neverMerge = previous.neverMerge;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = previous.prunedTypes;
this.switchMaps = switchMaps;
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 faf5317..7d5f9a5 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -364,6 +364,9 @@
} else if (allowTestOptions && acceptString("neverinline")) {
InlineRule rule = parseInlineRule(Type.NEVER, optionStart);
configurationBuilder.addRule(rule);
+ } else if (allowTestOptions && acceptString("nevermerge")) {
+ ClassMergingRule rule = parseClassMergingRule(ClassMergingRule.Type.NEVER, optionStart);
+ configurationBuilder.addRule(rule);
} else if (acceptString("useuniqueclassmembernames")) {
configurationBuilder.setUseUniqueClassMemberNames(true);
} else if (acceptString("adaptclassstrings")) {
@@ -594,6 +597,17 @@
return keepRuleBuilder.build();
}
+ private ClassMergingRule parseClassMergingRule(ClassMergingRule.Type type, Position start)
+ throws ProguardRuleParserException {
+ ClassMergingRule.Builder keepRuleBuilder =
+ ClassMergingRule.builder().setOrigin(origin).setStart(start).setType(type);
+ parseClassSpec(keepRuleBuilder, false);
+ Position end = getPosition();
+ keepRuleBuilder.setSource(getSourceSnippet(contents, start, end));
+ keepRuleBuilder.setEnd(end);
+ return keepRuleBuilder.build();
+ }
+
private InlineRule parseInlineRule(InlineRule.Type type, Position start)
throws ProguardRuleParserException {
InlineRule.Builder keepRuleBuilder = InlineRule.builder()
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index 432ae10..19eb127 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -66,6 +66,7 @@
private final Set<DexMethod> alwaysInline = Sets.newIdentityHashSet();
private final Set<DexMethod> forceInline = Sets.newIdentityHashSet();
private final Set<DexMethod> neverInline = Sets.newIdentityHashSet();
+ private final Set<DexType> neverMerge = Sets.newIdentityHashSet();
private final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking =
new IdentityHashMap<>();
private final Map<DexDefinition, ProguardMemberRule> noSideEffects = new IdentityHashMap<>();
@@ -173,6 +174,10 @@
} else if (rule instanceof ProguardAssumeNoSideEffectRule) {
markMatchingVisibleMethods(clazz, memberKeepRules, rule, null);
markMatchingFields(clazz, memberKeepRules, rule, null);
+ } else if (rule instanceof ClassMergingRule) {
+ if (allRulesSatisfied(memberKeepRules, clazz)) {
+ markClass(clazz, rule);
+ }
} else if (rule instanceof InlineRule) {
markMatchingMethods(clazz, memberKeepRules, rule, null);
} else if (rule instanceof ProguardAssumeValuesRule) {
@@ -245,6 +250,7 @@
alwaysInline,
forceInline,
neverInline,
+ neverMerge,
noSideEffects,
assumedValues,
dependentNoShrinking,
@@ -844,6 +850,16 @@
default:
throw new Unreachable();
}
+ } else if (context instanceof ClassMergingRule) {
+ switch (((ClassMergingRule) context).getType()) {
+ case NEVER:
+ if (item.isDexClass()) {
+ neverMerge.add(item.asDexClass().type);
+ }
+ break;
+ default:
+ throw new Unreachable();
+ }
} else if (context instanceof ProguardIdentifierNameStringRule) {
if (item.isDexEncodedField()) {
identifierNameStrings.add(item.asDexEncodedField().field);
@@ -864,6 +880,7 @@
public final Set<DexMethod> alwaysInline;
public final Set<DexMethod> forceInline;
public final Set<DexMethod> neverInline;
+ public final Set<DexType> neverMerge;
public final Map<DexDefinition, ProguardMemberRule> noSideEffects;
public final Map<DexDefinition, ProguardMemberRule> assumedValues;
private final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking;
@@ -880,6 +897,7 @@
Set<DexMethod> alwaysInline,
Set<DexMethod> forceInline,
Set<DexMethod> neverInline,
+ Set<DexType> neverMerge,
Map<DexDefinition, ProguardMemberRule> noSideEffects,
Map<DexDefinition, ProguardMemberRule> assumedValues,
Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking,
@@ -894,6 +912,7 @@
this.alwaysInline = Collections.unmodifiableSet(alwaysInline);
this.forceInline = Collections.unmodifiableSet(forceInline);
this.neverInline = Collections.unmodifiableSet(neverInline);
+ this.neverMerge = Collections.unmodifiableSet(neverMerge);
this.noSideEffects = Collections.unmodifiableMap(noSideEffects);
this.assumedValues = Collections.unmodifiableMap(assumedValues);
this.dependentNoShrinking = dependentNoShrinking;
diff --git a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
new file mode 100644
index 0000000..71a249f
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
@@ -0,0 +1,389 @@
+// Copyright (c) 2018, 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.graph.AppView;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
+import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.VerticalClassMerger.IllegalAccessDetector;
+import com.android.tools.r8.utils.FieldSignatureEquivalence;
+import com.android.tools.r8.utils.MethodSignatureEquivalence;
+import com.google.common.base.Equivalence.Wrapper;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Streams;
+import java.util.ArrayDeque;
+import java.util.Arrays;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+/**
+ * This optimization merges all classes that only have static members and private virtual methods.
+ *
+ * <p>If a merge candidate does not access any package-private or protected members, then it is
+ * merged into a global representative. Otherwise it is merged into a representative for its
+ * package. If no such representatives exist, then the merge candidate is promoted to be the
+ * representative.
+ *
+ * <p>Note that, when merging a merge candidate X into Y, this optimization merely moves the members
+ * of X into Y -- it does not change all occurrences of X in the program into Y. This makes the
+ * optimization more applicable, because it would otherwise not be possible to merge two classes if
+ * they inherited from, say, X' and Y' (since multiple inheritance is not allowed).
+ */
+public class StaticClassMerger {
+
+ private final AppView<? extends AppInfoWithLiveness> appView;
+
+ private final Map<String, DexProgramClass> representatives = new HashMap<>();
+
+ private DexProgramClass globalRepresentative = null;
+
+ private final BiMap<DexField, DexField> fieldMapping = HashBiMap.create();
+ private final BiMap<DexMethod, DexMethod> methodMapping = HashBiMap.create();
+
+ public StaticClassMerger(AppView<? extends AppInfoWithLiveness> appView) {
+ this.appView = appView;
+ }
+
+ // TODO(christofferqa): Share top-down traversal with vertical class merger.
+ public GraphLense run() {
+ // Visit classes in top-down order.
+ Deque<DexProgramClass> worklist = new ArrayDeque<>();
+ Set<DexProgramClass> seenBefore = new HashSet<>();
+
+ Iterator<DexProgramClass> classIterator =
+ appView.appInfo().app.classesWithDeterministicOrder().iterator();
+
+ // Visit the program classes in a top-down order according to the class hierarchy.
+ while (classIterator.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, seenBefore);
+ if (worklist.isEmpty()) {
+ continue;
+ }
+ }
+
+ DexProgramClass clazz = worklist.removeFirst();
+ if (!seenBefore.add(clazz)) {
+ continue;
+ }
+
+ if (satisfiesMergeCriteria(clazz)) {
+ merge(clazz);
+ }
+ }
+
+ BiMap<DexField, DexField> originalFieldSignatures = fieldMapping.inverse();
+ BiMap<DexMethod, DexMethod> originalMethodSignatures = methodMapping.inverse();
+ return new NestedGraphLense(
+ ImmutableMap.of(),
+ methodMapping,
+ fieldMapping,
+ originalFieldSignatures,
+ originalMethodSignatures,
+ appView.graphLense(),
+ appView.dexItemFactory());
+ }
+
+ private void addAncestorsToWorklist(
+ DexProgramClass clazz, Deque<DexProgramClass> worklist, Set<DexProgramClass> seenBefore) {
+ if (seenBefore.contains(clazz)) {
+ return;
+ }
+
+ worklist.addFirst(clazz);
+
+ // 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, seenBefore);
+ }
+ }
+
+ // 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, seenBefore);
+ }
+ }
+ }
+
+ public boolean satisfiesMergeCriteria(DexProgramClass clazz) {
+ if (appView.appInfo().neverMerge.contains(clazz.type)) {
+ return false;
+ }
+ if (clazz.accessFlags.isInterface()) {
+ return false;
+ }
+ if (clazz.staticFields().length + clazz.directMethods().length + clazz.virtualMethods().length
+ == 0) {
+ return false;
+ }
+ if (clazz.instanceFields().length > 0) {
+ return false;
+ }
+ if (Arrays.stream(clazz.staticFields())
+ .anyMatch(field -> appView.appInfo().isPinned(field.field))) {
+ return false;
+ }
+ if (Arrays.stream(clazz.directMethods()).anyMatch(DexEncodedMethod::isInitializer)) {
+ return false;
+ }
+ if (!Arrays.stream(clazz.virtualMethods()).allMatch(DexEncodedMethod::isPrivateMethod)) {
+ return false;
+ }
+ if (Streams.stream(clazz.methods())
+ .anyMatch(
+ method ->
+ method.accessFlags.isNative()
+ || appView.appInfo().isPinned(method.method)
+ // TODO(christofferqa): Remove the invariant that the graph lense should not
+ // modify any methods from the sets alwaysInline and noSideEffects.
+ || appView.appInfo().alwaysInline.contains(method.method)
+ || appView.appInfo().noSideEffects.keySet().contains(method))) {
+ return false;
+ }
+ return true;
+ }
+
+ public boolean merge(DexProgramClass clazz) {
+ assert satisfiesMergeCriteria(clazz);
+
+ String pkg = clazz.type.getPackageDescriptor();
+ DexProgramClass representativeForPackage = representatives.get(pkg);
+
+ if (mayMergeAcrossPackageBoundaries(clazz)) {
+ if (globalRepresentative != null) {
+ // Merge this class into the global representative.
+ moveMembersFromSourceToTarget(clazz, globalRepresentative);
+ return true;
+ }
+
+ // Make the current class the global representative as well as the representative for this
+ // package.
+ if (Log.ENABLED) {
+ Log.info(getClass(), "Making %s a global representative", clazz.type.toSourceString(), pkg);
+ Log.info(getClass(), "Making %s a representative for %s", clazz.type.toSourceString(), pkg);
+ }
+ globalRepresentative = clazz;
+ representatives.put(pkg, clazz);
+
+ if (representativeForPackage != null) {
+ // If there was a previous representative for this package, we can merge it into the current
+ // class that has just become the global representative.
+ moveMembersFromSourceToTarget(representativeForPackage, clazz);
+ return true;
+ }
+
+ // No merge.
+ return false;
+ }
+
+ if (representativeForPackage != null) {
+ if (clazz.accessFlags.isMoreVisibleThan(representativeForPackage.accessFlags)) {
+ // Use `clazz` as a representative for this package instead.
+ representatives.put(pkg, clazz);
+ moveMembersFromSourceToTarget(representativeForPackage, clazz);
+ return true;
+ }
+
+ // Merge current class into the representative for this package.
+ moveMembersFromSourceToTarget(clazz, representativeForPackage);
+ return true;
+ }
+
+ // No merge.
+ if (Log.ENABLED) {
+ Log.info(getClass(), "Making %s a representative for %s", clazz.type.toSourceString(), pkg);
+ }
+ representatives.put(pkg, clazz);
+ return false;
+ }
+
+ private boolean mayMergeAcrossPackageBoundaries(DexProgramClass clazz) {
+ // Check that the class is public. Otherwise, accesses to `clazz` from within its current
+ // package may become illegal.
+ if (!clazz.accessFlags.isPublic()) {
+ return false;
+ }
+ // Check that all of the members are private or public.
+ if (!Arrays.stream(clazz.directMethods())
+ .allMatch(method -> method.accessFlags.isPrivate() || method.accessFlags.isPublic())) {
+ return false;
+ }
+ if (!Arrays.stream(clazz.staticFields())
+ .allMatch(method -> method.accessFlags.isPrivate() || method.accessFlags.isPublic())) {
+ return false;
+ }
+
+ // Note that a class is only considered a candidate if it has no instance fields and all of its
+ // virtual methods are private. Therefore, we don't need to consider check if there are any
+ // package-private or protected instance fields or virtual methods here.
+ assert Arrays.stream(clazz.instanceFields()).count() == 0;
+ assert Arrays.stream(clazz.virtualMethods()).allMatch(method -> method.accessFlags.isPrivate());
+
+ // Check that no methods access package-private or protected members.
+ IllegalAccessDetector registry = new IllegalAccessDetector(appView.appInfo(), clazz);
+ for (DexEncodedMethod method : clazz.methods()) {
+ method.registerCodeReferences(registry);
+ if (registry.foundIllegalAccess()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private void moveMembersFromSourceToTarget(
+ DexProgramClass sourceClass, DexProgramClass targetClass) {
+ if (Log.ENABLED) {
+ Log.info(
+ getClass(),
+ "Merging %s into %s",
+ sourceClass.type.toSourceString(),
+ targetClass.type.toSourceString());
+ }
+
+ assert targetClass.accessFlags.isAtLeastAsVisibleAs(sourceClass.accessFlags);
+ assert sourceClass.instanceFields().length == 0;
+ assert targetClass.instanceFields().length == 0;
+
+ // Move members from source to target.
+ targetClass.setDirectMethods(
+ mergeMethods(sourceClass.directMethods(), targetClass.directMethods(), targetClass));
+ targetClass.setVirtualMethods(
+ mergeMethods(sourceClass.virtualMethods(), targetClass.virtualMethods(), targetClass));
+ targetClass.setStaticFields(
+ mergeFields(sourceClass.staticFields(), targetClass.staticFields(), targetClass));
+
+ // Cleanup source.
+ sourceClass.setDirectMethods(DexEncodedMethod.EMPTY_ARRAY);
+ sourceClass.setVirtualMethods(DexEncodedMethod.EMPTY_ARRAY);
+ sourceClass.setStaticFields(DexEncodedField.EMPTY_ARRAY);
+ }
+
+ private DexEncodedMethod[] mergeMethods(
+ DexEncodedMethod[] sourceMethods,
+ DexEncodedMethod[] targetMethods,
+ DexProgramClass targetClass) {
+ DexEncodedMethod[] result = new DexEncodedMethod[sourceMethods.length + targetMethods.length];
+
+ // Move all target methods to result.
+ System.arraycopy(targetMethods, 0, result, 0, targetMethods.length);
+
+ // Move source methods to result one by one, renaming them if needed.
+ MethodSignatureEquivalence equivalence = MethodSignatureEquivalence.get();
+ Set<Wrapper<DexMethod>> existingMethods =
+ Arrays.stream(targetMethods)
+ .map(targetMethod -> equivalence.wrap(targetMethod.method))
+ .collect(Collectors.toSet());
+
+ Predicate<DexMethod> availableMethodSignatures =
+ method -> !existingMethods.contains(equivalence.wrap(method));
+
+ int i = targetMethods.length;
+ for (DexEncodedMethod sourceMethod : sourceMethods) {
+ DexEncodedMethod sourceMethodAfterMove =
+ renameMethodIfNeeded(sourceMethod, targetClass, availableMethodSignatures);
+ result[i++] = sourceMethodAfterMove;
+
+ DexMethod originalMethod =
+ methodMapping.inverse().getOrDefault(sourceMethod.method, sourceMethod.method);
+ methodMapping.put(originalMethod, sourceMethodAfterMove.method);
+ }
+
+ return result;
+ }
+
+ private DexEncodedField[] mergeFields(
+ DexEncodedField[] sourceFields, DexEncodedField[] targetFields, DexProgramClass targetClass) {
+ DexEncodedField[] result = new DexEncodedField[sourceFields.length + targetFields.length];
+
+ // Move all target fields to result.
+ System.arraycopy(targetFields, 0, result, 0, targetFields.length);
+
+ // Move source fields to result one by one, renaming them if needed.
+ FieldSignatureEquivalence equivalence = FieldSignatureEquivalence.get();
+ Set<Wrapper<DexField>> existingFields =
+ Arrays.stream(targetFields)
+ .map(targetField -> equivalence.wrap(targetField.field))
+ .collect(Collectors.toSet());
+
+ Predicate<DexField> availableFieldSignatures =
+ field -> !existingFields.contains(equivalence.wrap(field));
+
+ int i = targetFields.length;
+ for (DexEncodedField sourceField : sourceFields) {
+ DexEncodedField sourceFieldAfterMove =
+ renameFieldIfNeeded(sourceField, targetClass, availableFieldSignatures);
+ result[i++] = sourceFieldAfterMove;
+
+ DexField originalField =
+ fieldMapping.inverse().getOrDefault(sourceField.field, sourceField.field);
+ fieldMapping.put(originalField, sourceFieldAfterMove.field);
+ }
+
+ return result;
+ }
+
+ private DexEncodedMethod renameMethodIfNeeded(
+ DexEncodedMethod method,
+ DexProgramClass targetClass,
+ Predicate<DexMethod> availableMethodSignatures) {
+ assert !method.accessFlags.isConstructor();
+ DexString oldName = method.method.name;
+ DexMethod newSignature =
+ appView.dexItemFactory().createMethod(targetClass.type, method.method.proto, oldName);
+ if (!availableMethodSignatures.test(newSignature)) {
+ int count = 1;
+ do {
+ DexString newName = appView.dexItemFactory().createString(oldName.toSourceString() + count);
+ newSignature =
+ appView.dexItemFactory().createMethod(targetClass.type, method.method.proto, newName);
+ count++;
+ } while (!availableMethodSignatures.test(newSignature));
+ }
+ return method.toTypeSubstitutedMethod(newSignature);
+ }
+
+ private DexEncodedField renameFieldIfNeeded(
+ DexEncodedField field,
+ DexProgramClass targetClass,
+ Predicate<DexField> availableFieldSignatures) {
+ DexString oldName = field.field.name;
+ DexField newSignature =
+ appView.dexItemFactory().createField(targetClass.type, field.field.type, oldName);
+ if (!availableFieldSignatures.test(newSignature)) {
+ int count = 1;
+ do {
+ DexString newName = appView.dexItemFactory().createString(oldName.toSourceString() + count);
+ newSignature =
+ appView.dexItemFactory().createField(targetClass.type, field.field.type, newName);
+ count++;
+ } while (!availableFieldSignatures.test(newSignature));
+ }
+ return field.toTypeSubstitutedField(newSignature);
+ }
+}
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 d41bd69..df4c786 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppInfo.ResolutionResult;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.Code;
@@ -18,7 +19,6 @@
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
-import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
@@ -353,7 +353,8 @@
if (appInfo.instantiatedTypes.contains(clazz.type)
|| appInfo.instantiatedLambdas.contains(clazz.type)
|| appInfo.isPinned(clazz.type)
- || pinnedTypes.contains(clazz.type)) {
+ || pinnedTypes.contains(clazz.type)
+ || appInfo.neverMerge.contains(clazz.type)) {
return false;
}
// Note that the property "singleSubtype == null" cannot change during merging, since we visit
@@ -477,7 +478,7 @@
// Check that all accesses from [source] to classes or members from the current package of
// [source] will continue to work. This is guaranteed if the methods of [source] do not access
// any private or protected classes or members from the current package of [source].
- IllegalAccessDetector registry = new IllegalAccessDetector(options.itemFactory, source);
+ IllegalAccessDetector registry = new IllegalAccessDetector(appInfo, source);
for (DexEncodedMethod method : source.methods()) {
method.registerCodeReferences(registry);
if (registry.foundIllegalAccess()) {
@@ -1334,7 +1335,7 @@
private class TreeFixer {
private final Builder lense = GraphLense.builder();
- Map<DexProto, DexProto> protoFixupCache = new IdentityHashMap<>();
+ private final Map<DexProto, DexProto> protoFixupCache = new IdentityHashMap<>();
private GraphLense fixupTypeReferences(GraphLense graphLense) {
// Globally substitute merged class types in protos and holders.
@@ -1345,7 +1346,7 @@
clazz.setStaticFields(substituteTypesIn(clazz.staticFields()));
clazz.setInstanceFields(substituteTypesIn(clazz.instanceFields()));
}
- // Record type renamings so instanceof and checkcast checks are also fixed.
+ // Record type renamings so check-cast and instance-of checks are also fixed.
for (DexType type : mergedClasses.keySet()) {
DexType fixed = fixupType(type);
lense.map(type, fixed);
@@ -1649,12 +1650,16 @@
// Searches for a reference to a non-public class, field or method declared in the same package
// as [source].
- private class IllegalAccessDetector extends UseRegistry {
- private boolean foundIllegalAccess = false;
- private DexClass source;
+ public static class IllegalAccessDetector extends UseRegistry {
- public IllegalAccessDetector(DexItemFactory factory, DexClass source) {
- super(factory);
+ private boolean foundIllegalAccess = false;
+
+ private final AppInfo appInfo;
+ private final DexClass source;
+
+ public IllegalAccessDetector(AppInfo appInfo, DexClass source) {
+ super(appInfo.dexItemFactory);
+ this.appInfo = appInfo;
this.source = source;
}
@@ -1664,7 +1669,8 @@
private boolean checkFieldReference(DexField field) {
if (!foundIllegalAccess) {
- if (field.clazz.isSamePackage(source.type)) {
+ DexType baseType = field.clazz.toBaseType(appInfo.dexItemFactory);
+ if (baseType.isClassType() && baseType.isSamePackage(source.type)) {
checkTypeReference(field.clazz);
checkTypeReference(field.type);
@@ -1679,7 +1685,8 @@
private boolean checkMethodReference(DexMethod method) {
if (!foundIllegalAccess) {
- if (method.holder.isSamePackage(source.type)) {
+ DexType baseType = method.holder.toBaseType(appInfo.dexItemFactory);
+ if (baseType.isClassType() && baseType.isSamePackage(source.type)) {
checkTypeReference(method.holder);
checkTypeReference(method.proto.returnType);
for (DexType type : method.proto.parameters.values) {
@@ -1696,8 +1703,9 @@
private boolean checkTypeReference(DexType type) {
if (!foundIllegalAccess) {
- if (type.isClassType() && type.isSamePackage(source.type)) {
- DexClass clazz = appInfo.definitionFor(type);
+ DexType baseType = type.toBaseType(appInfo.dexItemFactory);
+ if (baseType.isClassType() && baseType.isSamePackage(source.type)) {
+ DexClass clazz = appInfo.definitionFor(baseType);
if (clazz == null || !clazz.accessFlags.isPublic()) {
foundIllegalAccess = true;
}
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 65d1e3a..c76841d 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -79,6 +79,7 @@
itemFactory = proguardConfiguration.getDexItemFactory();
// -dontoptimize disables optimizations by flipping related flags.
if (!proguardConfiguration.isOptimizing()) {
+ enableHorizontalClassMerging = false;
enableVerticalClassMerging = false;
enableDevirtualization = false;
enableNonNullTracking = false;
@@ -97,6 +98,7 @@
public boolean passthroughDexCode = false;
// Optimization-related flags. These should conform to -dontoptimize.
+ public boolean enableHorizontalClassMerging = true;
public boolean enableVerticalClassMerging = true;
public boolean enableDevirtualization = true;
public boolean enableNonNullTracking = true;
diff --git a/src/test/java/com/android/tools/r8/D8TestBuilder.java b/src/test/java/com/android/tools/r8/D8TestBuilder.java
index 21a5e0e..4897701 100644
--- a/src/test/java/com/android/tools/r8/D8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/D8TestBuilder.java
@@ -5,19 +5,19 @@
import com.android.tools.r8.D8Command.Builder;
import com.android.tools.r8.TestBase.Backend;
+import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
import java.util.function.Consumer;
+import java.util.function.Supplier;
-public class D8TestBuilder extends TestCompilerBuilder<D8Command, Builder, D8TestBuilder> {
+public class D8TestBuilder
+ extends TestCompilerBuilder<D8Command, Builder, D8TestCompileResult, D8TestBuilder> {
- private final D8Command.Builder builder;
-
- private D8TestBuilder(TestState state, D8Command.Builder builder) {
+ private D8TestBuilder(TestState state, Builder builder) {
super(state, builder, Backend.DEX);
- this.builder = builder;
}
public static D8TestBuilder create(TestState state) {
@@ -30,9 +30,11 @@
}
@Override
- void internalCompile(Builder builder, Consumer<InternalOptions> optionsConsumer)
+ D8TestCompileResult internalCompile(
+ Builder builder, Consumer<InternalOptions> optionsConsumer, Supplier<AndroidApp> app)
throws CompilationFailedException {
ToolHelper.runD8(builder, optionsConsumer);
+ return new D8TestCompileResult(getState(), app.get());
}
public D8TestBuilder addClasspathClasses(Class<?>... classes) {
diff --git a/src/test/java/com/android/tools/r8/D8TestCompileResult.java b/src/test/java/com/android/tools/r8/D8TestCompileResult.java
new file mode 100644
index 0000000..482a611
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/D8TestCompileResult.java
@@ -0,0 +1,18 @@
+// Copyright (c) 2018, 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;
+
+import com.android.tools.r8.TestBase.Backend;
+import com.android.tools.r8.utils.AndroidApp;
+
+public class D8TestCompileResult extends TestCompileResult {
+ D8TestCompileResult(TestState state, AndroidApp app) {
+ super(state, app);
+ }
+
+ @Override
+ public Backend getBackend() {
+ return Backend.DEX;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/NeverMerge.java b/src/test/java/com/android/tools/r8/NeverMerge.java
new file mode 100644
index 0000000..7fc97b9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/NeverMerge.java
@@ -0,0 +1,6 @@
+// Copyright (c) 2018, 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;
+
+public @interface NeverMerge {}
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index 74b40b0..3b01a71 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.R8Command.Builder;
import com.android.tools.r8.TestBase.Backend;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.StringUtils;
import java.util.ArrayList;
@@ -13,14 +14,13 @@
import java.util.Collection;
import java.util.List;
import java.util.function.Consumer;
+import java.util.function.Supplier;
-public class R8TestBuilder extends TestCompilerBuilder<R8Command, Builder, R8TestBuilder> {
-
- private final R8Command.Builder builder;
+public class R8TestBuilder
+ extends TestCompilerBuilder<R8Command, Builder, R8TestCompileResult, R8TestBuilder> {
private R8TestBuilder(TestState state, Builder builder, Backend backend) {
super(state, builder, backend);
- this.builder = builder;
}
public static R8TestBuilder create(TestState state, Backend backend) {
@@ -35,12 +35,16 @@
}
@Override
- void internalCompile(Builder builder, Consumer<InternalOptions> optionsConsumer)
+ R8TestCompileResult internalCompile(
+ Builder builder, Consumer<InternalOptions> optionsConsumer, Supplier<AndroidApp> app)
throws CompilationFailedException {
if (enableInliningAnnotations) {
ToolHelper.allowTestProguardOptions(builder);
}
+ StringBuilder proguardMapBuilder = new StringBuilder();
+ builder.setProguardMapConsumer((string, ignore) -> proguardMapBuilder.append(string));
ToolHelper.runR8WithoutResult(builder.build(), optionsConsumer);
+ return new R8TestCompileResult(getState(), backend, app.get(), proguardMapBuilder.toString());
}
public R8TestBuilder addDataResources(List<DataEntryResource> resources) {
diff --git a/src/test/java/com/android/tools/r8/R8TestCompileResult.java b/src/test/java/com/android/tools/r8/R8TestCompileResult.java
new file mode 100644
index 0000000..45fb31c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/R8TestCompileResult.java
@@ -0,0 +1,32 @@
+// Copyright (c) 2018, 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;
+
+import com.android.tools.r8.TestBase.Backend;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+
+public class R8TestCompileResult extends TestCompileResult {
+
+ private final Backend backend;
+ private final String proguardMap;
+
+ R8TestCompileResult(TestState state, Backend backend, AndroidApp app, String proguardMap) {
+ super(state, app);
+ this.backend = backend;
+ this.proguardMap = proguardMap;
+ }
+
+ @Override
+ public Backend getBackend() {
+ return backend;
+ }
+
+ @Override
+ public CodeInspector inspector() throws IOException, ExecutionException {
+ return new CodeInspector(app, proguardMap);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/TestBuilder.java b/src/test/java/com/android/tools/r8/TestBuilder.java
index 6d5a158..7397c39 100644
--- a/src/test/java/com/android/tools/r8/TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestBuilder.java
@@ -53,7 +53,15 @@
}
public T addProgramClassesAndInnerClasses(Collection<Class<?>> classes) throws IOException {
- return addProgramFiles(getFilesForClassesAndInnerClasses(classes));
+ return addProgramClasses(classes).addInnerClasses(classes);
+ }
+
+ public T addInnerClasses(Class<?>... classes) throws IOException {
+ return addInnerClasses(Arrays.asList(classes));
+ }
+
+ public T addInnerClasses(Collection<Class<?>> classes) throws IOException {
+ return addProgramFiles(getFilesForInnerClasses(classes));
}
public abstract T addLibraryFiles(Collection<Path> files);
@@ -74,15 +82,14 @@
return ListUtils.map(classes, ToolHelper::getClassFileForTestClass);
}
- static Collection<Path> getFilesForClassesAndInnerClasses(Collection<Class<?>> classes)
- throws IOException {
+ static Collection<Path> getFilesForInnerClasses(Collection<Class<?>> classes) throws IOException {
Set<Path> paths = new HashSet<>();
for (Class clazz : classes) {
Path path = ToolHelper.getClassFileForTestClass(clazz);
String prefix = path.toString().replace(CLASS_EXTENSION, "$");
paths.addAll(
ToolHelper.getClassFilesForTestDirectory(
- path.getParent(), p -> p.equals(path) || p.toString().startsWith(prefix)));
+ path.getParent(), p -> p.toString().startsWith(prefix)));
}
return paths;
}
diff --git a/src/test/java/com/android/tools/r8/TestCompileResult.java b/src/test/java/com/android/tools/r8/TestCompileResult.java
index 4e7cf37..7505a83 100644
--- a/src/test/java/com/android/tools/r8/TestCompileResult.java
+++ b/src/test/java/com/android/tools/r8/TestCompileResult.java
@@ -12,19 +12,23 @@
import java.nio.file.Path;
import java.util.concurrent.ExecutionException;
-public class TestCompileResult {
- private final TestState state;
- private final Backend backend;
- private final AndroidApp app;
+public abstract class TestCompileResult {
+ final TestState state;
+ final AndroidApp app;
- public TestCompileResult(TestState state, Backend backend, AndroidApp app) {
+ TestCompileResult(TestState state, AndroidApp app) {
this.state = state;
- this.backend = backend;
this.app = app;
}
+ public abstract Backend getBackend();
+
+ public TestRunResult run(Class<?> mainClass) throws IOException {
+ return run(mainClass.getTypeName());
+ }
+
public TestRunResult run(String mainClass) throws IOException {
- switch (backend) {
+ switch (getBackend()) {
case DEX:
return runArt(mainClass);
case CF:
@@ -34,6 +38,12 @@
}
}
+ public TestCompileResult writeToZip(Path file) throws IOException {
+ app.writeToZip(
+ file, getBackend() == Backend.DEX ? OutputMode.DexIndexed : OutputMode.ClassFile);
+ return this;
+ }
+
public CodeInspector inspector() throws IOException, ExecutionException {
return new CodeInspector(app);
}
diff --git a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
index db25352..4706125 100644
--- a/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestCompilerBuilder.java
@@ -5,17 +5,21 @@
import com.android.tools.r8.TestBase.Backend;
import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.AndroidAppConsumers;
import com.android.tools.r8.utils.InternalOptions;
+import com.google.common.base.Suppliers;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Collection;
import java.util.function.Consumer;
+import java.util.function.Supplier;
public abstract class TestCompilerBuilder<
C extends BaseCompilerCommand,
B extends BaseCompilerCommand.Builder<C, B>,
- T extends TestCompilerBuilder<C, B, T>>
+ R extends TestCompileResult,
+ T extends TestCompilerBuilder<C, B, R, T>>
extends TestBuilder<T> {
public static final Consumer<InternalOptions> DEFAULT_OPTIONS =
@@ -24,8 +28,8 @@
public void accept(InternalOptions options) {}
};
- private final B builder;
- private final Backend backend;
+ final B builder;
+ final Backend backend;
// Default initialized setup. Can be overwritten if needed.
private Path defaultLibrary;
@@ -43,7 +47,8 @@
abstract T self();
- abstract void internalCompile(B builder, Consumer<InternalOptions> optionsConsumer)
+ abstract R internalCompile(
+ B builder, Consumer<InternalOptions> optionsConsumer, Supplier<AndroidApp> app)
throws CompilationFailedException;
public T addOptionsModification(Consumer<InternalOptions> optionsConsumer) {
@@ -51,7 +56,7 @@
return self();
}
- public TestCompileResult compile() throws CompilationFailedException {
+ public R compile() throws CompilationFailedException {
AndroidAppConsumers sink = new AndroidAppConsumers();
builder.setProgramConsumer(sink.wrapProgramConsumer(programConsumer));
if (defaultLibrary != null) {
@@ -60,8 +65,7 @@
if (backend == Backend.DEX && defaultMinApiLevel != null) {
builder.setMinApiLevel(defaultMinApiLevel.getLevel());
}
- internalCompile(builder, optionsConsumer);
- return new TestCompileResult(getState(), backend, sink.build());
+ return internalCompile(builder, optionsConsumer, Suppliers.memoize(sink::build));
}
@Override
diff --git a/src/test/java/com/android/tools/r8/cf/BootstrapCurrentEqualityTest.java b/src/test/java/com/android/tools/r8/cf/BootstrapCurrentEqualityTest.java
index 7202914..d2d434a 100644
--- a/src/test/java/com/android/tools/r8/cf/BootstrapCurrentEqualityTest.java
+++ b/src/test/java/com/android/tools/r8/cf/BootstrapCurrentEqualityTest.java
@@ -36,8 +36,8 @@
import org.junit.rules.TemporaryFolder;
/**
- * This test relies on a freshly built from builds/libs/r8lib_with_deps.jar. If this test fails
- * rebuild r8lib_with_deps by calling test.py or gradle r8libWithdeps.
+ * This test relies on a freshly built builds/libs/r8lib_with_deps.jar. If this test fails
+ * remove build directory and rebuild r8lib_with_deps by calling test.py or gradle r8libWithdeps.
*/
public class BootstrapCurrentEqualityTest extends TestBase {
diff --git a/src/test/java/com/android/tools/r8/classmerging/StaticClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/StaticClassMergerTest.java
new file mode 100644
index 0000000..2d2788c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/StaticClassMergerTest.java
@@ -0,0 +1,87 @@
+// Copyright (c) 2018, 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.classmerging;
+
+import static org.junit.Assert.assertEquals;
+
+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.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
+import java.util.List;
+import java.util.stream.Collectors;
+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 StaticClassMergerTest extends TestBase {
+
+ static class StaticMergeCandidateA {
+
+ @NeverInline
+ public static String m() {
+ return "StaticMergeCandidateA.m()";
+ }
+ }
+
+ static class StaticMergeCandidateB {
+
+ @NeverInline
+ public static String m() {
+ return "StaticMergeCandidateB.m()";
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(StaticMergeCandidateA.m());
+ System.out.print(StaticMergeCandidateB.m());
+ }
+ }
+
+ private Backend backend;
+
+ public StaticClassMergerTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ @Test
+ public void testStaticClassIsRemoved() throws Exception {
+ String expected =
+ StringUtils.joinLines("StaticMergeCandidateA.m()", "StaticMergeCandidateB.m()");
+
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expected);
+
+ CodeInspector inspector =
+ testForR8(backend)
+ .addInnerClasses(StaticClassMergerTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addOptionsModification(options -> options.enableMinification = false)
+ .enableInliningAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expected)
+ .inspector();
+
+ // Check that one of the two static merge candidates has been removed
+ List<FoundClassSubject> classes =
+ inspector.allClasses().stream()
+ .filter(clazz -> clazz.getOriginalName().contains("StaticMergeCandidate"))
+ .collect(Collectors.toList());
+ assertEquals(1, classes.size());
+
+ // Check that the remaining static merge candidate has two methods.
+ FoundClassSubject remaining = classes.get(0);
+ assertEquals(2, remaining.allMethods().size());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
similarity index 99%
rename from src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
rename to src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
index 7a1f831..29f4605 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerTest.java
@@ -61,7 +61,7 @@
// TODO(christofferqa): Add tests to check that statically typed invocations on method handles
// continue to work after class merging.
@RunWith(VmTestRunner.class)
-public class ClassMergingTest extends TestBase {
+public class VerticalClassMergerTest extends TestBase {
private static final Path CF_DIR =
Paths.get(ToolHelper.BUILD_DIR).resolve("test/examples/classes/classmerging");
@@ -1154,7 +1154,7 @@
public void test(AndroidApp app, Path proguardMapPath) throws Throwable {
Path appPath =
- File.createTempFile("app", ".zip", ClassMergingTest.this.temp.getRoot()).toPath();
+ File.createTempFile("app", ".zip", VerticalClassMergerTest.this.temp.getRoot()).toPath();
app.writeToZip(appPath, OutputMode.DexIndexed);
DexDebugTestConfig config = new DexDebugTestConfig(appPath);
diff --git a/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java b/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
index b1881ee..d83c891 100644
--- a/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
+++ b/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.CompatProguardCommandBuilder;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
@@ -26,8 +27,10 @@
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
+@NeverMerge
class Outer {
+ @NeverMerge
static class Inner {
@NeverInline
static void foo() {
@@ -79,8 +82,8 @@
// Note that reproducing b/116840216 relies on the order of following rules that cause
// the visiting of classes during class minification to be Outer$Inner before Outer.
"-keepnames class " + Outer.class.getCanonicalName() + "$Inner",
- keepOuterName ? "-keepnames class " + Outer.class.getCanonicalName() : ""
- ),
+ keepOuterName ? "-keepnames class " + Outer.class.getCanonicalName() : "",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *"),
Origin.unknown());
ToolHelper.allowTestProguardOptions(builder);
@@ -109,7 +112,6 @@
assertThat(foo, isRenamed());
}
-
@Test
public void test_keepOuterName() throws Exception {
runTest(true);
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/A.java b/src/test/java/com/android/tools/r8/shaking/testrules/A.java
index af05562..cf97eeb 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/A.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/A.java
@@ -4,6 +4,9 @@
package com.android.tools.r8.shaking.testrules;
+import com.android.tools.r8.NeverMerge;
+
+@NeverMerge
public class A {
public static int m(int a, int b) {
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/C.java b/src/test/java/com/android/tools/r8/shaking/testrules/C.java
index 5ee4d53..79370a4 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/C.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/C.java
@@ -4,6 +4,9 @@
package com.android.tools.r8.shaking.testrules;
+import com.android.tools.r8.NeverMerge;
+
+@NeverMerge
public class C {
private static int i;
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java b/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
index f37d6c6..96894df 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
@@ -66,10 +66,12 @@
@Test
public void testDefaultInlining() throws Exception {
- CodeInspector inspector = runTest(ImmutableList.of(
- "-keep class **.Main { *; }",
- "-dontobfuscate"
- ));
+ CodeInspector inspector =
+ runTest(
+ ImmutableList.of(
+ "-keep class **.Main { *; }",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
ClassSubject classB = inspector.clazz(B.class);
@@ -92,12 +94,14 @@
@Test
public void testNeverInline() throws Exception {
- CodeInspector inspector = runTest(ImmutableList.of(
- "-neverinline class **.A { method(); }",
- "-neverinline class **.B { method(); }",
- "-keep class **.Main { *; }",
- "-dontobfuscate"
- ));
+ CodeInspector inspector =
+ runTest(
+ ImmutableList.of(
+ "-neverinline class **.A { method(); }",
+ "-neverinline class **.B { method(); }",
+ "-keep class **.Main { *; }",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
ClassSubject classB = inspector.clazz(B.class);
@@ -117,12 +121,14 @@
@Test
public void testForceInline() throws Exception {
- CodeInspector inspector = runTest(ImmutableList.of(
- "-forceinline class **.A { int m(int, int); }",
- "-forceinline class **.B { int m(int, int); }",
- "-keep class **.Main { *; }",
- "-dontobfuscate"
- ));
+ CodeInspector inspector =
+ runTest(
+ ImmutableList.of(
+ "-forceinline class **.A { int m(int, int); }",
+ "-forceinline class **.B { int m(int, int); }",
+ "-keep class **.Main { *; }",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
ClassSubject classB = inspector.clazz(B.class);
@@ -139,11 +145,13 @@
@Test
public void testForceInlineFails() throws Exception {
try {
- CodeInspector inspector = runTest(ImmutableList.of(
- "-forceinline class **.A { int x(); }",
- "-keep class **.Main { *; }",
- "-dontobfuscate"
- ));
+ CodeInspector inspector =
+ runTest(
+ ImmutableList.of(
+ "-forceinline class **.A { int x(); }",
+ "-keep class **.Main { *; }",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-dontobfuscate"));
fail("Force inline of non-inlinable method succeeded");
} catch (Throwable t) {
// Ignore assertion error.
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
index 28404f0..660156a 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
@@ -26,6 +26,7 @@
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.naming.signature.GenericSignatureAction;
import com.android.tools.r8.naming.signature.GenericSignatureParser;
+import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
@@ -111,10 +112,18 @@
return internalOptions;
}
- public CodeInspector(AndroidApp app, Path proguardMap) throws IOException, ExecutionException {
+ public CodeInspector(AndroidApp app, Path proguardMapFile)
+ throws IOException, ExecutionException {
this(
new ApplicationReader(app, runOptionsConsumer(null), new Timing("CodeInspector"))
- .read(StringResource.fromFile(proguardMap)));
+ .read(StringResource.fromFile(proguardMapFile)));
+ }
+
+ public CodeInspector(AndroidApp app, String proguardMapContent)
+ throws IOException, ExecutionException {
+ this(
+ new ApplicationReader(app, runOptionsConsumer(null), new Timing("CodeInspector"))
+ .read(StringResource.fromString(proguardMapContent, Origin.unknown())));
}
public CodeInspector(DexApplication application) {
diff --git a/tools/test.py b/tools/test.py
index cd9b70f..f1543f1 100755
--- a/tools/test.py
+++ b/tools/test.py
@@ -103,6 +103,9 @@
if 'BUILDBOT_BUILDERNAME' in os.environ:
gradle.RunGradle(['clean'])
+ # Build R8lib with dependencies for bootstrapping tests before adding test sources
+ gradle.RunGradle(['r8libwithdeps'])
+
gradle_args = ['--stacktrace']
# Set all necessary Gradle properties and options first.
if options.verbose:
@@ -156,8 +159,6 @@
gradle_args.append('-PHEAD_sha1=' + utils.get_HEAD_sha1())
# Add Gradle tasks
gradle_args.append('cleanTest')
- # Build R8lib with dependencies for bootstrapping tests.
- gradle_args.append('r8libWithDeps')
gradle_args.append('test')
# Test filtering. Must always follow the 'test' task.
for testFilter in args:
