Merge "Rewrite ClassStaticizerTest with new test builder."
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index 00df99e..905e949 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -176,6 +176,9 @@
AppInfo appInfo = new AppInfo(app);
app = optimize(app, appInfo, options, timing, executor);
+ // Close any internal archive providers now the application is fully processed.
+ inputApp.closeInternalArchiveProviders();
+
// If a method filter is present don't produce output since the application is likely partial.
if (options.hasMethodsFilter()) {
System.out.println("Finished compilation with method filter: ");
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 328d524..3ed662f 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -253,6 +253,9 @@
DexApplication application =
new ApplicationReader(inputApp, options, timing).read(executorService).toDirect();
+ // Now that the dex-application is fully loaded, close any internal archive providers.
+ inputApp.closeInternalArchiveProviders();
+
AppView<AppInfoWithSubtyping> appView =
new AppView<>(new AppInfoWithSubtyping(application), GraphLense.getIdentityLense());
RootSet rootSet;
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 a217bc9..ec9b186 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -668,6 +668,7 @@
new ForwardMethodSourceCode(
accessFlags.isStatic() ? null : holder.type,
newMethod,
+ newMethod,
accessFlags.isStatic() ? null : method.holder,
method,
type,
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index d68cb7d..27e91ef 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -5,6 +5,7 @@
import com.android.tools.r8.ir.code.Invoke.Type;
import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import it.unimi.dsi.fastutil.ints.IntArrayList;
@@ -84,13 +85,15 @@
public static class Builder {
- protected Builder() {
- }
+ protected Builder() {}
protected final Map<DexType, DexType> typeMap = new IdentityHashMap<>();
protected final Map<DexMethod, DexMethod> methodMap = new IdentityHashMap<>();
protected final Map<DexField, DexField> fieldMap = new IdentityHashMap<>();
+ private final BiMap<DexField, DexField> originalFieldSignatures = HashBiMap.create();
+ private final BiMap<DexMethod, DexMethod> originalMethodSignatures = HashBiMap.create();
+
public void map(DexType from, DexType to) {
typeMap.put(from, to);
}
@@ -103,6 +106,16 @@
fieldMap.put(from, to);
}
+ public void move(DexMethod from, DexMethod to) {
+ map(from, to);
+ originalMethodSignatures.put(to, from);
+ }
+
+ public void move(DexField from, DexField to) {
+ fieldMap.put(from, to);
+ originalFieldSignatures.put(to, from);
+ }
+
public GraphLense build(DexItemFactory dexItemFactory) {
return build(dexItemFactory, new IdentityGraphLense());
}
@@ -112,7 +125,13 @@
return previousLense;
}
return new NestedGraphLense(
- typeMap, methodMap, fieldMap, null, null, previousLense, dexItemFactory);
+ typeMap,
+ methodMap,
+ fieldMap,
+ originalFieldSignatures,
+ originalMethodSignatures,
+ previousLense,
+ dexItemFactory);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
index 0ca4f16..d2a233b 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/ClassProcessor.java
@@ -120,6 +120,7 @@
new ForwardMethodSourceCode(
clazz.type,
newMethod,
+ newMethod,
null /* static method */,
rewriter.defaultAsMethodOfCompanionClass(method),
Invoke.Type.STATIC,
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformer.java b/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformer.java
index f963581..955abf0 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformer.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/CovariantReturnTypeAnnotationTransformer.java
@@ -170,6 +170,7 @@
new ForwardMethodSourceCode(
clazz.type,
newMethod,
+ newMethod,
method.method.holder,
method.method,
Invoke.Type.VIRTUAL,
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
index f989cc9..6ba9b6a 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
@@ -247,7 +247,13 @@
new SynthesizedCode(
callerPosition ->
new ForwardMethodSourceCode(
- null, newMethod, null, origMethod, Type.STATIC, callerPosition)));
+ null,
+ newMethod,
+ newMethod,
+ null,
+ origMethod,
+ Type.STATIC,
+ callerPosition)));
newEncodedMethod.getMutableOptimizationInfo().markNeverInline();
dispatchMethods.add(newEncodedMethod);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index c1cf5b5..bad5456 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -97,6 +97,7 @@
if (target.getOptimizationInfo().forceInline()
|| (inliner.appView.appInfo().hasLiveness()
&& inliner.appView.withLiveness().appInfo().forceInline.contains(target.method))) {
+ assert !appView.appInfo().neverInline.contains(target.method);
return Reason.FORCE;
}
if (inliner.appView.appInfo().hasLiveness()
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index d83c550..eb42723 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.ir.optimize.classinliner;
-import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
@@ -190,7 +189,7 @@
}
}
- private boolean isClassEligible(AppInfo appInfo, DexClass clazz) {
+ private boolean isClassEligible(AppInfoWithLiveness appInfo, DexClass clazz) {
Boolean eligible = knownClasses.get(clazz);
if (eligible == null) {
Boolean computed = computeClassEligible(appInfo, clazz);
@@ -205,9 +204,12 @@
// - is not an abstract class or interface
// - does not declare finalizer
// - does not trigger any static initializers except for its own
- private boolean computeClassEligible(AppInfo appInfo, DexClass clazz) {
- if (clazz == null || clazz.isLibraryClass() ||
- clazz.accessFlags.isAbstract() || clazz.accessFlags.isInterface()) {
+ private boolean computeClassEligible(AppInfoWithLiveness appInfo, DexClass clazz) {
+ if (clazz == null
+ || clazz.isLibraryClass()
+ || clazz.accessFlags.isAbstract()
+ || clazz.accessFlags.isInterface()
+ || appInfo.neverClassInline.contains(clazz.type)) {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java b/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
index b8e79cd..37341fb 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodSourceCode.java
@@ -27,22 +27,32 @@
public ForwardMethodSourceCode(
DexType receiver,
DexMethod method,
+ DexMethod originalMethod,
DexType targetReceiver,
DexMethod target,
Type invokeType,
Position callerPosition) {
- this(receiver, method, targetReceiver, target, invokeType, callerPosition, false);
+ this(
+ receiver,
+ method,
+ originalMethod,
+ targetReceiver,
+ target,
+ invokeType,
+ callerPosition,
+ false);
}
public ForwardMethodSourceCode(
DexType receiver,
DexMethod method,
+ DexMethod originalMethod,
DexType targetReceiver,
DexMethod target,
Type invokeType,
Position callerPosition,
boolean castResult) {
- super(receiver, method, callerPosition);
+ super(receiver, method, callerPosition, originalMethod);
assert (targetReceiver == null) == (invokeType == Invoke.Type.STATIC);
this.target = target;
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java b/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
index fd04882..88bbc6c 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
@@ -51,6 +51,11 @@
private final Position position;
protected SyntheticSourceCode(DexType receiver, DexMethod method, Position callerPosition) {
+ this(receiver, method, callerPosition, method);
+ }
+
+ protected SyntheticSourceCode(
+ DexType receiver, DexMethod method, Position callerPosition, DexMethod originalMethod) {
assert method != null;
this.receiver = receiver;
this.method = method;
@@ -67,7 +72,7 @@
this.paramRegisters[i] = nextRegister(ValueType.fromDexType(params[i]));
}
- position = Position.synthetic(0, method, callerPosition);
+ position = Position.synthetic(0, originalMethod, callerPosition);
}
protected final void add(Consumer<IRBuilder> constructor) {
diff --git a/src/main/java/com/android/tools/r8/shaking/ClassInlineRule.java b/src/main/java/com/android/tools/r8/shaking/ClassInlineRule.java
new file mode 100644
index 0000000..e7dc76c
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/ClassInlineRule.java
@@ -0,0 +1,105 @@
+// 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 ClassInlineRule extends ProguardConfigurationRule {
+
+ public enum Type {
+ NEVER
+ }
+
+ public static class Builder extends ProguardConfigurationRule.Builder<ClassInlineRule, Builder> {
+
+ private Builder() {
+ super();
+ }
+
+ Type type;
+
+ @Override
+ public Builder self() {
+ return this;
+ }
+
+ public Builder setType(Type type) {
+ this.type = type;
+ return this;
+ }
+
+ @Override
+ public ClassInlineRule build() {
+ return new ClassInlineRule(
+ origin,
+ getPosition(),
+ source,
+ classAnnotation,
+ classAccessFlags,
+ negatedClassAccessFlags,
+ classTypeNegated,
+ classType,
+ classNames,
+ inheritanceAnnotation,
+ inheritanceClassName,
+ inheritanceIsExtends,
+ memberRules,
+ type);
+ }
+ }
+
+ private final Type type;
+
+ protected ClassInlineRule(
+ 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 "neverclassinline";
+ }
+ throw new Unreachable("Unknown class inline 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 06df4a9..31771a6 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -44,6 +44,7 @@
import com.android.tools.r8.logging.Log;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.RootSetBuilder.ConsequentRootSet;
+import com.android.tools.r8.shaking.RootSetBuilder.IfRuleEvaluator;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.StringDiagnostic;
@@ -1349,9 +1350,12 @@
if (numOfLiveItemsAfterProcessing > numOfLiveItems) {
RootSetBuilder consequentSetBuilder =
new RootSetBuilder(appView, rootSet.ifRules, options);
- ConsequentRootSet consequentRootSet = consequentSetBuilder.runForIfRules(
- executorService, liveTypes, liveMethods.getItems(), liveFields.getItems());
+ IfRuleEvaluator ifRuleEvaluator =
+ consequentSetBuilder.getIfRuleEvaluator(
+ liveMethods.getItems(), liveFields.getItems(), executorService);
+ ConsequentRootSet consequentRootSet = ifRuleEvaluator.run(liveTypes);
enqueueRootItems(consequentRootSet.noShrinking);
+ rootSet.neverInline.addAll(consequentRootSet.neverInline);
rootSet.noOptimization.addAll(consequentRootSet.noOptimization);
rootSet.noObfuscation.addAll(consequentRootSet.noObfuscation);
rootSet.addDependentItems(consequentRootSet.dependentNoShrinking);
@@ -1839,6 +1843,10 @@
*/
public final Set<DexMethod> neverInline;
/**
+ * All types that *must* never be inlined due to a configuration directive (testing only).
+ */
+ public final Set<DexType> neverClassInline;
+ /**
* All types that *must* never be merged due to a configuration directive (testing only).
*/
public final Set<DexType> neverMerge;
@@ -1903,6 +1911,7 @@
this.alwaysInline = enqueuer.rootSet.alwaysInline;
this.forceInline = enqueuer.rootSet.forceInline;
this.neverInline = enqueuer.rootSet.neverInline;
+ this.neverClassInline = enqueuer.rootSet.neverClassInline;
this.neverMerge = enqueuer.rootSet.neverMerge;
this.identifierNameStrings = joinIdentifierNameStrings(
enqueuer.rootSet.identifierNameStrings, enqueuer.identifierNameStrings);
@@ -1947,6 +1956,7 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = previous.forceInline;
this.neverInline = previous.neverInline;
+ this.neverClassInline = previous.neverClassInline;
this.neverMerge = previous.neverMerge;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = mergeSets(previous.prunedTypes, removedClasses);
@@ -2005,6 +2015,7 @@
assert lense.assertDefinitionNotModified(
previous.neverMerge.stream().map(this::definitionFor).filter(Objects::nonNull)
.collect(Collectors.toList()));
+ this.neverClassInline = rewriteItems(previous.neverClassInline, lense::lookupType);
this.neverMerge = previous.neverMerge;
this.identifierNameStrings =
lense.rewriteReferencesConservatively(previous.identifierNameStrings);
@@ -2050,6 +2061,7 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = previous.forceInline;
this.neverInline = previous.neverInline;
+ this.neverClassInline = previous.neverClassInline;
this.neverMerge = previous.neverMerge;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = previous.prunedTypes;
diff --git a/src/main/java/com/android/tools/r8/shaking/InlineRule.java b/src/main/java/com/android/tools/r8/shaking/InlineRule.java
index f263bd6..3f255f2 100644
--- a/src/main/java/com/android/tools/r8/shaking/InlineRule.java
+++ b/src/main/java/com/android/tools/r8/shaking/InlineRule.java
@@ -42,7 +42,7 @@
private final Type type;
- private InlineRule(
+ protected InlineRule(
Origin origin,
Position position,
String source,
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardClassSpecification.java b/src/main/java/com/android/tools/r8/shaking/ProguardClassSpecification.java
index 1d970a3..1bbebe0 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardClassSpecification.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardClassSpecification.java
@@ -339,7 +339,7 @@
builder.append(' ');
classNames.writeTo(builder);
if (hasInheritanceClassName()) {
- builder.append(inheritanceIsExtends ? "extends" : "implements");
+ builder.append(' ').append(inheritanceIsExtends ? "extends" : "implements");
StringUtils.appendNonEmpty(builder, "@", inheritanceAnnotation, null);
builder.append(' ');
builder.append(inheritanceClassName);
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 ea20704..2ab9308 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -16,7 +16,6 @@
import com.android.tools.r8.position.Position;
import com.android.tools.r8.position.TextPosition;
import com.android.tools.r8.position.TextRange;
-import com.android.tools.r8.shaking.InlineRule.Type;
import com.android.tools.r8.shaking.ProguardConfiguration.Builder;
import com.android.tools.r8.shaking.ProguardTypeMatcher.ClassOrType;
import com.android.tools.r8.shaking.ProguardTypeMatcher.MatchSpecificType;
@@ -353,16 +352,19 @@
} else if (acceptString("packageobfuscationdictionary")) {
configurationBuilder.setPackageObfuscationDictionary(parseFileName(false));
} else if (acceptString("alwaysinline")) {
- InlineRule rule = parseInlineRule(Type.ALWAYS, optionStart);
+ InlineRule rule = parseInlineRule(InlineRule.Type.ALWAYS, optionStart);
configurationBuilder.addRule(rule);
} else if (allowTestOptions && acceptString("forceinline")) {
- InlineRule rule = parseInlineRule(Type.FORCE, optionStart);
+ InlineRule rule = parseInlineRule(InlineRule.Type.FORCE, optionStart);
configurationBuilder.addRule(rule);
// Insert a matching -checkdiscard rule to ensure force inlining happens.
ProguardCheckDiscardRule ruled = rule.asProguardCheckDiscardRule();
configurationBuilder.addRule(ruled);
} else if (allowTestOptions && acceptString("neverinline")) {
- InlineRule rule = parseInlineRule(Type.NEVER, optionStart);
+ InlineRule rule = parseInlineRule(InlineRule.Type.NEVER, optionStart);
+ configurationBuilder.addRule(rule);
+ } else if (allowTestOptions && acceptString("neverclassinline")) {
+ ClassInlineRule rule = parseClassInlineRule(ClassInlineRule.Type.NEVER, optionStart);
configurationBuilder.addRule(rule);
} else if (allowTestOptions && acceptString("nevermerge")) {
ClassMergingRule rule = parseClassMergingRule(ClassMergingRule.Type.NEVER, optionStart);
@@ -597,6 +599,17 @@
return keepRuleBuilder.build();
}
+ private ClassInlineRule parseClassInlineRule(ClassInlineRule.Type type, Position start)
+ throws ProguardRuleParserException {
+ ClassInlineRule.Builder keepRuleBuilder =
+ ClassInlineRule.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 ClassMergingRule parseClassMergingRule(ClassMergingRule.Type type, Position start)
throws ProguardRuleParserException {
ClassMergingRule.Builder keepRuleBuilder =
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java b/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
index 6d6d943..3011951 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
@@ -87,6 +87,52 @@
subsequentRule.materialize());
}
+ /**
+ * Consider the following rule, which requests that class Y should be kept if the method X.m() is
+ * in the final output.
+ *
+ * <pre>
+ * -if class X {
+ * public void m();
+ * }
+ * -keep class Y
+ * </pre>
+ *
+ * When the {@link Enqueuer} finds that the method X.m() is reachable, it applies the subsequent
+ * keep rule of the -if rule. Thus, Y will be marked as pinned, which guarantees, for example,
+ * that it will not be merged into another class by the vertical class merger.
+ *
+ * <p>However, when the {@link Enqueuer} runs for the second time, it is important that X.m() has
+ * not been inlined into another method Z.z(), because that would mean that Z.z() now relies on
+ * the presence of Y, meanwhile Y will not be kept because X.m() is no longer present.
+ *
+ * <p>Therefore, each time the subsequent rule of an -if rule is applied, we also apply a
+ * -neverinline rule for the condition of the -if rule.
+ */
+ protected InlineRule neverInlineRuleForCondition() {
+ if (getMemberRules() == null || getMemberRules().isEmpty()) {
+ return null;
+ }
+ return new InlineRule(
+ Origin.unknown(),
+ Position.UNKNOWN,
+ null,
+ getClassAnnotation() == null ? null : getClassAnnotation().materialize(),
+ getClassAccessFlags(),
+ getNegatedClassAccessFlags(),
+ getClassTypeNegated(),
+ getClassType(),
+ getClassNames().materialize(),
+ getInheritanceAnnotation() == null ? null : getInheritanceAnnotation().materialize(),
+ getInheritanceClassName() == null ? null : getInheritanceClassName().materialize(),
+ getInheritanceIsExtends(),
+ getMemberRules().stream()
+ .filter(rule -> rule.getRuleType().includesMethods())
+ .map(ProguardMemberRule::materialize)
+ .collect(Collectors.toList()),
+ InlineRule.Type.NEVER);
+ }
+
@Override
public boolean equals(Object o) {
if (!(o instanceof ProguardIfRule)) {
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 df72e96..c59081e 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> neverClassInline = Sets.newIdentityHashSet();
private final Set<DexType> neverMerge = Sets.newIdentityHashSet();
private final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking =
new IdentityHashMap<>();
@@ -180,6 +181,10 @@
}
} else if (rule instanceof InlineRule) {
markMatchingMethods(clazz, memberKeepRules, rule, null);
+ } else if (rule instanceof ClassInlineRule) {
+ if (allRulesSatisfied(memberKeepRules, clazz)) {
+ markClass(clazz, rule);
+ }
} else if (rule instanceof ProguardAssumeValuesRule) {
markMatchingVisibleMethods(clazz, memberKeepRules, rule, null);
markMatchingVisibleFields(clazz, memberKeepRules, rule, null);
@@ -250,6 +255,7 @@
alwaysInline,
forceInline,
neverInline,
+ neverClassInline,
neverMerge,
noSideEffects,
assumedValues,
@@ -258,55 +264,15 @@
ifRules);
}
- ConsequentRootSet runForIfRules(
- ExecutorService executorService,
- Set<DexType> liveTypes,
+ IfRuleEvaluator getIfRuleEvaluator(
Set<DexEncodedMethod> liveMethods,
- Set<DexEncodedField> liveFields) throws ExecutionException {
- application.timing.begin("Find consequent items for -if rules...");
- try {
- if (rules != null) {
- IfRuleEvaluator evaluator =
- new IfRuleEvaluator(liveTypes, liveMethods, liveFields, executorService);
- for (ProguardConfigurationRule rule : rules) {
- assert rule instanceof ProguardIfRule;
- ProguardIfRule ifRule = (ProguardIfRule) rule;
- // Depending on which types that trigger the -if rule, the application of the subsequent
- // -keep rule may vary (due to back references). So, we need to try all pairs of -if rule
- // and live types.
- for (DexType type : liveTypes) {
- DexClass clazz = appView.appInfo().definitionFor(type);
- if (clazz == null) {
- continue;
- }
-
- // Check if the class matches the if-rule.
- evaluator.evaluateIfRule(ifRule, clazz, clazz);
-
- // Check if one of the types that have been merged into `clazz` satisfies the if-rule.
- if (options.enableVerticalClassMerging && appView.verticallyMergedClasses() != null) {
- for (DexType sourceType : appView.verticallyMergedClasses().getSourcesFor(type)) {
- // Note that, although `sourceType` has been merged into `type`, the dex class for
- // `sourceType` is still available until the second round of tree shaking. This way
- // we can still retrieve the access flags of `sourceType`.
- DexClass sourceClass = appView.appInfo().definitionFor(sourceType);
- assert sourceClass != null;
- evaluator.evaluateIfRule(ifRule, sourceClass, clazz);
- }
- }
- }
- }
- ThreadUtils.awaitFutures(evaluator.futures);
- }
- } finally {
- application.timing.end();
- }
- return new ConsequentRootSet(noShrinking, noOptimization, noObfuscation, dependentNoShrinking);
+ Set<DexEncodedField> liveFields,
+ ExecutorService executorService) {
+ return new IfRuleEvaluator(liveMethods, liveFields, executorService);
}
- private class IfRuleEvaluator {
+ class IfRuleEvaluator {
- private final Set<DexType> liveTypes;
private final Set<DexEncodedMethod> liveMethods;
private final Set<DexEncodedField> liveFields;
private final ExecutorService executorService;
@@ -314,16 +280,56 @@
private final List<Future<?>> futures = new ArrayList<>();
public IfRuleEvaluator(
- Set<DexType> liveTypes,
Set<DexEncodedMethod> liveMethods,
Set<DexEncodedField> liveFields,
ExecutorService executorService) {
- this.liveTypes = liveTypes;
this.liveMethods = liveMethods;
this.liveFields = liveFields;
this.executorService = executorService;
}
+ public ConsequentRootSet run(Set<DexType> liveTypes) throws ExecutionException {
+ application.timing.begin("Find consequent items for -if rules...");
+ try {
+ if (rules != null) {
+ for (ProguardConfigurationRule rule : rules) {
+ assert rule instanceof ProguardIfRule;
+ ProguardIfRule ifRule = (ProguardIfRule) rule;
+ // Depending on which types that trigger the -if rule, the application of the subsequent
+ // -keep rule may vary (due to back references). So, we need to try all pairs of -if
+ // rule and live types.
+ for (DexType type : liveTypes) {
+ DexClass clazz = appView.appInfo().definitionFor(type);
+ if (clazz == null) {
+ continue;
+ }
+
+ // Check if the class matches the if-rule.
+ evaluateIfRule(ifRule, clazz, clazz);
+
+ // Check if one of the types that have been merged into `clazz` satisfies the if-rule.
+ if (options.enableVerticalClassMerging && appView.verticallyMergedClasses() != null) {
+ for (DexType sourceType : appView.verticallyMergedClasses().getSourcesFor(type)) {
+ // Note that, although `sourceType` has been merged into `type`, the dex class for
+ // `sourceType` is still available until the second round of tree shaking. This
+ // way
+ // we can still retrieve the access flags of `sourceType`.
+ DexClass sourceClass = appView.appInfo().definitionFor(sourceType);
+ assert sourceClass != null;
+ evaluateIfRule(ifRule, sourceClass, clazz);
+ }
+ }
+ }
+ }
+ ThreadUtils.awaitFutures(futures);
+ }
+ } finally {
+ application.timing.end();
+ }
+ return new ConsequentRootSet(
+ neverInline, noShrinking, noOptimization, noObfuscation, dependentNoShrinking);
+ }
+
/**
* Determines if `sourceClass` satisfies the given if-rule. If `sourceClass` has not been merged
* into another class, then `targetClass` is the same as `sourceClass`. Otherwise, `targetClass`
@@ -406,6 +412,16 @@
private void materializeIfRule(ProguardIfRule rule) {
ProguardIfRule materializedRule = rule.materialize();
+
+ // If the condition of the -if rule has any members, then we need to keep these members to
+ // ensure that the subsequent rule will be applied again in the second round of tree
+ // shaking.
+ InlineRule neverInlineRuleForCondition = materializedRule.neverInlineRuleForCondition();
+ if (neverInlineRuleForCondition != null) {
+ runPerRule(executorService, futures, neverInlineRuleForCondition, materializedRule);
+ }
+
+ // Keep whatever is required by the -if rule.
runPerRule(executorService, futures, materializedRule.subsequentRule, materializedRule);
}
}
@@ -852,20 +868,26 @@
} else if (context instanceof ProguardCheckDiscardRule) {
checkDiscarded.add(item);
} else if (context instanceof InlineRule) {
- switch (((InlineRule) context).getType()) {
- case ALWAYS:
- if (item.isDexEncodedMethod()) {
+ if (item.isDexEncodedMethod()) {
+ switch (((InlineRule) context).getType()) {
+ case ALWAYS:
alwaysInline.add(item.asDexEncodedMethod().method);
- }
- break;
- case FORCE:
- if (item.isDexEncodedMethod()) {
+ break;
+ case FORCE:
forceInline.add(item.asDexEncodedMethod().method);
- }
- break;
- case NEVER:
- if (item.isDexEncodedMethod()) {
+ break;
+ case NEVER:
neverInline.add(item.asDexEncodedMethod().method);
+ break;
+ default:
+ throw new Unreachable();
+ }
+ }
+ } else if (context instanceof ClassInlineRule) {
+ switch (((ClassInlineRule) context).getType()) {
+ case NEVER:
+ if (item.isDexClass()) {
+ neverClassInline.add(item.asDexClass().type);
}
break;
default:
@@ -901,6 +923,7 @@
public final Set<DexMethod> alwaysInline;
public final Set<DexMethod> forceInline;
public final Set<DexMethod> neverInline;
+ public final Set<DexType> neverClassInline;
public final Set<DexType> neverMerge;
public final Map<DexDefinition, ProguardMemberRule> noSideEffects;
public final Map<DexDefinition, ProguardMemberRule> assumedValues;
@@ -918,6 +941,7 @@
Set<DexMethod> alwaysInline,
Set<DexMethod> forceInline,
Set<DexMethod> neverInline,
+ Set<DexType> neverClassInline,
Set<DexType> neverMerge,
Map<DexDefinition, ProguardMemberRule> noSideEffects,
Map<DexDefinition, ProguardMemberRule> assumedValues,
@@ -932,7 +956,8 @@
this.checkDiscarded = Collections.unmodifiableSet(checkDiscarded);
this.alwaysInline = Collections.unmodifiableSet(alwaysInline);
this.forceInline = Collections.unmodifiableSet(forceInline);
- this.neverInline = Collections.unmodifiableSet(neverInline);
+ this.neverInline = neverInline;
+ this.neverClassInline = Collections.unmodifiableSet(neverClassInline);
this.neverMerge = Collections.unmodifiableSet(neverMerge);
this.noSideEffects = Collections.unmodifiableMap(noSideEffects);
this.assumedValues = Collections.unmodifiableMap(assumedValues);
@@ -984,16 +1009,19 @@
// A partial RootSet that becomes live due to the enabled -if rule.
static class ConsequentRootSet {
+ final Set<DexMethod> neverInline;
final Map<DexDefinition, ProguardKeepRule> noShrinking;
final Set<DexDefinition> noOptimization;
final Set<DexDefinition> noObfuscation;
final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking;
private ConsequentRootSet(
+ Set<DexMethod> neverInline,
Map<DexDefinition, ProguardKeepRule> noShrinking,
Set<DexDefinition> noOptimization,
Set<DexDefinition> noObfuscation,
Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking) {
+ this.neverInline = Collections.unmodifiableSet(neverInline);
this.noShrinking = Collections.unmodifiableMap(noShrinking);
this.noOptimization = Collections.unmodifiableSet(noOptimization);
this.noObfuscation = Collections.unmodifiableSet(noObfuscation);
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 b9b9a99..4dfee8e 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -60,6 +60,7 @@
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
@@ -613,9 +614,28 @@
assert result.assertDefinitionNotModified(appInfo.noSideEffects.keySet());
// TODO(christofferqa): Enable this assert.
// assert result.assertNotModified(appInfo.pinnedItems);
+ assert verifyGraphLense(graphLense);
return result;
}
+ private boolean verifyGraphLense(GraphLense graphLense) {
+ for (DexProgramClass clazz : appInfo.classes()) {
+ for (DexEncodedMethod encodedMethod : clazz.methods()) {
+ DexMethod method = encodedMethod.method;
+ DexMethod originalMethod = graphLense.getOriginalMethodSignature(method);
+
+ // Must be able to map back.
+ assert method == graphLense.getRenamedMethodSignature(originalMethod);
+
+ // Verify that all types are up-to-date. After vertical class merging, there should be no
+ // more references to types that have been merged into another type.
+ assert !mergedClasses.containsKey(method.proto.returnType);
+ assert Arrays.stream(method.proto.parameters.values).noneMatch(mergedClasses::containsKey);
+ }
+ }
+ return true;
+ }
+
private GraphLense mergeClasses(GraphLense graphLense) {
// Visit the program classes in a top-down order according to the class hierarchy.
TopDownClassHierarchyTraversal.visit(appView, mergeCandidates, this::mergeClassIfPossible);
@@ -900,6 +920,7 @@
// it turns out that the method is never used, it will be removed by the final round
// of tree shaking.
shadowedBy = buildBridgeMethod(virtualMethod, resultingDirectMethod);
+ deferredRenamings.recordCreationOfBridgeMethod(virtualMethod.method, shadowedBy.method);
add(virtualMethods, shadowedBy, MethodSignatureEquivalence.get());
}
@@ -1109,6 +1130,7 @@
SynthesizedBridgeCode code =
new SynthesizedBridgeCode(
newMethod,
+ graphLense.getOriginalMethodSignature(method.method),
invocationTarget.method,
invocationTarget.isPrivateMethod() ? DIRECT : STATIC);
@@ -1393,7 +1415,7 @@
DexMethod newMethod = application.dexItemFactory.createMethod(newHolder, newProto,
method.name);
if (newMethod != encodedMethod.method) {
- lense.map(encodedMethod.method, newMethod);
+ lense.move(encodedMethod.method, newMethod);
methods[i] = encodedMethod.toTypeSubstitutedMethod(newMethod);
}
}
@@ -1411,7 +1433,7 @@
DexType newHolder = fixupType(field.clazz);
DexField newField = application.dexItemFactory.createField(newHolder, newType, field.name);
if (newField != encodedField.field) {
- lense.map(encodedField.field, newField);
+ lense.move(encodedField.field, newField);
fields[i] = encodedField.toTypeSubstitutedField(newField);
}
}
@@ -1801,11 +1823,14 @@
protected static class SynthesizedBridgeCode extends AbstractSynthesizedCode {
private DexMethod method;
+ private DexMethod originalMethod;
private DexMethod invocationTarget;
private Type type;
- public SynthesizedBridgeCode(DexMethod method, DexMethod invocationTarget, Type type) {
+ public SynthesizedBridgeCode(
+ DexMethod method, DexMethod originalMethod, DexMethod invocationTarget, Type type) {
this.method = method;
+ this.originalMethod = originalMethod;
this.invocationTarget = invocationTarget;
this.type = type;
}
@@ -1832,6 +1857,7 @@
new ForwardMethodSourceCode(
method.holder,
method,
+ originalMethod,
type == DIRECT ? method.holder : null,
invocationTarget,
type,
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
index 43c5e92..6b66465 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLense.java
@@ -17,10 +17,10 @@
import com.android.tools.r8.shaking.VerticalClassMerger.SynthesizedBridgeCode;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
-import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import java.util.HashMap;
+import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -53,9 +53,10 @@
public class VerticalClassMergerGraphLense extends NestedGraphLense {
private final AppInfo appInfo;
- private final Set<DexMethod> mergedMethods;
private final Map<DexType, Map<DexMethod, GraphLenseLookupResult>>
contextualVirtualToDirectMethodMaps;
+ private final Set<DexMethod> mergedMethods;
+ private final Map<DexMethod, DexMethod> originalMethodSignaturesForBridges;
public VerticalClassMergerGraphLense(
AppInfo appInfo,
@@ -65,6 +66,7 @@
Map<DexType, Map<DexMethod, GraphLenseLookupResult>> contextualVirtualToDirectMethodMaps,
BiMap<DexField, DexField> originalFieldSignatures,
BiMap<DexMethod, DexMethod> originalMethodSignatures,
+ Map<DexMethod, DexMethod> originalMethodSignaturesForBridges,
GraphLense previousLense) {
super(
ImmutableMap.of(),
@@ -75,8 +77,15 @@
previousLense,
appInfo.dexItemFactory);
this.appInfo = appInfo;
- this.mergedMethods = mergedMethods;
this.contextualVirtualToDirectMethodMaps = contextualVirtualToDirectMethodMaps;
+ this.mergedMethods = mergedMethods;
+ this.originalMethodSignaturesForBridges = originalMethodSignaturesForBridges;
+ }
+
+ @Override
+ public DexMethod getOriginalMethodSignature(DexMethod method) {
+ return super.getOriginalMethodSignature(
+ originalMethodSignaturesForBridges.getOrDefault(method, method));
}
@Override
@@ -154,7 +163,9 @@
private final Map<DexType, Map<DexMethod, GraphLenseLookupResult>>
contextualVirtualToDirectMethodMaps = new HashMap<>();
- private final Map<DexMethod, DexMethod> originalMethodSignatures = HashBiMap.create();
+ private final BiMap<DexMethod, DexMethod> originalMethodSignatures = HashBiMap.create();
+ private final Map<DexMethod, DexMethod> originalMethodSignaturesForBridges =
+ new IdentityHashMap<>();
public GraphLense build(
GraphLense previousLense,
@@ -184,10 +195,9 @@
getMergedMethodSignaturesAfterClassMerging(
mergedMethodsBuilder.build(), mergedClasses, appInfo.dexItemFactory, cache),
contextualVirtualToDirectMethodMaps,
- getOriginalFieldSignaturesAfterClassMerging(
- originalFieldSignatures, mergedClasses, appInfo.dexItemFactory),
- getOriginalMethodSignaturesAfterClassMerging(
- originalMethodSignatures, mergedClasses, appInfo.dexItemFactory, cache),
+ originalFieldSignatures,
+ originalMethodSignatures,
+ originalMethodSignaturesForBridges,
previousLense);
}
@@ -207,44 +217,6 @@
return result.build();
}
- private static BiMap<DexField, DexField> getOriginalFieldSignaturesAfterClassMerging(
- Map<DexField, DexField> originalFieldSignatures,
- Map<DexType, DexType> mergedClasses,
- DexItemFactory dexItemFactory) {
- ImmutableBiMap.Builder<DexField, DexField> result = ImmutableBiMap.builder();
- for (Map.Entry<DexField, DexField> entry : originalFieldSignatures.entrySet()) {
- result.put(
- getFieldSignatureAfterClassMerging(entry.getKey(), mergedClasses, dexItemFactory),
- entry.getValue());
- }
- return result.build();
- }
-
- private static BiMap<DexMethod, DexMethod> getOriginalMethodSignaturesAfterClassMerging(
- Map<DexMethod, DexMethod> originalMethodSignatures,
- Map<DexType, DexType> mergedClasses,
- DexItemFactory dexItemFactory,
- Map<DexProto, DexProto> cache) {
- ImmutableBiMap.Builder<DexMethod, DexMethod> result = ImmutableBiMap.builder();
- for (Map.Entry<DexMethod, DexMethod> entry : originalMethodSignatures.entrySet()) {
- result.put(
- getMethodSignatureAfterClassMerging(
- entry.getKey(), mergedClasses, dexItemFactory, cache),
- entry.getValue());
- }
- return result.build();
- }
-
- private static DexField getFieldSignatureAfterClassMerging(
- DexField signature, Map<DexType, DexType> mergedClasses, DexItemFactory dexItemFactory) {
- DexType newClass = mergedClasses.getOrDefault(signature.clazz, signature.clazz);
- DexType newType = mergedClasses.getOrDefault(signature.type, signature.type);
- if (signature.clazz.equals(newClass) && signature.type.equals(newType)) {
- return signature;
- }
- return dexItemFactory.createField(newClass, newType, signature.name);
- }
-
private static DexMethod getMethodSignatureAfterClassMerging(
DexMethod signature,
Map<DexType, DexType> mergedClasses,
@@ -285,6 +257,10 @@
originalMethodSignatures.put(to, from);
}
+ public void recordCreationOfBridgeMethod(DexMethod from, DexMethod to) {
+ originalMethodSignaturesForBridges.put(to, from);
+ }
+
public void mapVirtualMethodToDirectInType(
DexMethod from, GraphLenseLookupResult to, DexType type) {
Map<DexMethod, GraphLenseLookupResult> virtualToDirectMethodMap =
@@ -297,6 +273,7 @@
methodMap.putAll(builder.methodMap);
mergedMethodsBuilder.addAll(builder.mergedMethodsBuilder.build());
originalMethodSignatures.putAll(builder.originalMethodSignatures);
+ originalMethodSignaturesForBridges.putAll(builder.originalMethodSignaturesForBridges);
for (DexType context : builder.contextualVirtualToDirectMethodMaps.keySet()) {
Map<DexMethod, GraphLenseLookupResult> current =
contextualVirtualToDirectMethodMaps.get(context);
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidApp.java b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
index ed12177..49573db 100644
--- a/src/main/java/com/android/tools/r8/utils/AndroidApp.java
+++ b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
@@ -7,7 +7,6 @@
import static com.android.tools.r8.utils.FileUtils.isClassFile;
import static com.android.tools.r8.utils.FileUtils.isDexFile;
-import com.android.tools.r8.ArchiveClassFileProvider;
import com.android.tools.r8.ClassFileConsumer;
import com.android.tools.r8.ClassFileResourceProvider;
import com.android.tools.r8.DataDirectoryResource;
@@ -56,10 +55,19 @@
private final ImmutableList<ClassFileResourceProvider> classpathResourceProviders;
private final ImmutableList<ClassFileResourceProvider> libraryResourceProviders;
+ // List of internally added archive providers for which we must close their resources.
+ private final ImmutableList<InternalArchiveClassFileProvider> archiveProvidersToClose;
+
private final StringResource proguardMapOutputData;
private final List<StringResource> mainDexListResources;
private final List<String> mainDexClasses;
+ public void closeInternalArchiveProviders() throws IOException {
+ for (InternalArchiveClassFileProvider provider : archiveProvidersToClose) {
+ provider.close();
+ }
+ }
+
@Override
public String toString() {
StringBuilder builder = new StringBuilder();
@@ -118,6 +126,7 @@
ImmutableMap<Resource, String> programResourcesMainDescriptor,
ImmutableList<ClassFileResourceProvider> classpathResourceProviders,
ImmutableList<ClassFileResourceProvider> libraryResourceProviders,
+ ImmutableList<InternalArchiveClassFileProvider> archiveProvidersToClose,
StringResource proguardMapOutputData,
List<StringResource> mainDexListResources,
List<String> mainDexClasses) {
@@ -125,9 +134,20 @@
this.programResourcesMainDescriptor = programResourcesMainDescriptor;
this.classpathResourceProviders = classpathResourceProviders;
this.libraryResourceProviders = libraryResourceProviders;
+ this.archiveProvidersToClose = archiveProvidersToClose;
this.proguardMapOutputData = proguardMapOutputData;
this.mainDexListResources = mainDexListResources;
this.mainDexClasses = mainDexClasses;
+ assert verifyInternalProvidersInCloseSet(classpathResourceProviders, archiveProvidersToClose);
+ assert verifyInternalProvidersInCloseSet(libraryResourceProviders, archiveProvidersToClose);
+ }
+
+ private static boolean verifyInternalProvidersInCloseSet(
+ ImmutableList<ClassFileResourceProvider> providers,
+ ImmutableList<InternalArchiveClassFileProvider> providersToClose) {
+ return providers.stream()
+ .allMatch(
+ p -> !(p instanceof InternalArchiveClassFileProvider) || providersToClose.contains(p));
}
static Reporter defaultReporter() {
@@ -327,6 +347,8 @@
private final Map<ProgramResource, String> programResourcesMainDescriptor = new HashMap<>();
private final List<ClassFileResourceProvider> classpathResourceProviders = new ArrayList<>();
private final List<ClassFileResourceProvider> libraryResourceProviders = new ArrayList<>();
+ private final List<InternalArchiveClassFileProvider> archiveProvidersToClose =
+ new ArrayList<>();
private List<StringResource> mainDexListResources = new ArrayList<>();
private List<String> mainDexListClasses = new ArrayList<>();
private boolean ignoreDexInArchive = false;
@@ -347,6 +369,7 @@
programResourceProviders.addAll(app.programResourceProviders);
classpathResourceProviders.addAll(app.classpathResourceProviders);
libraryResourceProviders.addAll(app.libraryResourceProviders);
+ archiveProvidersToClose.addAll(app.archiveProvidersToClose);
mainDexListResources = app.mainDexListResources;
mainDexListClasses = app.mainDexClasses;
}
@@ -436,7 +459,9 @@
for (FilteredClassPath archive : filteredArchives) {
assert isArchive(archive.getPath());
try {
- libraryResourceProviders.add(new FilteredArchiveClassFileProvider(archive));
+ FilteredArchiveClassFileProvider provider = new FilteredArchiveClassFileProvider(archive);
+ archiveProvidersToClose.add(provider);
+ libraryResourceProviders.add(provider);
} catch (IOException e) {
reporter.error(new ExceptionDiagnostic(e, new PathOrigin(archive.getPath())));
}
@@ -625,6 +650,7 @@
ImmutableMap.copyOf(programResourcesMainDescriptor),
ImmutableList.copyOf(classpathResourceProviders),
ImmutableList.copyOf(libraryResourceProviders),
+ ImmutableList.copyOf(archiveProvidersToClose),
proguardMapOutputData,
mainDexListResources,
mainDexListClasses);
@@ -673,7 +699,9 @@
}
if (isArchive(file)) {
try {
- providerList.add(new ArchiveClassFileProvider(file));
+ InternalArchiveClassFileProvider provider = new InternalArchiveClassFileProvider(file);
+ archiveProvidersToClose.add(provider);
+ providerList.add(provider);
} catch (IOException e) {
reporter.error(new ExceptionDiagnostic(e, new PathOrigin(file)));
}
diff --git a/src/main/java/com/android/tools/r8/utils/FilteredArchiveClassFileProvider.java b/src/main/java/com/android/tools/r8/utils/FilteredArchiveClassFileProvider.java
index 0c6cc14..43cbb4d 100644
--- a/src/main/java/com/android/tools/r8/utils/FilteredArchiveClassFileProvider.java
+++ b/src/main/java/com/android/tools/r8/utils/FilteredArchiveClassFileProvider.java
@@ -3,12 +3,11 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.utils;
-import com.android.tools.r8.ArchiveClassFileProvider;
import com.android.tools.r8.shaking.FilteredClassPath;
import java.io.IOException;
// Internal filtered class-file provider.
-class FilteredArchiveClassFileProvider extends ArchiveClassFileProvider {
+class FilteredArchiveClassFileProvider extends InternalArchiveClassFileProvider {
FilteredArchiveClassFileProvider(FilteredClassPath archive) throws IOException {
super(archive.getPath(), entry -> archive.matchesFile(entry));
diff --git a/src/main/java/com/android/tools/r8/utils/InternalArchiveClassFileProvider.java b/src/main/java/com/android/tools/r8/utils/InternalArchiveClassFileProvider.java
new file mode 100644
index 0000000..cf30d7a
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/InternalArchiveClassFileProvider.java
@@ -0,0 +1,125 @@
+// 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.utils;
+
+import static com.android.tools.r8.utils.FileUtils.CLASS_EXTENSION;
+import static com.android.tools.r8.utils.FileUtils.isArchive;
+
+import com.android.tools.r8.ClassFileResourceProvider;
+import com.android.tools.r8.ProgramResource;
+import com.android.tools.r8.ProgramResource.Kind;
+import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.origin.ArchiveEntryOrigin;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.origin.PathOrigin;
+import com.google.common.io.ByteStreams;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+
+/**
+ * Internal-only provider for providing class files when clients use the addClasspathFiles and
+ * addLibraryFiles API.
+ *
+ * <p>The purpose of the internal provider is that for the API use above we know it is safe to use
+ * the zip-file descriptor throughout compilation and close at the end of reading. It must also be
+ * safe to reopen it as currently our own tests reuse AndroidApp structures.
+ */
+class InternalArchiveClassFileProvider implements ClassFileResourceProvider, AutoCloseable {
+ private final Path path;
+ private final Origin origin;
+ private final Set<String> descriptors = new HashSet<>();
+
+ private ZipFile openedZipFile = null;
+
+ /**
+ * Creates a lazy class-file program-resource provider.
+ *
+ * @param archive Zip archive to provide resources from.
+ */
+ public InternalArchiveClassFileProvider(Path archive) throws IOException {
+ this(archive, entry -> true);
+ }
+
+ /**
+ * Creates a lazy class-file program-resource provider with an include filter.
+ *
+ * @param archive Zip archive to provide resources from.
+ * @param include Predicate deciding if a given class-file entry should be provided.
+ */
+ public InternalArchiveClassFileProvider(Path archive, Predicate<String> include)
+ throws IOException {
+ assert isArchive(archive);
+ path = archive;
+ origin = new PathOrigin(archive);
+ final Enumeration<? extends ZipEntry> entries = getOpenZipFile().entries();
+ while (entries.hasMoreElements()) {
+ ZipEntry entry = entries.nextElement();
+ String name = entry.getName();
+ if (ZipUtils.isClassFile(name) && include.test(name)) {
+ descriptors.add(DescriptorUtils.guessTypeDescriptor(name));
+ }
+ }
+ }
+
+ @Override
+ public Set<String> getClassDescriptors() {
+ return Collections.unmodifiableSet(descriptors);
+ }
+
+ @Override
+ public ProgramResource getProgramResource(String descriptor) {
+ if (!descriptors.contains(descriptor)) {
+ return null;
+ }
+ try {
+ ZipEntry zipEntry = getZipEntryFromDescriptor(descriptor);
+ try (InputStream inputStream = getOpenZipFile().getInputStream(zipEntry)) {
+ return ProgramResource.fromBytes(
+ new ArchiveEntryOrigin(zipEntry.getName(), origin),
+ Kind.CF,
+ ByteStreams.toByteArray(inputStream),
+ Collections.singleton(descriptor));
+ }
+ } catch (IOException e) {
+ throw new CompilationError("Failed to read '" + descriptor, origin);
+ }
+ }
+
+ private ZipFile getOpenZipFile() throws IOException {
+ if (openedZipFile == null) {
+ try {
+ openedZipFile = new ZipFile(path.toFile(), StandardCharsets.UTF_8);
+ } catch (IOException e) {
+ if (!Files.exists(path)) {
+ throw new NoSuchFileException(path.toString());
+ } else {
+ throw e;
+ }
+ }
+ }
+ return openedZipFile;
+ }
+
+ @Override
+ public void close() throws IOException {
+ openedZipFile.close();
+ openedZipFile = null;
+ }
+
+ private ZipEntry getZipEntryFromDescriptor(String descriptor) throws IOException {
+ return getOpenZipFile()
+ .getEntry(descriptor.substring(1, descriptor.length() - 1) + CLASS_EXTENSION);
+ }
+}
diff --git a/src/test/examples/inlining/AlwaysInline.java b/src/test/examples/inlining/AlwaysInline.java
index 815d50a..22cb31e 100644
--- a/src/test/examples/inlining/AlwaysInline.java
+++ b/src/test/examples/inlining/AlwaysInline.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package inlining;
-public @interface AlwaysInline {
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
-}
+@Target({ElementType.METHOD})
+public @interface AlwaysInline {}
diff --git a/src/test/examples/inlining/NeverInline.java b/src/test/examples/inlining/NeverInline.java
index 0db5239..0c5b581 100644
--- a/src/test/examples/inlining/NeverInline.java
+++ b/src/test/examples/inlining/NeverInline.java
@@ -3,4 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package inlining;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD})
public @interface NeverInline {}
diff --git a/src/test/java/com/android/tools/r8/NeverClassInline.java b/src/test/java/com/android/tools/r8/NeverClassInline.java
new file mode 100644
index 0000000..aa7f9b5
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/NeverClassInline.java
@@ -0,0 +1,10 @@
+// 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 java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.TYPE})
+public @interface NeverClassInline {}
diff --git a/src/test/java/com/android/tools/r8/NeverMerge.java b/src/test/java/com/android/tools/r8/NeverMerge.java
index 7fc97b9..7c6922a 100644
--- a/src/test/java/com/android/tools/r8/NeverMerge.java
+++ b/src/test/java/com/android/tools/r8/NeverMerge.java
@@ -3,4 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.TYPE})
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 3a249e6..5550415 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -30,6 +30,7 @@
}
private boolean enableInliningAnnotations = false;
+ private boolean enableClassInliningAnnotations = false;
private boolean enableMergeAnnotations = false;
@Override
@@ -41,7 +42,7 @@
R8TestCompileResult internalCompile(
Builder builder, Consumer<InternalOptions> optionsConsumer, Supplier<AndroidApp> app)
throws CompilationFailedException {
- if (enableInliningAnnotations || enableMergeAnnotations) {
+ if (enableInliningAnnotations || enableClassInliningAnnotations || enableMergeAnnotations) {
ToolHelper.allowTestProguardOptions(builder);
}
StringBuilder proguardMapBuilder = new StringBuilder();
@@ -71,6 +72,14 @@
return self();
}
+ public R8TestBuilder enableClassInliningAnnotations() {
+ if (!enableClassInliningAnnotations) {
+ enableClassInliningAnnotations = true;
+ addKeepRules("-neverclassinline @com.android.tools.r8.NeverClassInline class *");
+ }
+ return self();
+ }
+
public R8TestBuilder enableMergeAnnotations() {
if (!enableMergeAnnotations) {
enableMergeAnnotations = true;
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index d6e7c0d..f968b8c 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -143,6 +143,10 @@
this.shortName = shortName;
}
+ public boolean isLatest() {
+ return this == DEFAULT;
+ }
+
public boolean isNewerThan(Version other) {
return compareTo(other) > 0;
}
diff --git a/src/test/java/com/android/tools/r8/d8/IncompatiblePrimitiveTypesTest.java b/src/test/java/com/android/tools/r8/d8/IncompatiblePrimitiveTypesTest.java
new file mode 100644
index 0000000..010f903
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/d8/IncompatiblePrimitiveTypesTest.java
@@ -0,0 +1,80 @@
+// 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.d8;
+
+import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestRunResult;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
+import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
+import org.hamcrest.Matcher;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class IncompatiblePrimitiveTypesTest extends TestBase {
+
+ @ClassRule
+ public static TemporaryFolder tempFolder = ToolHelper.getTemporaryFolderForTest();
+
+ private static Path inputJar;
+ private static String expectedOutput = "true";
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ ClassBuilder classBuilder = jasminBuilder.addClass("TestClass");
+ classBuilder
+ .staticMethodBuilder("main", ImmutableList.of("[Ljava/lang/String;"), "V")
+ .setCode(
+ "invokestatic TestClass/getByte()B", "invokestatic TestClass/takeBoolean(Z)V", "return")
+ .build();
+ classBuilder
+ .staticMethodBuilder("getByte", ImmutableList.of(), "B")
+ .setCode("iconst_1", "ireturn")
+ .build();
+ classBuilder
+ .staticMethodBuilder("takeBoolean", ImmutableList.of("Z"), "V")
+ .setCode(
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "iload_0",
+ "invokevirtual java/io/PrintStream/print(Z)V",
+ "return")
+ .build();
+ inputJar = tempFolder.getRoot().toPath().resolve("input.jar");
+ jasminBuilder.writeJar(inputJar);
+ }
+
+ @Test
+ public void jvmTest() throws Exception {
+ assumeTrue(
+ "JVM test independent of Art version - only run when testing on latest",
+ ToolHelper.getDexVm().getVersion().isLatest());
+ testForJvm().addClasspath(inputJar).run("TestClass").assertSuccessWithOutput(expectedOutput);
+ }
+
+ @Test
+ public void dexTest() throws Exception {
+ TestRunResult d8Result = testForD8().addProgramFiles(inputJar).run("TestClass");
+ TestRunResult dxResult = testForDX().addProgramFiles(inputJar).run("TestClass");
+ if (ToolHelper.getDexVm().getVersion().isNewerThan(Version.V4_4_4)) {
+ d8Result.assertSuccessWithOutput(expectedOutput);
+ dxResult.assertSuccessWithOutput(expectedOutput);
+ } else {
+ // TODO(b/119812046): On Art 4.0.4 and 4.4.4 it is a verification error to use one short type
+ // as another short type.
+ Matcher<String> expectedError = containsString("java.lang.VerifyError");
+ d8Result.assertFailureWithErrorThatMatches(expectedError);
+ dxResult.assertFailureWithErrorThatMatches(expectedError);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/ParameterRewritingTest.java b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/ParameterRewritingTest.java
new file mode 100644
index 0000000..d9dfcd8
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/ParameterRewritingTest.java
@@ -0,0 +1,136 @@
+// 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.ir.optimize.uninstantiatedtypes;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.graph.DexMethod;
+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.Ignore;
+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 ParameterRewritingTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public ParameterRewritingTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Ignore("b/110806787")
+ @Test
+ public void test() throws Exception {
+ String expected =
+ StringUtils.lines(
+ "Factory.createStatic() -> null",
+ "Factory.createStaticWithUnused1() -> null",
+ "Factory.createStaticWithUnused2() -> null",
+ "Factory.createStaticWithUnused3() -> null");
+
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expected);
+
+ CodeInspector inspector =
+ testForR8(backend)
+ .addInnerClasses(ParameterRewritingTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .addKeepRules("-dontobfuscate")
+ .addOptionsModification(options -> options.enableClassInlining = false)
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expected)
+ .inspector();
+
+ ClassSubject factoryClassSubject = inspector.clazz(Factory.class);
+ MethodSubject createStaticMethodSubject =
+ factoryClassSubject.uniqueMethodWithName("createStatic");
+ assertThat(createStaticMethodSubject, isPresent());
+ assertEquals(1, createStaticMethodSubject.getMethod().method.proto.parameters.size());
+
+ for (int i = 1; i <= 3; ++i) {
+ String createStaticWithUnusedMethodName = "createStaticWithUnused" + i;
+ MethodSubject createStaticWithUnusedMethodSubject =
+ factoryClassSubject.uniqueMethodWithName(createStaticWithUnusedMethodName);
+ assertThat(createStaticWithUnusedMethodSubject, isPresent());
+
+ DexMethod method = createStaticWithUnusedMethodSubject.getMethod().method;
+ assertEquals(1, method.proto.parameters.size());
+ assertEquals("java.lang.String", method.proto.parameters.toString());
+ }
+
+ assertThat(inspector.clazz(Uninstantiated.class), not(isPresent()));
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ Object obj1 = Factory.createStatic(null, "Factory.createStatic()");
+ System.out.println(" -> " + obj1);
+
+ Object obj2 =
+ Factory.createStaticWithUnused1(new Object(), null, "Factory.createStaticWithUnused1()");
+ System.out.println(" -> " + obj2);
+
+ Object obj3 =
+ Factory.createStaticWithUnused2(null, new Object(), "Factory.createStaticWithUnused2()");
+ System.out.println(" -> " + obj3);
+
+ Object obj4 =
+ Factory.createStaticWithUnused3(null, "Factory.createStaticWithUnused3()", new Object());
+ System.out.println(" -> " + obj4);
+ }
+ }
+
+ @NeverMerge
+ static class Uninstantiated {}
+
+ @NeverMerge
+ static class Factory {
+
+ @NeverInline
+ public static Object createStatic(Uninstantiated uninstantiated, String msg) {
+ System.out.print(msg);
+ return uninstantiated;
+ }
+
+ @NeverInline
+ public static Object createStaticWithUnused1(
+ Object unused, Uninstantiated uninstantiated, String msg) {
+ System.out.print(msg);
+ return uninstantiated;
+ }
+
+ @NeverInline
+ public static Object createStaticWithUnused2(
+ Uninstantiated uninstantiated, Object unused, String msg) {
+ System.out.print(msg);
+ return uninstantiated;
+ }
+
+ @NeverInline
+ public static Object createStaticWithUnused3(
+ Uninstantiated uninstantiated, String msg, Object unused) {
+ System.out.print(msg);
+ return uninstantiated;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/VoidReturnTypeRewritingTest.java b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/VoidReturnTypeRewritingTest.java
new file mode 100644
index 0000000..0fb68e6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/VoidReturnTypeRewritingTest.java
@@ -0,0 +1,149 @@
+// 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.ir.optimize.uninstantiatedtypes;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+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.Ignore;
+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 VoidReturnTypeRewritingTest extends TestBase {
+
+ private final Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ public VoidReturnTypeRewritingTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Ignore("b/110806787")
+ @Test
+ public void test() throws Exception {
+ String expected =
+ StringUtils.lines(
+ "Factory.createStatic() -> null",
+ "Factory.createVirtual() -> null",
+ "SubFactory.createVirtual() -> null",
+ "SubSubFactory.createVirtual() -> null");
+
+ testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expected);
+
+ CodeInspector inspector =
+ testForR8(backend)
+ .addInnerClasses(VoidReturnTypeRewritingTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .addKeepRules("-dontobfuscate")
+ .addOptionsModification(options -> options.enableClassInlining = false)
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expected)
+ .inspector();
+
+ ClassSubject factoryClassSubject = inspector.clazz(Factory.class);
+ MethodSubject createStaticMethodSubject =
+ factoryClassSubject.uniqueMethodWithName("createStatic");
+ assertThat(createStaticMethodSubject, isPresent());
+ assertTrue(createStaticMethodSubject.getMethod().method.proto.returnType.isVoidType());
+ MethodSubject createVirtualMethodSubject =
+ factoryClassSubject.uniqueMethodWithName("createVirtual");
+ assertThat(createVirtualMethodSubject, isPresent());
+ assertTrue(createVirtualMethodSubject.getMethod().method.proto.returnType.isVoidType());
+
+ createVirtualMethodSubject =
+ inspector.clazz(SubFactory.class).uniqueMethodWithName("createVirtual");
+ assertThat(createVirtualMethodSubject, isPresent());
+ assertTrue(createVirtualMethodSubject.getMethod().method.proto.returnType.isVoidType());
+
+ ClassSubject subSubFactoryClassSubject = inspector.clazz(SubSubFactory.class);
+ assertThat(subSubFactoryClassSubject.method("void", "createVirtual"), isPresent());
+ assertThat(
+ subSubFactoryClassSubject.method(SubUninstantiated.class.getTypeName(), "createVirtual"),
+ isPresent());
+
+ // TODO(b/110806787): Uninstantiated is kept because SubUninstantiated inherits from it.
+ // We should consider rewriting SubUninstantiated such that it no longer inherits from
+ // Uninstantiated.
+ assertThat(inspector.clazz(Uninstantiated.class), isPresent());
+ assertThat(inspector.clazz(SubUninstantiated.class), isPresent());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ Uninstantiated obj1 = Factory.createStatic();
+ System.out.println(" -> " + obj1);
+
+ Uninstantiated obj2 = new Factory().createVirtual();
+ System.out.println(" -> " + obj2);
+
+ Uninstantiated obj3 = new SubFactory().createVirtual();
+ System.out.println(" -> " + obj3);
+
+ Uninstantiated obj4 = new SubSubFactory().createVirtual();
+ System.out.println(" -> " + obj4);
+ }
+ }
+
+ @NeverMerge
+ static class Uninstantiated {}
+
+ @NeverMerge
+ static class SubUninstantiated extends Uninstantiated {}
+
+ @NeverMerge
+ static class Factory {
+
+ @NeverInline
+ public static Uninstantiated createStatic() {
+ System.out.print("Factory.createStatic()");
+ return null;
+ }
+
+ @NeverInline
+ public Uninstantiated createVirtual() {
+ System.out.print("Factory.createVirtual()");
+ return null;
+ }
+ }
+
+ @NeverMerge
+ static class SubFactory extends Factory {
+
+ @Override
+ @NeverInline
+ public Uninstantiated createVirtual() {
+ System.out.print("SubFactory.createVirtual()");
+ return null;
+ }
+ }
+
+ @NeverMerge
+ static class SubSubFactory extends SubFactory {
+
+ @Override
+ @NeverInline
+ public SubUninstantiated createVirtual() {
+ System.out.print("SubSubFactory.createVirtual()");
+ return null;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/AdaptResourceFileContentsTest.java b/src/test/java/com/android/tools/r8/naming/AdaptResourceFileContentsTest.java
index 4a7db37..0f3accb 100644
--- a/src/test/java/com/android/tools/r8/naming/AdaptResourceFileContentsTest.java
+++ b/src/test/java/com/android/tools/r8/naming/AdaptResourceFileContentsTest.java
@@ -168,7 +168,8 @@
getProguardConfig(enableAdaptResourceFileContents, adaptResourceFileContentsPathFilter),
"-neverinline class com.android.tools.r8.naming.AdaptResourceFileContentsTestClass$B {",
" public void method();",
- "}");
+ "}",
+ "-neverclassinline class com.android.tools.r8.naming.AdaptResourceFileContentsTestClass$B");
}
@Test
@@ -324,13 +325,7 @@
.addDataResources(getDataResources())
.enableProguardTestOptions()
.addKeepRules(proguardConfig)
- .addOptionsModification(
- o -> {
- // TODO(christofferqa): Class inliner should respect -neverinline.
- o.enableClassInlining = false;
- o.enableVerticalClassMerging = true;
- o.dataResourceConsumer = dataResourceConsumer;
- })
+ .addOptionsModification(o -> o.dataResourceConsumer = dataResourceConsumer)
.compile();
}
diff --git a/src/test/java/com/android/tools/r8/naming/AdaptResourceFileNamesTest.java b/src/test/java/com/android/tools/r8/naming/AdaptResourceFileNamesTest.java
index ecfddf4..095d7e9 100644
--- a/src/test/java/com/android/tools/r8/naming/AdaptResourceFileNamesTest.java
+++ b/src/test/java/com/android/tools/r8/naming/AdaptResourceFileNamesTest.java
@@ -27,6 +27,7 @@
import com.android.tools.r8.utils.ArchiveResourceProvider;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.KeepingDiagnosticHandler;
+import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.io.File;
@@ -93,15 +94,15 @@
private static String getProguardConfigWithNeverInline(
boolean enableAdaptResourceFileNames, String adaptResourceFileNamesPathFilter) {
- return String.join(
- System.lineSeparator(),
+ return StringUtils.lines(
getProguardConfig(enableAdaptResourceFileNames, adaptResourceFileNamesPathFilter),
"-neverinline class " + adaptresourcefilenames.A.class.getName() + " {",
" public void method();",
"}",
"-neverinline class " + adaptresourcefilenames.B.Inner.class.getName() + " {",
" public void method();",
- "}");
+ "}",
+ "-neverclassinline class *");
}
@Test
@@ -262,9 +263,6 @@
return ToolHelper.runR8(
command,
options -> {
- // TODO(christofferqa): Class inliner should respect -neverinline.
- options.enableClassInlining = false;
- options.enableVerticalClassMerging = true;
options.dataResourceConsumer = dataResourceConsumer;
options.proguardMapConsumer = proguardMapConsumer;
options.testing.suppressExperimentalCfBackendWarning = true;
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
index e420e1e..a603f92 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
@@ -11,13 +11,11 @@
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatibilityTestBase;
-import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
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 com.google.common.collect.ImmutableList;
-import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
import org.junit.Test;
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/inlining/IfRuleWithInlining.java b/src/test/java/com/android/tools/r8/shaking/ifrule/inlining/IfRuleWithInlining.java
index d02fa5b..de981ae 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/inlining/IfRuleWithInlining.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/inlining/IfRuleWithInlining.java
@@ -48,18 +48,17 @@
@RunWith(Parameterized.class)
public class IfRuleWithInlining extends ProguardCompatibilityTestBase {
- private final static List<Class> CLASSES = ImmutableList.of(
- A.class, D.class, Main.class);
+ private static final List<Class> CLASSES = ImmutableList.of(A.class, D.class, Main.class);
private final Shrinker shrinker;
- private final boolean inlineMethod;
+ private final boolean neverInlineMethod;
- public IfRuleWithInlining(Shrinker shrinker, boolean inlineMethod) {
+ public IfRuleWithInlining(Shrinker shrinker, boolean neverInlineMethod) {
this.shrinker = shrinker;
- this.inlineMethod = inlineMethod;
+ this.neverInlineMethod = neverInlineMethod;
}
- @Parameters(name = "shrinker: {0} inlineMethod: {1}")
+ @Parameters(name = "shrinker: {0} neverInlineMethod: {1}")
public static Collection<Object[]> data() {
// We don't run this on Proguard, as triggering inlining in Proguard is out of our control.
return ImmutableList.of(
@@ -73,10 +72,9 @@
CodeInspector inspector = new CodeInspector(app);
ClassSubject clazzA = inspector.clazz(A.class);
assertThat(clazzA, isPresent());
- // A.a might be inlined.
- assertEquals(!inlineMethod, clazzA.method("int", "a", ImmutableList.of()).isPresent());
- // TODO(110148109): class D should be present - inlining or not.
- assertEquals(!inlineMethod, inspector.clazz(D.class).isPresent());
+ // A.a should not be inlined.
+ assertThat(clazzA.method("int", "a", ImmutableList.of()), isPresent());
+ assertThat(inspector.clazz(D.class), isPresent());
ProcessResult result;
if (shrinker == Shrinker.R8) {
result = runOnArtRaw(app, Main.class.getName());
@@ -87,22 +85,18 @@
result = ToolHelper.runJava(file, Main.class.getName());
}
assertEquals(0, result.exitCode);
- // TODO(110148109): Output should be the same - inlining or not.
- assertEquals(!inlineMethod ? "1" : "2", result.stdout);
+ assertEquals("1", result.stdout);
}
@Test
public void testMergedClassMethodInIfRule() throws Exception {
- List<String> config = ImmutableList.of(
- "-keep class **.Main { public static void main(java.lang.String[]); }",
- inlineMethod
- ? "-forceinline class **.A { int a(); }"
- : "-neverinline class **.A { int a(); }",
- "-if class **.A { static int a(); }",
- "-keep class **.D",
- "-dontobfuscate"
- );
-
+ List<String> config =
+ ImmutableList.of(
+ "-keep class **.Main { public static void main(java.lang.String[]); }",
+ neverInlineMethod ? "-neverinline class **.A { int a(); }" : "",
+ "-if class **.A { static int a(); }",
+ "-keep class **.D",
+ "-dontobfuscate");
check(runShrinker(shrinker, CLASSES, config));
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java
index 57ad556..f470aab 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/IfRuleWithVerticalClassMerging.java
@@ -8,14 +8,13 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.TestBase;
-import com.android.tools.r8.ir.optimize.Inliner.Reason;
-import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
+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.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import java.util.Collection;
import java.util.List;
import org.junit.Test;
@@ -23,6 +22,7 @@
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
+@NeverClassInline
class A {
int x = 1;
int a() throws ClassNotFoundException {
@@ -34,6 +34,7 @@
}
}
+@NeverClassInline
class B extends A {
int y = 2;
int b() {
@@ -41,6 +42,7 @@
}
}
+@NeverClassInline
class C extends B {
int z = 3;
int c() {
@@ -48,8 +50,8 @@
}
}
-class D {
-}
+@NeverClassInline
+class D {}
class Main {
public static void main(String[] args) throws ClassNotFoundException {
@@ -61,7 +63,7 @@
@RunWith(Parameterized.class)
public class IfRuleWithVerticalClassMerging extends TestBase {
- private static final List<Class> CLASSES =
+ private static final List<Class<?>> CLASSES =
ImmutableList.of(A.class, B.class, C.class, D.class, Main.class);
private final Backend backend;
@@ -84,35 +86,15 @@
private void configure(InternalOptions options) {
options.enableVerticalClassMerging = enableVerticalClassMerging;
-
- // TODO(b/110148109): Allow ordinary method inlining when -if rules work with inlining.
- options.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE);
- }
-
- private void check(AndroidApp app) throws Exception {
- CodeInspector inspector = new CodeInspector(app);
- ClassSubject clazzA = inspector.clazz(A.class);
- assertEquals(!enableVerticalClassMerging, clazzA.isPresent());
- ClassSubject clazzB = inspector.clazz(B.class);
- assertThat(clazzB, isPresent());
- ClassSubject clazzD = inspector.clazz(D.class);
- assertThat(clazzD, isPresent());
- assertEquals("123456", runOnVM(app, Main.class, backend));
}
@Test
public void testMergedClassInIfRule() throws Exception {
// Class C is kept, meaning that it will not be touched.
// Class A will be merged into class B.
- String config =
- String.join(
- System.lineSeparator(),
- "-keep class **.Main { public static void main(java.lang.String[]); }",
- "-keep class **.C",
- "-if class **.A",
- "-keep class **.D",
- "-dontobfuscate");
- check(compileWithR8(readClasses(CLASSES), config, this::configure, backend));
+ runTestWithProguardConfig(
+ StringUtils.lines(
+ "-keep class **.C", "-if class **.A", "-keep class **.D", "-dontobfuscate"));
}
@Test
@@ -120,15 +102,9 @@
// Class C is kept, meaning that it will not be touched.
// Class A will be merged into class B.
// Main.main access A.x, so that field exists satisfying the if rule.
- String config =
- String.join(
- System.lineSeparator(),
- "-keep class **.Main { public static void main(java.lang.String[]); }",
- "-keep class **.C",
- "-if class **.A { int x; }",
- "-keep class **.D",
- "-dontobfuscate");
- check(compileWithR8(readClasses(CLASSES), config, this::configure, backend));
+ runTestWithProguardConfig(
+ StringUtils.lines(
+ "-keep class **.C", "-if class **.A { int x; }", "-keep class **.D", "-dontobfuscate"));
}
@Test
@@ -136,14 +112,31 @@
// Class C is kept, meaning that it will not be touched.
// Class A will be merged into class B.
// Main.main access A.a(), that method exists satisfying the if rule.
- String config =
- String.join(
- System.lineSeparator(),
- "-keep class **.Main { public static void main(java.lang.String[]); }",
+ runTestWithProguardConfig(
+ StringUtils.lines(
"-keep class **.C",
"-if class **.A { int a(); }",
"-keep class **.D",
- "-dontobfuscate");
- check(compileWithR8(readClasses(CLASSES), config, this::configure, backend));
+ "-dontobfuscate"));
+ }
+
+ private void runTestWithProguardConfig(String config) throws Exception {
+ CodeInspector inspector =
+ testForR8(backend)
+ .addProgramClasses(CLASSES)
+ .addKeepMainRule(Main.class)
+ .addKeepRules(config)
+ .enableClassInliningAnnotations()
+ .addOptionsModification(this::configure)
+ .run(Main.class)
+ .assertSuccessWithOutput("123456")
+ .inspector();
+
+ ClassSubject clazzA = inspector.clazz(A.class);
+ assertEquals(!enableVerticalClassMerging, clazzA.isPresent());
+ ClassSubject clazzB = inspector.clazz(B.class);
+ assertThat(clazzB, isPresent());
+ ClassSubject clazzD = inspector.clazz(D.class);
+ assertThat(clazzD, isPresent());
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedFieldTypeTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedFieldTypeTest.java
index 7c85a7b..879efe0 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedFieldTypeTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedFieldTypeTest.java
@@ -10,6 +10,7 @@
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertThat;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.google.common.collect.ImmutableList;
@@ -55,6 +56,7 @@
}
}
+ @NeverClassInline
static class TestClass extends SuperTestClass {
private A field = null;
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedTypeBaseTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedTypeBaseTest.java
index cda8096..10a5ba0 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedTypeBaseTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/verticalclassmerging/MergedTypeBaseTest.java
@@ -5,18 +5,14 @@
package com.android.tools.r8.shaking.ifrule.verticalclassmerging;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsNot.not;
import static org.junit.Assert.assertEquals;
import com.android.tools.r8.TestBase;
-import com.android.tools.r8.ir.optimize.Inliner.Reason;
-import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import java.util.Collection;
import java.util.List;
import org.junit.Test;
@@ -42,7 +38,7 @@
static class Unused {}
final Backend backend;
- final List<Class> classes;
+ final List<Class<?>> classes;
final boolean enableVerticalClassMerging;
public MergedTypeBaseTest(Backend backend, boolean enableVerticalClassMerging) {
@@ -54,7 +50,7 @@
this.backend = backend;
this.enableVerticalClassMerging = enableVerticalClassMerging;
this.classes =
- ImmutableList.<Class>builder()
+ ImmutableList.<Class<?>>builder()
.add(A.class, B.class, C.class, I.class, J.class, K.class, Unused.class, getTestClass())
.addAll(additionalClasses)
.build();
@@ -95,17 +91,18 @@
String expected = getExpectedStdout();
assertEquals(expected, runOnJava(getTestClass()));
- String config =
- StringUtils.joinLines(
- "-keep class " + getTestClass().getTypeName() + " {",
- " public static void main(java.lang.String[]);",
- "}",
+ testForR8(backend)
+ .addProgramClasses(classes)
+ .addKeepMainRule(getTestClass())
+ .addKeepRules(
getConditionForProguardIfRule(),
"-keep class " + Unused.class.getTypeName(),
- getAdditionalKeepRules());
- AndroidApp output = compileWithR8(readClasses(classes), config, this::configure, backend);
- assertEquals(expected, runOnVM(output, getTestClass(), backend));
- inspect(new CodeInspector(output));
+ getAdditionalKeepRules())
+ .addOptionsModification(this::configure)
+ .enableClassInliningAnnotations()
+ .run(getTestClass())
+ .assertSuccessWithOutput(expected)
+ .inspect(this::inspect);
}
private void configure(InternalOptions options) {
@@ -115,8 +112,5 @@
// To ensure that the handling of extends and implements rules work as intended,
// and that we don't end up keeping `Unused` only because one of the two implementations work.
options.testing.allowProguardRulesThatUseExtendsOrImplementsWrong = false;
-
- // TODO(b/110148109): Allow ordinary method inlining when -if rules work with inlining.
- options.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE);
}
}
