Make isInterface a conservative method on app-view.
This CL also ensures that we never put null in interfaces.
Bug: 130296309
Change-Id: If5b73f85f87ab1143e8f52c344a072c349fe95b3
diff --git a/src/main/java/com/android/tools/r8/OptionalBool.java b/src/main/java/com/android/tools/r8/OptionalBool.java
index db49872..92f5d07 100644
--- a/src/main/java/com/android/tools/r8/OptionalBool.java
+++ b/src/main/java/com/android/tools/r8/OptionalBool.java
@@ -12,6 +12,11 @@
public boolean isTrue() {
return true;
}
+
+ @Override
+ public String toString() {
+ return "true";
+ }
};
private static final OptionalBool FALSE =
@@ -20,6 +25,11 @@
public boolean isFalse() {
return true;
}
+
+ @Override
+ public String toString() {
+ return "false";
+ }
};
private static final OptionalBool UNKNOWN =
@@ -28,6 +38,11 @@
public boolean isUnknown() {
return true;
}
+
+ @Override
+ public String toString() {
+ return "unknown";
+ }
};
public static OptionalBool of(boolean bool) {
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
index 35df2ef..0c8196d 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
@@ -466,7 +466,7 @@
return getTypeInfo(type).isUnknown();
}
- public boolean isInterface(DexType type) {
+ public boolean isMarkedAsInterface(DexType type) {
return getTypeInfo(type).isInterface();
}
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 5798d1a..95e8ab3 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -87,6 +87,21 @@
return appInfo().definitionFor(type);
}
+ public OptionalBool isInterface(DexType type) {
+ // Without whole program information we should not assume anything about any other class than
+ // the current holder in a given context.
+ if (enableWholeProgramOptimizations()) {
+ assert appInfo().hasSubtyping();
+ if (appInfo().hasSubtyping()) {
+ AppInfoWithSubtyping appInfo = appInfo().withSubtyping();
+ return appInfo.isUnknown(type)
+ ? OptionalBool.unknown()
+ : OptionalBool.of(appInfo.isMarkedAsInterface(type));
+ }
+ }
+ return OptionalBool.unknown();
+ }
+
@Override
public DexItemFactory dexItemFactory() {
return dexItemFactory;
@@ -132,6 +147,13 @@
this.verticallyMergedClasses = verticallyMergedClasses;
}
+ @SuppressWarnings("unchecked")
+ public AppView<AppInfoWithSubtyping> withSubtyping() {
+ return appInfo.hasSubtyping()
+ ? (AppView<AppInfoWithSubtyping>) this
+ : null;
+ }
+
public AppView<AppInfoWithLiveness> withLiveness() {
@SuppressWarnings("unchecked")
AppView<AppInfoWithLiveness> appViewWithLiveness = (AppView<AppInfoWithLiveness>) this;
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 2e85ad1..7c3a73f 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -36,6 +36,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.List;
@@ -1214,23 +1215,21 @@
type,
t -> {
if (type.isClassType()) {
- if (!appView.appInfo().hasSubtyping()) {
- return ClassTypeLatticeElement.create(type, nullability, (Set<DexType>) null);
+ if (!appView.enableWholeProgramOptimizations()) {
+ // Don't reason at the level of interfaces in D8.
+ return ClassTypeLatticeElement.create(type, nullability, Collections.emptySet());
}
- AppView<? extends AppInfoWithSubtyping> appViewWithSubtyping =
- appView.withLiveness();
- // TODO(zerny): It should never be the case that we have unknown at this point!
- if (!appViewWithSubtyping.appInfo().isUnknown(type)
- && appViewWithSubtyping.appInfo().isInterface(type)) {
+ assert appView.appInfo().hasSubtyping();
+ if (appView.isInterface(type).isTrue()) {
return ClassTypeLatticeElement.create(
- objectType, nullability, ImmutableSet.of(type));
+ objectType, nullability, Collections.singleton(type));
}
// In theory, `interfaces` is the least upper bound of implemented interfaces.
// It is expensive to walk through type hierarchy; collect implemented interfaces;
// and compute the least upper bound of two interface sets. Hence, lazy
// computations. Most likely during lattice join. See {@link
// ClassTypeLatticeElement#getInterfaces}.
- return ClassTypeLatticeElement.create(type, nullability, appViewWithSubtyping);
+ return ClassTypeLatticeElement.create(type, nullability, appView.withSubtyping());
}
assert type.isArrayType();
return ArrayTypeLatticeElement.create(finalMemberType, nullability);
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/type/ClassTypeLatticeElement.java b/src/main/java/com/android/tools/r8/ir/analysis/type/ClassTypeLatticeElement.java
index 298251d..84b79c2 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/type/ClassTypeLatticeElement.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/type/ClassTypeLatticeElement.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.graph.DexType;
import com.google.common.collect.ImmutableSet;
import java.util.ArrayDeque;
+import java.util.Collections;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Map;
@@ -28,6 +29,7 @@
public static ClassTypeLatticeElement create(
DexType classType, Nullability nullability, Set<DexType> interfaces) {
+ assert interfaces != null;
return NullabilityVariants.create(
nullability,
(variants) ->
@@ -36,6 +38,7 @@
public static ClassTypeLatticeElement create(
DexType classType, Nullability nullability, AppView<? extends AppInfoWithSubtyping> appView) {
+ assert appView != null;
return NullabilityVariants.create(
nullability,
(variants) -> new ClassTypeLatticeElement(classType, nullability, null, variants, appView));
@@ -133,14 +136,14 @@
ClassTypeLatticeElement join(ClassTypeLatticeElement other, AppView<?> appView) {
Nullability nullability = nullability().join(other.nullability());
if (!appView.appInfo().hasSubtyping()) {
- assert getInterfaces() == null;
- assert other.getInterfaces() == null;
+ assert getInterfaces().isEmpty();
+ assert other.getInterfaces().isEmpty();
return ClassTypeLatticeElement.create(
getClassType() == other.getClassType()
? getClassType()
: appView.dexItemFactory().objectType,
nullability,
- (Set<DexType>) null);
+ Collections.emptySet());
}
DexType lubType =
appView
@@ -176,6 +179,9 @@
static Set<DexType> computeLeastUpperBoundOfInterfaces(
AppView<? extends AppInfoWithSubtyping> appView, Set<DexType> s1, Set<DexType> s2) {
+ if (s1.isEmpty() || s2.isEmpty()) {
+ return Collections.emptySet();
+ }
Set<DexType> cached = appView.dexItemFactory().leastUpperBoundOfInterfacesTable.get(s1, s2);
if (cached != null) {
return cached;
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 13d9035..5f340d7 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -598,7 +598,7 @@
}
private boolean canInvokeTargetWithInvokeVirtual(DexEncodedMethod target) {
- return target.isVirtualMethod() && !appView.appInfo().isInterface(target.method.holder);
+ return target.isVirtualMethod() && appView.isInterface(target.method.holder).isFalse();
}
private boolean hasAccessToInvokeTargetFromContext(DexEncodedMethod target, DexType context) {
diff --git a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
index 54ab3eb..d26a3d9 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
@@ -208,7 +208,6 @@
private void reserveNamesInClasses(
DexType type, DexType libraryFrontier, NamingState<DexProto, ?> parent) {
- assert !appView.appInfo().isInterface(type);
NamingState<DexProto, ?> state =
frontierState.allocateNamingStateAndReserve(type, libraryFrontier, parent);
@@ -216,7 +215,6 @@
// frontier forward.
DexClass holder = appView.definitionFor(type);
for (DexType subtype : appView.appInfo().allExtendsSubtypes(type)) {
- assert !appView.appInfo().isInterface(subtype);
reserveNamesInClasses(
subtype, holder == null || holder.isNotProgramClass() ? subtype : libraryFrontier, state);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 98cd095..71c5bcd 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1428,7 +1428,7 @@
}
private void fillWorkList(Deque<DexType> worklist, DexType type) {
- if (appInfo.isInterface(type)) {
+ if (appInfo.isMarkedAsInterface(type)) {
// We need to check if the method is shadowed by a class that directly implements
// the interface and go recursively down to the sub interfaces to reach class
// implementing the interface
diff --git a/src/test/java/com/android/tools/r8/internal/R8GMSCoreLookupTest.java b/src/test/java/com/android/tools/r8/internal/R8GMSCoreLookupTest.java
index c027f4a..d4cec12 100644
--- a/src/test/java/com/android/tools/r8/internal/R8GMSCoreLookupTest.java
+++ b/src/test/java/com/android/tools/r8/internal/R8GMSCoreLookupTest.java
@@ -66,7 +66,8 @@
private void testInterfaceLookup(DexProgramClass clazz, DexEncodedMethod method) {
Set<DexEncodedMethod> targets = appInfo.lookupInterfaceTargets(method.method);
- if (appInfo.subtypes(method.method.holder).stream().allMatch(appInfo::isInterface)) {
+ if (appInfo.subtypes(method.method.holder).stream()
+ .allMatch(t -> appInfo.definitionFor(t).isInterface())) {
assertTrue(targets.isEmpty());
} else {
assertFalse(targets.isEmpty());
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/type/TypeAnalysisTest.java b/src/test/java/com/android/tools/r8/ir/analysis/type/TypeAnalysisTest.java
index 5ab424a..d604130 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/type/TypeAnalysisTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/type/TypeAnalysisTest.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -120,7 +121,7 @@
new ApplicationReader(app, TEST_OPTIONS, new Timing("TypeAnalysisTest.appReader"))
.read().toDirect();
inspection.accept(
- AppView.createForR8(new AppInfo(dexApplication), TEST_OPTIONS),
+ AppView.createForR8(new AppInfoWithSubtyping(dexApplication), TEST_OPTIONS),
new CodeInspector(dexApplication));
}
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/type/TypeLatticeTest.java b/src/test/java/com/android/tools/r8/ir/analysis/type/TypeLatticeTest.java
index cf21b50..d63a3d2 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/type/TypeLatticeTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/type/TypeLatticeTest.java
@@ -508,6 +508,7 @@
@Test
public void testSelfOrderWithoutSubtypingInfo() {
DexType type = factory.createType("Lmy/Type;");
+ appView.withSubtyping().appInfo().registerNewType(type, factory.objectType);
TypeLatticeElement nonNullType = fromDexType(type, Nullability.definitelyNotNull(), appView);
ReferenceTypeLatticeElement nullableType =
nonNullType.asReferenceTypeLatticeElement().getOrCreateVariant(Nullability.maybeNull());
