Disallow class merging between classes in features and base
Bug: 122902374,141452765
Change-Id: I2c72079e5107967b71e216ab71556a13a1f11a78
diff --git a/src/main/java/com/android/tools/r8/features/FeatureSplitConfiguration.java b/src/main/java/com/android/tools/r8/features/FeatureSplitConfiguration.java
index b7f6158..22e819e 100644
--- a/src/main/java/com/android/tools/r8/features/FeatureSplitConfiguration.java
+++ b/src/main/java/com/android/tools/r8/features/FeatureSplitConfiguration.java
@@ -59,7 +59,12 @@
return result;
}
- public boolean inSameFeatureOrBase(DexMethod a, DexMethod b) {
+ public boolean isInFeature(DexProgramClass clazz) {
+ return javaTypeToFeatureSplitMapping.containsKey(
+ DescriptorUtils.descriptorToJavaType(clazz.type.toDescriptorString()));
+ }
+
+ public boolean inSameFeatureOrBase(DexMethod a, DexMethod b){
return inSameFeatureOrBase(a.holder, b.holder);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
index 7f16e07..487afa2 100644
--- a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
@@ -256,6 +256,11 @@
if (appView.appInfo().neverMerge.contains(clazz.type)) {
return MergeGroup.DONT_MERGE;
}
+ if (appView.options().featureSplitConfiguration != null &&
+ appView.options().featureSplitConfiguration.isInFeature(clazz)) {
+ // TODO(b/141452765): Allow class merging between classes in features.
+ return MergeGroup.DONT_MERGE;
+ }
if (clazz.staticFields().size() + clazz.directMethods().size() + clazz.virtualMethods().size()
== 0) {
return MergeGroup.DONT_MERGE;
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 cd77544..b8b8482 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -374,6 +374,12 @@
|| appInfo.neverMerge.contains(clazz.type)) {
return false;
}
+ if (appView.options().featureSplitConfiguration != null &&
+ appView.options().featureSplitConfiguration.isInFeature(clazz)) {
+ // TODO(b/141452765): Allow class merging between classes in features.
+ return false;
+ }
+
// Note that the property "singleSubtype == null" cannot change during merging, since we visit
// classes in a top-down order.
DexType singleSubtype = appInfo.getSingleSubtype(clazz.type);
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java
index 9ea52d0..25cbe9a 100644
--- a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java
+++ b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java
@@ -4,9 +4,13 @@
package com.android.tools.r8.dexsplitter;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.FeatureSplit;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NeverMerge;
import com.android.tools.r8.R8FullTestBuilder;
@@ -77,6 +81,26 @@
assertTrue(processResult.stderr.contains("NoClassDefFoundError"));
}
+ @Test
+ public void testOnR8Splitter() throws IOException, CompilationFailedException,
+ ExecutionException {
+ assumeTrue(parameters.isDexRuntime());
+ Consumer<R8FullTestBuilder> configurator =
+ r8FullTestBuilder -> r8FullTestBuilder.enableMergeAnnotations().noMinification();
+ ProcessResult processResult =
+ testR8Splitter(
+ parameters,
+ ImmutableSet.of(BaseClass.class, BaseWithStatic.class),
+ ImmutableSet.of(FeatureClass.class, AFeatureWithStatic.class),
+ FeatureClass.class,
+ EXPECTED,
+ a -> true,
+ configurator);
+
+ assertEquals(processResult.exitCode, 0);
+ assertTrue(processResult.stdout.equals(StringUtils.lines("42", "foobar")));
+ }
+
@NeverMerge
public static class BaseClass implements RunInterface {
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java b/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java
index 9d8c12b..b8154cc 100644
--- a/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java
+++ b/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java
@@ -1,6 +1,5 @@
package com.android.tools.r8.dexsplitter;
-import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertTrue;
import static org.junit.Assume.assumeTrue;
@@ -66,7 +65,6 @@
ByteDataView data,
Set<String> descriptors,
DiagnosticsHandler handler) {
- assertEquals(classNames.size(), descriptors.size());
for (String descriptor : descriptors) {
assertTrue(classNames.contains(DescriptorUtils.descriptorToJavaType(descriptor)));
}
