Don't allow inlining of code that will error because of features
Bug: 181571571
Change-Id: I65edb6093405dd605230bf56e61dc60300944a8b
diff --git a/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java b/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
index d96643d..c8c462b 100644
--- a/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
+++ b/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
@@ -159,6 +159,11 @@
public boolean isInBaseOrSameFeatureAs(
DexProgramClass clazz, ProgramDefinition context, SyntheticItems syntheticItems) {
+ return isInBaseOrSameFeatureAs(clazz.getContextType(), context, syntheticItems);
+ }
+
+ public boolean isInBaseOrSameFeatureAs(
+ DexType clazz, ProgramDefinition context, SyntheticItems syntheticItems) {
FeatureSplit split = getFeatureSplit(clazz, syntheticItems);
return split.isBase() || split == getFeatureSplit(context, syntheticItems);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index 976e869..e00c267 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -370,6 +370,17 @@
// This will fail at runtime.
return ConstraintWithTarget.NEVER;
}
+ if (!appView
+ .appInfo()
+ .getClassToFeatureSplitMap()
+ .isInBaseOrSameFeatureAs(
+ resolvedMember.getHolderType(),
+ context.asProgramMethod(),
+ appView.getSyntheticItems())) {
+ // We never inline into the base from a feature (calls should never happen) and we
+ // never inline between features, so this check should be sufficient.
+ return ConstraintWithTarget.NEVER;
+ }
DexType resolvedHolder = graphLens.lookupType(resolvedMember.getHolderType());
assert initialResolutionHolder != null;
ConstraintWithTarget memberConstraintWithTarget =
diff --git a/src/test/java/com/android/tools/r8/regress/Regress181837660.java b/src/test/java/com/android/tools/r8/regress/Regress181837660.java
index 7fd9e77..b5e0a7c 100644
--- a/src/test/java/com/android/tools/r8/regress/Regress181837660.java
+++ b/src/test/java/com/android/tools/r8/regress/Regress181837660.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.regress;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -14,7 +16,6 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.dexsplitter.SplitterTestBase;
-import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableSet;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -28,8 +29,6 @@
@RunWith(Parameterized.class)
public class Regress181837660 extends SplitterTestBase {
- public static final String EXPECTED = StringUtils.lines("42");
-
@Parameters(name = "{0}")
public static TestParametersCollection params() {
return getTestParameters().withDexRuntimes().withAllApiLevels().build();
@@ -67,10 +66,26 @@
FeatureClass.class,
b -> {},
this::configureNoInlineAnnotations);
- // TODO(b/181571571): This should not succeed as illustrated by non inlining case
- assertEquals(0, processResult.exitCode);
+ // This should not succeed as illustrated by non inlining case
+ assertEquals(1, processResult.exitCode);
// We can't actually read the field since it is in the feature.
- assertFalse(processResult.stderr.contains("NoClassDefFoundError"));
+ assertThat(processResult.stderr, containsString("NoClassDefFoundError"));
+ }
+
+ @Test
+ public void testRegress181571571StillInlineValid() throws Exception {
+ ProcessResult processResult =
+ testR8Splitter(
+ parameters,
+ ImmutableSet.of(Base2Class.class),
+ ImmutableSet.of(Feature2Class.class),
+ Feature2Class.class,
+ r8TestCompileResult ->
+ r8TestCompileResult.inspect(
+ base -> assertFalse(base.clazz(Base2Class.class).isPresent())),
+ this::configureNoInlineAnnotations);
+ assertEquals(0, processResult.exitCode);
+ assertEquals(processResult.stdout, "42\n");
}
private void configure(R8FullTestBuilder testBuilder) throws NoSuchMethodException {
@@ -83,20 +98,52 @@
}
public static class BaseClass {
+
@NeverInline
public static String getFromFeature() {
return FeatureClass.featureString;
}
+
+ @NeverInline
+ public static String getSecondFromFeature() {
+ return FeatureClass.getFeatureString();
+ }
}
public static class FeatureClass implements RunInterface {
public static String featureString = "22";
+ public static String getFeatureString() {
+ return "42";
+ }
+
public static String getAString() {
return BaseClass.getFromFeature();
}
+ public static String getSecondString() {
+ return BaseClass.getSecondFromFeature();
+ }
+
+ @Override
+ public void run() {
+ System.out.println(getAString());
+ System.out.println(getSecondString());
+ }
+ }
+
+ public static class Base2Class {
+ public static String getFromFeature() {
+ return System.currentTimeMillis() > 2 ? "42" : "-19";
+ }
+ }
+
+ public static class Feature2Class implements RunInterface {
+ public static String getAString() {
+ return Base2Class.getFromFeature();
+ }
+
@Override
public void run() {
System.out.println(getAString());
