Extend test to demonstrate inlining constraint limitations
Bug: 128967328
Change-Id: I747be26af2f1182577ece22359f7025b86a888fc
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 3d3ed81..4955807 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -57,10 +57,10 @@
/**
* Encodes the processing state of a method.
- * <p>
- * We also use this enum to encode under what constraints a method may be inlined.
+ *
+ * <p>We also use this enum to encode under what constraints a method may be inlined.
*/
- // TODO(b/111080693): Need to extend this to a state with the context.
+ // TODO(b/128967328): Need to extend this to a state with the context.
public enum CompilationState {
/**
* Has not been processed, yet.
@@ -100,7 +100,7 @@
public DexAnnotationSet annotations;
public ParameterAnnotationsList parameterAnnotationsList;
private Code code;
- // TODO(b/111080693): towards finer-grained inlining constraints,
+ // TODO(b/128967328): towards finer-grained inlining constraints,
// we need to maintain a set of states with (potentially different) contexts.
private CompilationState compilationState = CompilationState.NOT_PROCESSED;
private OptimizationInfo optimizationInfo = DefaultOptimizationInfoImpl.DEFAULT_INSTANCE;
@@ -275,7 +275,7 @@
}
return true;
}
- // TODO(b/111080693): inlining candidate should satisfy all states if multiple states are there.
+ // TODO(b/128967328): inlining candidate should satisfy all states if multiple states are there.
switch (compilationState) {
case PROCESSED_INLINING_CANDIDATE_ANY:
return true;
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 bf92dfa..9b5061f 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
@@ -100,7 +100,7 @@
result = state;
break;
}
- // TODO(b/111080693): we may need to collect all meaningful constraints.
+ // TODO(b/128967328): we may need to collect all meaningful constraints.
result = ConstraintWithTarget.meet(result, state, appView.appInfo());
}
return result;
@@ -366,7 +366,7 @@
// Then, PACKAGE is more restrictive constraint.
return one;
}
- // TODO(b/111080693): towards finer-grained constraints, we need both.
+ // TODO(b/128967328): towards finer-grained constraints, we need both.
// The target method is still inlineable to methods with a holder from the same package of
// one's holder and a subtype of other's holder.
return NEVER;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineNonReboundFieldTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineNonReboundFieldTest.java
index fffb84a..9da44b6 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineNonReboundFieldTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineNonReboundFieldTest.java
@@ -4,10 +4,15 @@
package com.android.tools.r8.ir.optimize.inliner;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+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.testclasses.Greeting;
import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
import org.junit.Test;
/** Regression test for b/128604123. */
@@ -16,14 +21,27 @@
@Test
public void test() throws Exception {
String expectedOutput = StringUtils.lines("Greeter: Hello world!");
- testForR8(Backend.DEX)
- .addProgramClasses(
- TestClass.class, Greeter.class, Greeting.class, Greeting.getGreetingBase())
- .addKeepMainRule(TestClass.class)
- .enableClassInliningAnnotations()
- .enableMergeAnnotations()
- .run(TestClass.class)
- .assertSuccessWithOutput(expectedOutput);
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addProgramClasses(
+ TestClass.class, Greeter.class, Greeting.class, Greeting.getGreetingBase())
+ .addKeepMainRule(TestClass.class)
+ .enableClassInliningAnnotations()
+ .enableMergeAnnotations()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput)
+ .inspector();
+
+ ClassSubject greeterSubject = inspector.clazz(Greeter.class);
+ assertThat(greeterSubject, isPresent());
+
+ // Verify that greet() is not inlined into main() -- that would lead to illegal access errors
+ // since main() does not have access to the GreetingBase.greeting field.
+ assertThat(greeterSubject.uniqueMethodWithName("greet"), isPresent());
+
+ // TODO(b/128967328): The method greetInternal() should be inlined into greet() since it has a
+ // single call site and nothing prevents it from being inlined.
+ assertThat(greeterSubject.uniqueMethodWithName("greetInternal"), isPresent());
}
static class TestClass {
@@ -43,6 +61,10 @@
}
void greet() {
+ greetInternal();
+ }
+
+ private void greetInternal() {
System.out.println(TAG + ": " + greeting);
}
}
