Allow merging classes with different interfaces
Change-Id: I3048f517e0df31cacab795cc441c8a1555bac8e6
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 5215ada..dbc2868 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -130,6 +130,10 @@
return interfaces;
}
+ public void setInterfaces(DexTypeList interfaces) {
+ this.interfaces = interfaces;
+ }
+
public DexString getSourceFile() {
return sourceFile;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexTypeList.java b/src/main/java/com/android/tools/r8/graph/DexTypeList.java
index a641039..e4462ac 100644
--- a/src/main/java/com/android/tools/r8/graph/DexTypeList.java
+++ b/src/main/java/com/android/tools/r8/graph/DexTypeList.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import static com.android.tools.r8.utils.PredicateUtils.not;
+
import com.android.tools.r8.dex.IndexedItemCollection;
import com.android.tools.r8.dex.MixedSectionCollection;
import com.android.tools.r8.errors.Unreachable;
@@ -13,8 +15,10 @@
import com.android.tools.r8.utils.structural.StructuralItem;
import com.google.common.collect.Iterators;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Iterator;
import java.util.function.Consumer;
+import java.util.function.Predicate;
import java.util.stream.Stream;
public class DexTypeList extends DexItem implements Iterable<DexType>, StructuralItem<DexTypeList> {
@@ -36,12 +40,36 @@
this.values = values;
}
+ public DexTypeList(Collection<DexType> values) {
+ this(values.toArray(DexType.EMPTY_ARRAY));
+ }
+
+ public static DexTypeList create(DexType[] values) {
+ return values.length == 0 ? DexTypeList.empty() : new DexTypeList(values);
+ }
+
+ public static DexTypeList create(Collection<DexType> values) {
+ return values.isEmpty() ? DexTypeList.empty() : new DexTypeList(values);
+ }
+
@Override
public StructuralAccept<DexTypeList> getStructuralAccept() {
// Structural accept is never accessed as all accept methods are defined directly.
throw new Unreachable();
}
+ public DexTypeList keepIf(Predicate<DexType> predicate) {
+ DexType[] filtered = ArrayUtils.filter(DexType[].class, values, predicate);
+ if (filtered != values) {
+ return DexTypeList.create(filtered);
+ }
+ return this;
+ }
+
+ public DexTypeList removeIf(Predicate<DexType> predicate) {
+ return keepIf(not(predicate));
+ }
+
@Override
public DexTypeList self() {
return this;
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
index f75f969..dc0310c 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -17,6 +17,7 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.DexTypeList;
import com.android.tools.r8.graph.FieldAccessFlags;
import com.android.tools.r8.graph.GenericSignature.FieldTypeSignature;
import com.android.tools.r8.graph.ProgramMethod;
@@ -25,6 +26,7 @@
import com.android.tools.r8.utils.MethodSignatureEquivalence;
import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
import java.util.ArrayList;
@@ -34,6 +36,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
/**
* The class merger is responsible for moving methods from {@link ClassMerger#toMergeGroup} into the
@@ -194,6 +197,16 @@
}
}
+ private void mergeInterfaces() {
+ Set<DexType> interfaces = Sets.newLinkedHashSet(target.getInterfaces());
+ for (DexProgramClass clazz : toMergeGroup) {
+ Iterables.addAll(interfaces, clazz.getInterfaces());
+ }
+ if (interfaces.size() > target.getInterfaces().size()) {
+ target.setInterfaces(new DexTypeList(interfaces));
+ }
+ }
+
void mergeInstanceFields() {
toMergeGroup.forEach(
clazz -> {
@@ -207,6 +220,8 @@
fixAccessFlags();
appendClassIdField();
+ mergeInterfaces();
+
mergeVirtualMethods();
mergeDirectMethods(syntheticArgumentClass);
classMethodsBuilder.setClassMethods(target);
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
index 69be609..097bb58 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -9,7 +9,6 @@
import com.android.tools.r8.graph.DirectMappedDexApplication;
import com.android.tools.r8.horizontalclassmerging.policies.AllInstantiatedOrUninstantiated;
import com.android.tools.r8.horizontalclassmerging.policies.CheckAbstractClasses;
-import com.android.tools.r8.horizontalclassmerging.policies.ClassesHaveSameInterfaces;
import com.android.tools.r8.horizontalclassmerging.policies.DontInlinePolicy;
import com.android.tools.r8.horizontalclassmerging.policies.DontMergeIntoLessVisible;
import com.android.tools.r8.horizontalclassmerging.policies.DontMergeSynchronizedClasses;
@@ -17,14 +16,15 @@
import com.android.tools.r8.horizontalclassmerging.policies.LimitGroups;
import com.android.tools.r8.horizontalclassmerging.policies.NoAnnotations;
import com.android.tools.r8.horizontalclassmerging.policies.NoClassesOrMembersWithAnnotations;
+import com.android.tools.r8.horizontalclassmerging.policies.NoDirectRuntimeTypeChecks;
import com.android.tools.r8.horizontalclassmerging.policies.NoEnums;
+import com.android.tools.r8.horizontalclassmerging.policies.NoIndirectRuntimeTypeChecks;
import com.android.tools.r8.horizontalclassmerging.policies.NoInnerClasses;
import com.android.tools.r8.horizontalclassmerging.policies.NoInterfaces;
import com.android.tools.r8.horizontalclassmerging.policies.NoKeepRules;
import com.android.tools.r8.horizontalclassmerging.policies.NoKotlinLambdas;
import com.android.tools.r8.horizontalclassmerging.policies.NoKotlinMetadata;
import com.android.tools.r8.horizontalclassmerging.policies.NoNativeMethods;
-import com.android.tools.r8.horizontalclassmerging.policies.NoRuntimeTypeChecks;
import com.android.tools.r8.horizontalclassmerging.policies.NoServiceLoaders;
import com.android.tools.r8.horizontalclassmerging.policies.NoStaticClassInitializer;
import com.android.tools.r8.horizontalclassmerging.policies.NotMatchedByNoHorizontalClassMerging;
@@ -109,7 +109,6 @@
new NotMatchedByNoHorizontalClassMerging(appView),
new SameFields(),
new NoInterfaces(),
- new ClassesHaveSameInterfaces(),
new NoAnnotations(),
new NoEnums(appView),
new CheckAbstractClasses(appView),
@@ -123,7 +122,8 @@
new NoKotlinLambdas(appView),
new NoServiceLoaders(appView),
new NotVerticallyMergedIntoSubtype(appView),
- new NoRuntimeTypeChecks(runtimeTypeCheckInfo),
+ new NoDirectRuntimeTypeChecks(runtimeTypeCheckInfo),
+ new NoIndirectRuntimeTypeChecks(appView, runtimeTypeCheckInfo),
new PreventMethodImplementation(appView),
new DontInlinePolicy(appView, mainDexTracingResult),
new PreventMergeIntoMainDex(appView, mainDexTracingResult),
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/ClassesHaveSameInterfaces.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/ClassesHaveSameInterfaces.java
deleted file mode 100644
index 060e1a6..0000000
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/ClassesHaveSameInterfaces.java
+++ /dev/null
@@ -1,17 +0,0 @@
-// Copyright (c) 2020, 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.horizontalclassmerging.policies;
-
-import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.DexTypeList;
-import com.android.tools.r8.horizontalclassmerging.MultiClassSameReferencePolicy;
-
-public class ClassesHaveSameInterfaces extends MultiClassSameReferencePolicy<DexTypeList> {
-
- @Override
- public DexTypeList getMergeKey(DexProgramClass clazz) {
- return clazz.interfaces.getSorted();
- }
-}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoRuntimeTypeChecks.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoDirectRuntimeTypeChecks.java
similarity index 75%
rename from src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoRuntimeTypeChecks.java
rename to src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoDirectRuntimeTypeChecks.java
index 010fee8..b0f3766 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoRuntimeTypeChecks.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoDirectRuntimeTypeChecks.java
@@ -8,16 +8,15 @@
import com.android.tools.r8.horizontalclassmerging.SingleClassPolicy;
import com.android.tools.r8.shaking.RuntimeTypeCheckInfo;
-public class NoRuntimeTypeChecks extends SingleClassPolicy {
+public class NoDirectRuntimeTypeChecks extends SingleClassPolicy {
private final RuntimeTypeCheckInfo runtimeTypeCheckInfo;
- public NoRuntimeTypeChecks(RuntimeTypeCheckInfo runtimeTypeCheckInfo) {
+ public NoDirectRuntimeTypeChecks(RuntimeTypeCheckInfo runtimeTypeCheckInfo) {
this.runtimeTypeCheckInfo = runtimeTypeCheckInfo;
}
@Override
public boolean canMerge(DexProgramClass clazz) {
- // We currently assume we only merge classes that implement the same set of interfaces.
return !runtimeTypeCheckInfo.isRuntimeCheckType(clazz);
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoIndirectRuntimeTypeChecks.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoIndirectRuntimeTypeChecks.java
new file mode 100644
index 0000000..a7e9ee8
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoIndirectRuntimeTypeChecks.java
@@ -0,0 +1,68 @@
+// Copyright (c) 2020, 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.horizontalclassmerging.policies;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.DexTypeList;
+import com.android.tools.r8.horizontalclassmerging.MultiClassSameReferencePolicy;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.RuntimeTypeCheckInfo;
+import it.unimi.dsi.fastutil.objects.Reference2BooleanMap;
+import it.unimi.dsi.fastutil.objects.Reference2BooleanOpenHashMap;
+
+public class NoIndirectRuntimeTypeChecks extends MultiClassSameReferencePolicy<DexTypeList> {
+
+ private final AppView<AppInfoWithLiveness> appView;
+ private final RuntimeTypeCheckInfo runtimeTypeCheckInfo;
+
+ private final Reference2BooleanMap<DexType> cache = new Reference2BooleanOpenHashMap<>();
+
+ public NoIndirectRuntimeTypeChecks(
+ AppView<AppInfoWithLiveness> appView, RuntimeTypeCheckInfo runtimeTypeCheckInfo) {
+ this.appView = appView;
+ this.runtimeTypeCheckInfo = runtimeTypeCheckInfo;
+ }
+
+ @Override
+ public DexTypeList getMergeKey(DexProgramClass clazz) {
+ // Require that classes that implement an interface that has a runtime type check (directly or
+ // indirectly on a parent interface) are only merged with classes that implement the same
+ // interfaces.
+ return clazz
+ .getInterfaces()
+ .keepIf(this::computeInterfaceHasDirectOrIndirectRuntimeTypeCheck)
+ .getSorted();
+ }
+
+ private boolean computeInterfaceHasDirectOrIndirectRuntimeTypeCheck(DexType type) {
+ if (cache.containsKey(type)) {
+ return cache.getBoolean(type);
+ }
+ DexClass clazz = appView.definitionFor(type);
+ if (!clazz.isInterface()) {
+ cache.put(type, true);
+ return true;
+ }
+ if (!clazz.isProgramClass()) {
+ // Conservatively return true because there could be a type check in the library.
+ cache.put(type, true);
+ return true;
+ }
+ if (runtimeTypeCheckInfo.isRuntimeCheckType(clazz.asProgramClass())) {
+ cache.put(type, true);
+ return true;
+ }
+ for (DexType parentType : clazz.getInterfaces()) {
+ if (computeInterfaceHasDirectOrIndirectRuntimeTypeCheck(parentType)) {
+ cache.put(type, true);
+ return true;
+ }
+ }
+ return false;
+ }
+}
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 8a8d2d4..a179513 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
@@ -265,6 +265,14 @@
}
if (inliner.neverInline(invoke, resolutionResult, singleTarget, whyAreYouNotInliningReporter)) {
+ if (singleTarget.getDefinition().getOptimizationInfo().forceInline()) {
+ throw new Unreachable(
+ "Unexpected attempt to force inline method `"
+ + singleTarget.toSourceString()
+ + "` in `"
+ + context.toSourceString()
+ + "`.");
+ }
return null;
}
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 3abbf9b..b26bb34 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
@@ -7,7 +7,6 @@
import static com.google.common.base.Predicates.not;
import com.android.tools.r8.androidapi.AvailableApiExceptions;
-import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AccessControl;
import com.android.tools.r8.graph.AccessFlags;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
@@ -125,11 +124,6 @@
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
AppInfoWithLiveness appInfo = appView.appInfo();
DexMethod singleTargetReference = singleTarget.getReference();
- if (singleTarget.getDefinition().getOptimizationInfo().forceInline()
- && appInfo.isNeverInlineMethod(singleTargetReference)) {
- throw new Unreachable();
- }
-
if (appInfo.isPinned(singleTargetReference)) {
whyAreYouNotInliningReporter.reportPinned();
return true;
@@ -151,7 +145,7 @@
if (appInfo.noSideEffects.containsKey(invoke.getInvokedMethod())
|| appInfo.noSideEffects.containsKey(resolutionResult.getResolvedMethod().getReference())
|| appInfo.noSideEffects.containsKey(singleTargetReference)) {
- return true;
+ return !singleTarget.getDefinition().getOptimizationInfo().forceInline();
}
return false;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
index be88541..b0d1fa8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -573,7 +573,7 @@
return null;
}
if (singleTarget.isInstanceInitializer() && invoke.getReceiver() == receiver) {
- if (builder.hasParent()) {
+ if (builder.hasParent() && builder.getParent() != singleTarget.getReference()) {
return null;
}
// java.lang.Enum.<init>() and java.lang.Object.<init>() are considered trivial.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/initializer/NonTrivialInstanceInitializerInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/initializer/NonTrivialInstanceInitializerInfo.java
index ee08c65..75c74dd 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/initializer/NonTrivialInstanceInitializerInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/initializer/NonTrivialInstanceInitializerInfo.java
@@ -189,8 +189,12 @@
return parent != null;
}
+ public DexMethod getParent() {
+ return parent;
+ }
+
public Builder setParent(DexMethod parent) {
- assert !hasParent();
+ assert !hasParent() || getParent() == parent;
this.parent = parent;
return this;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/RuntimeTypeCheckInfo.java b/src/main/java/com/android/tools/r8/shaking/RuntimeTypeCheckInfo.java
index ddfea43..fb3868a 100644
--- a/src/main/java/com/android/tools/r8/shaking/RuntimeTypeCheckInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/RuntimeTypeCheckInfo.java
@@ -49,17 +49,24 @@
@Override
public void traceCheckCast(DexType type, ProgramMethod context) {
- checkCastTypes.add(type.toBaseType(factory));
+ add(type, checkCastTypes);
}
@Override
public void traceInstanceOf(DexType type, ProgramMethod context) {
- instanceOfTypes.add(type.toBaseType(factory));
+ add(type, instanceOfTypes);
}
@Override
public void traceExceptionGuard(DexType guard, ProgramMethod context) {
- exceptionGuardTypes.add(guard);
+ add(guard, exceptionGuardTypes);
+ }
+
+ private void add(DexType type, Set<DexType> set) {
+ DexType baseType = type.toBaseType(factory);
+ if (baseType.isClassType()) {
+ set.add(baseType);
+ }
}
public void attach(Enqueuer enqueuer) {
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index 058c2e5..34d573a 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -484,6 +484,10 @@
return self();
}
+ public T addNoHorizontalClassMergingRule(String clazz) {
+ return addKeepRules("-nohorizontalclassmerging class " + clazz);
+ }
+
public T enableNoHorizontalClassMergingAnnotations() {
if (!enableNoHorizontalClassMergingAnnotations) {
enableNoHorizontalClassMergingAnnotations = true;
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/NonConstructorRelaxationTest.java b/src/test/java/com/android/tools/r8/accessrelaxation/NonConstructorRelaxationTest.java
index 8624277..8cd7860 100644
--- a/src/test/java/com/android/tools/r8/accessrelaxation/NonConstructorRelaxationTest.java
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/NonConstructorRelaxationTest.java
@@ -165,6 +165,7 @@
.enableNeverClassInliningAnnotations()
.enableInliningAnnotations()
.enableMemberValuePropagationAnnotations()
+ .enableNoHorizontalClassMergingAnnotations()
.noMinification()
.addKeepRules(
"-checkdiscard class " + Base.class.getCanonicalName() + "{",
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java
index deb2559..c832fe7 100644
--- a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub1.java
@@ -5,8 +5,10 @@
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoHorizontalClassMerging;
@NeverClassInline
+@NoHorizontalClassMerging
public class Sub1 extends Base implements Itf1 {
@Override
diff --git a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java
index 15cc549..ffdeec6 100644
--- a/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java
+++ b/src/test/java/com/android/tools/r8/accessrelaxation/privateinstance/Sub2.java
@@ -5,8 +5,10 @@
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoHorizontalClassMerging;
@NeverClassInline
+@NoHorizontalClassMerging
public class Sub2 extends Base implements Itf2 {
@Override
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithDifferentInterfacesTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithDifferentInterfacesTest.java
index d1333a3..3c9d066 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithDifferentInterfacesTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithDifferentInterfacesTest.java
@@ -5,13 +5,13 @@
package com.android.tools.r8.classmerging.horizontal;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.notIf;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.utils.codeinspector.HorizontallyMergedClassesInspector;
import org.junit.Test;
public class ClassesWithDifferentInterfacesTest extends HorizontalClassMergingTestBase {
@@ -31,8 +31,8 @@
.enableNeverClassInliningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
- .addHorizontallyMergedClassesInspector(
- HorizontallyMergedClassesInspector::assertNoClassesMerged)
+ .addHorizontallyMergedClassesInspectorIf(
+ enableHorizontalClassMerging, inspector -> inspector.assertMergedInto(Z.class, Y.class))
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("bar", "foo y", "bar")
.inspect(
@@ -40,7 +40,8 @@
assertThat(codeInspector.clazz(I.class), isPresent());
assertThat(codeInspector.clazz(X.class), isPresent());
assertThat(codeInspector.clazz(Y.class), isPresent());
- assertThat(codeInspector.clazz(Z.class), isPresent());
+ assertThat(
+ codeInspector.clazz(Z.class), notIf(isPresent(), enableHorizontalClassMerging));
});
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithIdenticalInterfacesTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithIdenticalInterfacesTest.java
index 5c03d3b..2bab975 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithIdenticalInterfacesTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithIdenticalInterfacesTest.java
@@ -30,14 +30,17 @@
.enableNeverClassInliningAnnotations()
.setMinApi(parameters.getApiLevel())
.addHorizontallyMergedClassesInspectorIf(
- enableHorizontalClassMerging, inspector -> inspector.assertMergedInto(Z.class, Y.class))
+ enableHorizontalClassMerging,
+ inspector ->
+ inspector.assertMergedInto(Y.class, X.class).assertMergedInto(Z.class, X.class))
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("bar", "foo y", "foo z")
.inspect(
codeInspector -> {
assertThat(codeInspector.clazz(I.class), isPresent());
assertThat(codeInspector.clazz(X.class), isPresent());
- assertThat(codeInspector.clazz(Y.class), isPresent());
+ assertThat(
+ codeInspector.clazz(Y.class), notIf(isPresent(), enableHorizontalClassMerging));
assertThat(
codeInspector.clazz(Z.class), notIf(isPresent(), enableHorizontalClassMerging));
});
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/ConflictWasDetectedTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/ConflictWasDetectedTest.java
new file mode 100644
index 0000000..8b24e99
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/ConflictWasDetectedTest.java
@@ -0,0 +1,98 @@
+// Copyright (c) 2020, 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.vertical;
+
+import com.android.tools.r8.KeepUnusedArguments;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.VerticallyMergedClassesInspector;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class ConflictWasDetectedTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public ConflictWasDetectedTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addVerticallyMergedClassesInspector(
+ VerticallyMergedClassesInspector::assertNoClassesMerged)
+ .enableInliningAnnotations()
+ // .enableNoHorizontalClassMergingAnnotations()
+ .enableUnusedArgumentAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), Main.class);
+ }
+
+ static class Main {
+
+ public static void main(String... args) {
+ ConflictingInterfaceImpl impl = new ConflictingInterfaceImpl();
+ callMethodOnIface(impl);
+
+ // Ensure that the instantiations are not dead code eliminated.
+ escape(impl);
+ }
+
+ private static void callMethodOnIface(ConflictingInterface iface) {
+ System.out.println(iface.method());
+ System.out.println(ClassWithConflictingMethod.conflict(null));
+ System.out.println(OtherClassWithConflictingMethod.conflict(null));
+ }
+
+ @NeverInline
+ private static void escape(Object o) {
+ if (System.currentTimeMillis() < 0) {
+ System.out.println(o);
+ }
+ }
+ }
+
+ public interface ConflictingInterface {
+
+ String method();
+ }
+
+ public static class ConflictingInterfaceImpl implements ConflictingInterface {
+
+ @Override
+ public String method() {
+ return "ConflictingInterfaceImpl::method";
+ }
+ }
+
+ public static class ClassWithConflictingMethod {
+
+ @KeepUnusedArguments
+ public static int conflict(ConflictingInterface item) {
+ return 123;
+ }
+ }
+
+ public static class OtherClassWithConflictingMethod {
+
+ @KeepUnusedArguments
+ public static int conflict(ConflictingInterfaceImpl item) {
+ return 321;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/LambdaRewritingTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/LambdaRewritingTest.java
new file mode 100644
index 0000000..4cb9707
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/LambdaRewritingTest.java
@@ -0,0 +1,89 @@
+// Copyright (c) 2020, 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.vertical;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class LambdaRewritingTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public LambdaRewritingTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addVerticallyMergedClassesInspector(
+ inspector -> inspector.assertMergedIntoSubtype(Interface.class))
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), Main.class);
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ Interface obj = new InterfaceImpl();
+
+ // Leads to an invoke-custom instruction that mentions the type of `obj` since it is captured.
+ invoke(() -> obj.foo());
+
+ FunctionImpl functionImpl = new FunctionImpl();
+ if (System.currentTimeMillis() < 0) {
+ System.out.println(functionImpl);
+ }
+ }
+
+ @NeverInline
+ private static void invoke(Function f) {
+ f.accept();
+ }
+ }
+
+ // Cannot be merged as it has two subtypes: FunctionImpl and a lambda.
+ public interface Function {
+
+ void accept();
+ }
+
+ public static class FunctionImpl implements Function {
+
+ @Override
+ public void accept() {
+ System.out.println("In FunctionImpl.accept()");
+ }
+ }
+
+ // Will be merged into InterfaceImpl.
+ public interface Interface {
+
+ void foo();
+ }
+
+ public static class InterfaceImpl implements Interface {
+
+ @Override
+ public void foo() {
+ System.out.println("In InterfaceImpl.foo()");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
index d7d31c9..2fb36cd 100644
--- a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
@@ -305,13 +305,6 @@
}
@Test
- public void testConflictWasDetected() throws Throwable {
- runR8(EXAMPLE_KEEP, this::configure);
- assertThat(inspector.clazz("classmerging.ConflictingInterface"), isPresent());
- assertThat(inspector.clazz("classmerging.ConflictingInterfaceImpl"), isPresent());
- }
-
- @Test
public void testFieldCollision() throws Throwable {
String main = "classmerging.FieldCollisionTest";
Path[] programFiles =
@@ -334,34 +327,6 @@
}
@Test
- public void testLambdaRewriting() throws Throwable {
- String main = "classmerging.LambdaRewritingTest";
- Path[] programFiles =
- new Path[] {
- JAVA8_CF_DIR.resolve("LambdaRewritingTest.class"),
- JAVA8_CF_DIR.resolve("LambdaRewritingTest$Function.class"),
- JAVA8_CF_DIR.resolve("LambdaRewritingTest$FunctionImpl.class"),
- JAVA8_CF_DIR.resolve("LambdaRewritingTest$Interface.class"),
- JAVA8_CF_DIR.resolve("LambdaRewritingTest$InterfaceImpl.class")
- };
- Set<String> preservedClassNames =
- ImmutableSet.of(
- "classmerging.LambdaRewritingTest",
- "classmerging.LambdaRewritingTest$Function",
- "classmerging.LambdaRewritingTest$FunctionImpl",
- "classmerging.LambdaRewritingTest$InterfaceImpl");
- runTestOnInput(
- testForR8(parameters.getBackend())
- .addKeepRules(getProguardConfig(JAVA8_EXAMPLE_KEEP))
- .addOptionsModification(this::configure)
- .addOptionsModification(options -> options.enableClassInlining = false)
- .allowUnusedProguardConfigurationRules(),
- main,
- readProgramFiles(programFiles),
- name -> preservedClassNames.contains(name) || name.contains("$Lambda$"));
- }
-
- @Test
public void testMergeInterfaceWithoutInlining() throws Throwable {
String main = "classmerging.ConflictingInterfaceSignaturesTest";
Path[] programFiles =
diff --git a/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java b/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
index cd743a9..d88e281 100644
--- a/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
@@ -82,9 +82,13 @@
.addKeepMainRule("proto2.TestClass")
.addKeepRuleFiles(PROTOBUF_LITE_PROGUARD_RULES)
.addKeepRules(allGeneratedMessageLiteSubtypesAreInstantiatedRule())
+ // TODO(b/173340579): This rule should not be needed to allow shrinking of
+ // PartiallyUsed$Enum.
+ .addNoHorizontalClassMergingRule(PARTIALLY_USED + "$Enum$1")
.allowAccessModification(allowAccessModification)
.allowDiagnosticMessages()
.allowUnusedProguardConfigurationRules()
+ .enableProguardTestOptions()
.enableProtoShrinking()
.minification(enableMinification)
.setMinApi(parameters.getApiLevel())
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index cf52085..e0266bd 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.optimize.classinliner;
import static com.android.tools.r8.ir.desugar.LambdaRewriter.LAMBDA_CLASS_NAME_PREFIX;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
@@ -88,9 +89,15 @@
.enableSideEffectAnnotations()
.addKeepMainRule(main)
.addKeepAttributes("LineNumberTable")
+ .addHorizontallyMergedClassesInspector(
+ inspector ->
+ inspector
+ .assertMergedInto(Iface1Impl.class, CycleReferenceBA.class)
+ .assertMergedInto(Iface2Impl.class, CycleReferenceBA.class))
.allowAccessModification()
.noMinification()
- .run(main)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), main)
.assertSuccessWithOutput(javaOutput);
CodeInspector inspector = result.inspector();
@@ -143,7 +150,7 @@
"com.android.tools.r8.ir.optimize.classinliner.trivial.CycleReferenceAB"),
collectTypes(inspector.clazz(CycleReferenceAB.class).uniqueMethodWithName("foo")));
- assertFalse(inspector.clazz(CycleReferenceBA.class).isPresent());
+ assertThat(inspector.clazz(CycleReferenceBA.class), isAbsent());
}
@Test
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
index 8dbe868..3e28157 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
@@ -37,6 +37,7 @@
@NoHorizontalClassMerging
static class EffectivelyFinal {}
+ @NoHorizontalClassMerging
static class Reflection implements Callable<Class<?>> {
@ForceInline
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleTargetExecutionTest.java b/src/test/java/com/android/tools/r8/resolution/SingleTargetExecutionTest.java
index bae2760..c6efef7 100644
--- a/src/test/java/com/android/tools/r8/resolution/SingleTargetExecutionTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/SingleTargetExecutionTest.java
@@ -99,6 +99,7 @@
.addProgramClasses(CLASSES)
.addProgramClassFileData(ASM_CLASSES)
.addKeepMainRule(Main.class)
+ .enableNoHorizontalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutput(EXPECTED);
diff --git a/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassOne.java b/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassOne.java
index ff585af..c140fed 100644
--- a/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassOne.java
+++ b/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassOne.java
@@ -3,6 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.resolution.singletarget.one;
+import com.android.tools.r8.NoHorizontalClassMerging;
+
+@NoHorizontalClassMerging
public class SubSubClassOne extends AbstractSubClass implements IrrelevantInterfaceWithDefault {
// Avoid SubSubClassOne.class.getCanonicalName() as it may change during shrinking.
diff --git a/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassTwo.java b/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassTwo.java
index bd4c45c..1534f36 100644
--- a/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassTwo.java
+++ b/src/test/java/com/android/tools/r8/resolution/singletarget/one/SubSubClassTwo.java
@@ -3,6 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.resolution.singletarget.one;
+import com.android.tools.r8.NoHorizontalClassMerging;
+
+@NoHorizontalClassMerging
public class SubSubClassTwo extends AbstractSubClass {
@Override
