Merge "Update RootSetBuilder.verifyKeptItemsAreKept()"
diff --git a/build.gradle b/build.gradle
index 5631d94..9e84b01 100644
--- a/build.gradle
+++ b/build.gradle
@@ -276,6 +276,8 @@
"android_jar/lib-v24",
"android_jar/lib-v25",
"android_jar/lib-v26",
+ "android_jar/lib-v27",
+ "android_jar/lib-v28",
"core-lambda-stubs",
"dart-sdk",
"ddmlib",
diff --git a/src/main/java/com/android/tools/r8/GenerateMainDexList.java b/src/main/java/com/android/tools/r8/GenerateMainDexList.java
index 48c67fd..7b15c6e 100644
--- a/src/main/java/com/android/tools/r8/GenerateMainDexList.java
+++ b/src/main/java/com/android/tools/r8/GenerateMainDexList.java
@@ -44,7 +44,8 @@
DexApplication application =
new ApplicationReader(app, options, timing).read(executor).toDirect();
AppView<? extends AppInfoWithSubtyping> appView =
- new AppView<>(new AppInfoWithSubtyping(application), GraphLense.getIdentityLense());
+ new AppView<>(
+ new AppInfoWithSubtyping(application), GraphLense.getIdentityLense(), options);
RootSet mainDexRootSet =
new RootSetBuilder(appView, application, options.mainDexKeepRules, options).run(executor);
diff --git a/src/main/java/com/android/tools/r8/PrintSeeds.java b/src/main/java/com/android/tools/r8/PrintSeeds.java
index 7bef553..475ca70 100644
--- a/src/main/java/com/android/tools/r8/PrintSeeds.java
+++ b/src/main/java/com/android/tools/r8/PrintSeeds.java
@@ -84,7 +84,8 @@
DexApplication application =
new ApplicationReader(command.getInputApp(), options, timing).read(executor).toDirect();
AppView<? extends AppInfoWithSubtyping> appView =
- new AppView<>(new AppInfoWithSubtyping(application), GraphLense.getIdentityLense());
+ new AppView<>(
+ new AppInfoWithSubtyping(application), GraphLense.getIdentityLense(), options);
RootSet rootSet =
new RootSetBuilder(
appView, application, options.getProguardConfiguration().getRules(), options)
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index fd16278..70c0f40 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -250,7 +250,8 @@
inputApp.closeInternalArchiveProviders();
AppView<AppInfoWithSubtyping> appView =
- new AppView<>(new AppInfoWithSubtyping(application), GraphLense.getIdentityLense());
+ new AppView<>(
+ new AppInfoWithSubtyping(application), GraphLense.getIdentityLense(), options);
RootSet rootSet;
String proguardSeedsData = null;
timing.begin("Strip unused code");
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index 2354918..f9a9dde 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -6,18 +6,21 @@
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.shaking.VerticalClassMerger.VerticallyMergedClasses;
+import com.android.tools.r8.utils.InternalOptions;
public class AppView<T extends AppInfo> {
private T appInfo;
private final DexItemFactory dexItemFactory;
private GraphLense graphLense;
+ private final InternalOptions options;
private VerticallyMergedClasses verticallyMergedClasses;
- public AppView(T appInfo, GraphLense graphLense) {
+ public AppView(T appInfo, GraphLense graphLense, InternalOptions options) {
this.appInfo = appInfo;
this.dexItemFactory = appInfo != null ? appInfo.dexItemFactory : null;
this.graphLense = graphLense;
+ this.options = options;
}
public T appInfo() {
@@ -46,6 +49,10 @@
this.graphLense = graphLense;
}
+ public InternalOptions options() {
+ return options;
+ }
+
// Get the result of vertical class merging. Returns null if vertical class merging has not been
// run.
public VerticallyMergedClasses verticallyMergedClasses() {
@@ -63,7 +70,7 @@
private class AppViewWithLiveness extends AppView<AppInfoWithLiveness> {
private AppViewWithLiveness() {
- super(null, null);
+ super(null, null, null);
}
@Override
@@ -94,6 +101,11 @@
}
@Override
+ public InternalOptions options() {
+ return AppView.this.options();
+ }
+
+ @Override
public AppView<AppInfoWithLiveness> withLiveness() {
return this;
}
diff --git a/src/main/java/com/android/tools/r8/graph/JarCode.java b/src/main/java/com/android/tools/r8/graph/JarCode.java
index 9d25704..04e6516 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.shaking.ProguardKeepAttributes;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.OffOrAuto;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.Iterator;
@@ -226,12 +227,23 @@
public ConstraintWithTarget computeInliningConstraint(
DexEncodedMethod encodedMethod,
- AppInfoWithLiveness appInfo,
+ AppView<? extends AppInfoWithLiveness> appView,
GraphLense graphLense,
DexType invocationContext) {
InliningConstraintVisitor visitor =
new InliningConstraintVisitor(
- application, appInfo, graphLense, encodedMethod, invocationContext);
+ application, appView.appInfo(), graphLense, encodedMethod, invocationContext);
+
+ if (appView.options().enableDesugaring
+ && appView.options().interfaceMethodDesugaring == OffOrAuto.Auto
+ && !appView.options().canUseDefaultAndStaticInterfaceMethods()) {
+ // TODO(b/120130831): Conservatively need to say "no" at this point if there are invocations
+ // to static interface methods. This should be fixed by making sure that the desugared
+ // versions of default and static interface methods are present in the application during
+ // IR processing.
+ visitor.disallowStaticInterfaceMethodCalls();
+ }
+
AbstractInsnNode insn = node.instructions.getFirst();
while (insn != null) {
insn.accept(visitor);
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 4e3f9be..3292704 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
@@ -24,6 +24,8 @@
private AppInfoWithLiveness appInfo;
+ private boolean allowStaticInterfaceMethodCalls = true;
+
// Currently used only by the vertical class merger (in all other cases this is the identity).
//
// When merging a type A into its subtype B we need to inline A.<init>() into B.<init>().
@@ -46,6 +48,10 @@
this.graphLense = graphLense;
}
+ public void disallowStaticInterfaceMethodCalls() {
+ allowStaticInterfaceMethodCalls = false;
+ }
+
public ConstraintWithTarget forAlwaysMaterializingUser() {
return ConstraintWithTarget.ALWAYS;
}
@@ -280,6 +286,11 @@
DexType methodHolder = graphLense.lookupType(target.method.holder);
DexClass methodClass = appInfo.definitionFor(methodHolder);
if (methodClass != null) {
+ if (!allowStaticInterfaceMethodCalls && methodClass.isInterface() && target.hasCode()) {
+ // See b/120121170.
+ return ConstraintWithTarget.NEVER;
+ }
+
ConstraintWithTarget methodConstraintWithTarget =
ConstraintWithTarget.deriveConstraint(
invocationContext, methodHolder, target.accessFlags, appInfo);
diff --git a/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java b/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
index c707968..18998c8 100644
--- a/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
+++ b/src/main/java/com/android/tools/r8/jar/InliningConstraintVisitor.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.graph.GraphLense.GraphLenseLookupResult;
import com.android.tools.r8.graph.JarApplicationReader;
import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.conversion.JarSourceCode;
@@ -62,6 +63,10 @@
? inliningConstraints.forMonitor() : ConstraintWithTarget.ALWAYS;
}
+ public void disallowStaticInterfaceMethodCalls() {
+ inliningConstraints.disallowStaticInterfaceMethodCalls();
+ }
+
public ConstraintWithTarget getConstraint() {
return constraint;
}
@@ -160,12 +165,15 @@
}
break;
- case Opcodes.INVOKESTATIC:
- type = Invoke.Type.STATIC;
- assert noNeedToUseGraphLense(target, type);
+ case Opcodes.INVOKESTATIC: {
+ // Static invokes may have changed as a result of horizontal class merging.
+ GraphLenseLookupResult lookup = graphLense.lookupMethod(target, null, Invoke.Type.STATIC);
+ target = lookup.getMethod();
+ type = lookup.getType();
break;
+ }
- case Opcodes.INVOKEVIRTUAL:
+ case Opcodes.INVOKEVIRTUAL: {
type = Invoke.Type.VIRTUAL;
// Instructions that target a private method in the same class translates to invoke-direct.
if (target.holder == method.method.holder) {
@@ -174,8 +182,13 @@
type = Invoke.Type.DIRECT;
}
}
- assert noNeedToUseGraphLense(target, type);
+
+ // Virtual invokes may have changed to interface invokes as a result of member rebinding.
+ GraphLenseLookupResult lookup = graphLense.lookupMethod(target, null, type);
+ target = lookup.getMethod();
+ type = lookup.getType();
break;
+ }
default:
throw new Unreachable("Unexpected opcode " + opcode);
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 4abd3e3..0d69178 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -1619,7 +1619,7 @@
ConstraintWithTarget constraint =
jarCode.computeInliningConstraint(
method,
- appInfo,
+ appView,
new SingleTypeMapperGraphLense(method.method.holder, invocationContext),
invocationContext);
return constraint == ConstraintWithTarget.NEVER;
diff --git a/src/test/java/com/android/tools/r8/classmerging/ForceInliningWithStaticInterfaceMethodTest.java b/src/test/java/com/android/tools/r8/classmerging/ForceInliningWithStaticInterfaceMethodTest.java
new file mode 100644
index 0000000..7c86d1a
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/ForceInliningWithStaticInterfaceMethodTest.java
@@ -0,0 +1,62 @@
+// Copyright (c) 2018, 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 com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+
+/** Regression test for b/120121170. */
+public class ForceInliningWithStaticInterfaceMethodTest extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput = StringUtils.lines("A.<init>()", "I.m()", "B.<init>()");
+ testForR8(Backend.DEX)
+ .addInnerClasses(ForceInliningWithStaticInterfaceMethodTest.class)
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(AndroidApiLevel.M)
+ .compile()
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ new B();
+ }
+ }
+
+ static class A {
+
+ public A() {
+ System.out.println("A.<init>()");
+
+ // By the time the vertical class merger runs, I.m() still exists, so the inlining oracle
+ // concludes that A.<init>() is eligible for inlining. However, by the time A.<init>() will
+ // be force-inlined into B.<init>(), I.m() has been rewritten as a result of interface method
+ // desugaring, but the target method is not yet added to the application. Hence the inlining
+ // oracle concludes that A.<init>() is not eligible for inlining, which leads to an error
+ // "FORCE inlining on non-inlinable".
+ I.m();
+ }
+ }
+
+ static class B extends A {
+
+ public B() {
+ System.out.println("B.<init>()");
+ }
+ }
+
+ interface I {
+
+ static void m() {
+ System.out.println("I.m()");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTestBase.java b/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTestBase.java
index 855e2ad..58c4237 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTestBase.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTestBase.java
@@ -32,7 +32,8 @@
DexApplication dexApplication =
new ApplicationReader(app, TEST_OPTIONS, timing).read().toDirect();
AppView<AppInfoWithSubtyping> appView =
- new AppView<>(new AppInfoWithSubtyping(dexApplication), GraphLense.getIdentityLense());
+ new AppView<>(
+ new AppInfoWithSubtyping(dexApplication), GraphLense.getIdentityLense(), TEST_OPTIONS);
ExecutorService executorService = ThreadUtils.getExecutorService(TEST_OPTIONS);
RootSet rootSet =
new RootSetBuilder(
diff --git a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
index 301247d..4f4639a 100644
--- a/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
+++ b/src/test/java/com/android/tools/r8/naming/NamingTestBase.java
@@ -44,7 +44,6 @@
private DexApplication program;
DexItemFactory dexItemFactory;
- private AppView<AppInfoWithSubtyping> appView;
NamingTestBase(
String test,
@@ -61,7 +60,6 @@
public void readApp() throws IOException, ExecutionException {
program = ToolHelper.buildApplication(ImmutableList.of(appFileName));
dexItemFactory = program.dexItemFactory;
- appView = new AppView<>(new AppInfoWithSubtyping(program), GraphLense.getIdentityLense());
}
NamingLens runMinifier(List<Path> configPaths) throws ExecutionException {
@@ -71,6 +69,8 @@
ExecutorService executor = ThreadUtils.getExecutorService(1);
+ AppView<AppInfoWithSubtyping> appView =
+ new AppView<>(new AppInfoWithSubtyping(program), GraphLense.getIdentityLense(), options);
RootSet rootSet =
new RootSetBuilder(appView, program, configuration.getRules(), options).run(executor);
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
index a84f585..7ec1613 100644
--- a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
@@ -105,7 +105,8 @@
AndroidApp app = readClassesAndAsmDump(CLASSES, ASM_CLASSES);
DexApplication application = new ApplicationReader(app, options, timing).read().toDirect();
AppView<? extends AppInfoWithSubtyping> appView =
- new AppView<>(new AppInfoWithSubtyping(application), GraphLense.getIdentityLense());
+ new AppView<>(
+ new AppInfoWithSubtyping(application), GraphLense.getIdentityLense(), options);
ExecutorService executor = Executors.newSingleThreadExecutor();
RootSet rootSet =
