Improve meet of inlining constraints
Bug: 128967328, 144861881
Change-Id: I95eb51875922abe9efbea4a0481b50855a0c7c8e
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 6a9c3af..d26e953 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
@@ -474,6 +474,9 @@
// Then, PACKAGE is more restrictive constraint.
return one;
}
+ if (appView.isSubtype(one.targetHolder, other.targetHolder).isTrue()) {
+ return new ConstraintWithTarget(Constraint.SAMECLASS, one.targetHolder);
+ }
// 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.
diff --git a/src/test/java/com/android/tools/r8/cf/bootstrap/KotlinCompilerTreeShakingTest.java b/src/test/java/com/android/tools/r8/cf/bootstrap/KotlinCompilerTreeShakingTest.java
index c7a6bc0..1a941d5 100644
--- a/src/test/java/com/android/tools/r8/cf/bootstrap/KotlinCompilerTreeShakingTest.java
+++ b/src/test/java/com/android/tools/r8/cf/bootstrap/KotlinCompilerTreeShakingTest.java
@@ -63,7 +63,6 @@
@Ignore(
"b/136457753: assertion error in static class merger; "
- + "b/144861881: force-inlining of non-inlineable constructors in vertical class merger; "
+ "b/144877828: assertion error in method naming state during interface method renaming; "
+ "b/144859533: umbrella"
)
@@ -101,8 +100,6 @@
o.ignoreMissingClasses = true;
// b/144861100: invoke-static on interface is allowed up to JDK 8.
o.testing.allowInvokeErrors = true;
- // TODO(b/144861881): force-inlining of non-inlineable constructors.
- o.enableVerticalClassMerging = false;
})
.compile()
.writeToZip();
diff --git a/src/test/java/com/android/tools/r8/classmerging/ForceInlineConstructorWithMultiPackageAccessesTest.java b/src/test/java/com/android/tools/r8/classmerging/ForceInlineConstructorWithMultiPackageAccessesTest.java
new file mode 100644
index 0000000..06344db
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/ForceInlineConstructorWithMultiPackageAccessesTest.java
@@ -0,0 +1,75 @@
+// Copyright (c) 2019, 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;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.classmerging.testclasses.ForceInlineConstructorWithMultiPackageAccessesTestClasses;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class ForceInlineConstructorWithMultiPackageAccessesTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public ForceInlineConstructorWithMultiPackageAccessesTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(ForceInlineConstructorWithMultiPackageAccessesTest.class)
+ .addInnerClasses(ForceInlineConstructorWithMultiPackageAccessesTestClasses.class)
+ .addKeepMainRule(TestClass.class)
+ .enableMergeAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ private void inspect(CodeInspector inspector) {
+ assertThat(inspector.clazz(B.class), not(isPresent()));
+ // Check that C is present to ensure that B is not only removed as a result of the entire
+ // object allocation being removed.
+ assertThat(inspector.clazz(C.class), isPresent());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(new C());
+ }
+ }
+
+ static class B extends ForceInlineConstructorWithMultiPackageAccessesTestClasses.A {
+
+ // B.<init>() invokes protected A.<init>() and accesses package-private field `greeting`.
+ String greeting = System.currentTimeMillis() >= 0 ? "Hello world!" : null;
+ }
+
+ static class C extends B {
+
+ @Override
+ public String toString() {
+ return greeting;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/testclasses/ForceInlineConstructorWithMultiPackageAccessesTestClasses.java b/src/test/java/com/android/tools/r8/classmerging/testclasses/ForceInlineConstructorWithMultiPackageAccessesTestClasses.java
new file mode 100644
index 0000000..80ab1d6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/testclasses/ForceInlineConstructorWithMultiPackageAccessesTestClasses.java
@@ -0,0 +1,16 @@
+// Copyright (c) 2019, 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.testclasses;
+
+import com.android.tools.r8.NeverMerge;
+
+public class ForceInlineConstructorWithMultiPackageAccessesTestClasses {
+
+ @NeverMerge
+ public static class A {
+
+ protected A() {}
+ }
+}
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 39872ae..beb56a7 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
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.optimize.inliner;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverClassInline;
@@ -39,9 +40,9 @@
// 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());
+ // 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"), not(isPresent()));
}
static class TestClass {
