Use a seen set when computing if class initialization has side effects.
Bug: 147865209
Change-Id: I4cc6ddd85856b9068404147ec41fbb8ee7c765bd
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 6dac57d..3601754 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -774,11 +774,15 @@
}
public boolean classInitializationMayHaveSideEffects(AppView<?> appView) {
- return classInitializationMayHaveSideEffects(appView, Predicates.alwaysFalse());
+ return classInitializationMayHaveSideEffects(
+ appView, Predicates.alwaysFalse(), Sets.newIdentityHashSet());
}
public boolean classInitializationMayHaveSideEffects(
- AppView<?> appView, Predicate<DexType> ignore) {
+ AppView<?> appView, Predicate<DexType> ignore, Set<DexType> seen) {
+ if (!seen.add(type)) {
+ return false;
+ }
if (ignore.test(type)) {
return false;
}
@@ -796,7 +800,7 @@
if (defaultValuesForStaticFieldsMayTriggerAllocation()) {
return true;
}
- return initializationOfParentTypesMayHaveSideEffects(appView, ignore);
+ return initializationOfParentTypesMayHaveSideEffects(appView, ignore, seen);
}
private boolean hasClassInitializerThatCannotBePostponed() {
@@ -821,17 +825,19 @@
}
public boolean initializationOfParentTypesMayHaveSideEffects(AppView<?> appView) {
- return initializationOfParentTypesMayHaveSideEffects(appView, Predicates.alwaysFalse());
+ return initializationOfParentTypesMayHaveSideEffects(
+ appView, Predicates.alwaysFalse(), Sets.newIdentityHashSet());
}
public boolean initializationOfParentTypesMayHaveSideEffects(
- AppView<?> appView, Predicate<DexType> ignore) {
+ AppView<?> appView, Predicate<DexType> ignore, Set<DexType> seen) {
for (DexType iface : interfaces.values) {
- if (iface.classInitializationMayHaveSideEffects(appView, ignore)) {
+ if (iface.classInitializationMayHaveSideEffects(appView, ignore, seen)) {
return true;
}
}
- if (superType != null && superType.classInitializationMayHaveSideEffects(appView, ignore)) {
+ if (superType != null
+ && superType.classInitializationMayHaveSideEffects(appView, ignore, seen)) {
return true;
}
return false;
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
index d807874..ce89a41 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.ir.optimize.info.MutableFieldOptimizationInfo;
import com.android.tools.r8.kotlin.KotlinMemberInfo;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.google.common.collect.Sets;
public class DexEncodedField extends DexEncodedMember<DexEncodedField, DexField> {
public static final DexEncodedField[] EMPTY_ARRAY = {};
@@ -217,7 +218,8 @@
appView,
// Types that are a super type of the current context are guaranteed to be initialized
// already.
- type -> appView.isSubtype(context, type).isTrue())) {
+ type -> appView.isSubtype(context, type).isTrue(),
+ Sets.newIdentityHashSet())) {
// Ignore class initialization side-effects for dead proto extension fields to ensure that
// we force replace these field reads by null.
boolean ignore =
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index 9a8a73c..08d4d61 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -25,9 +25,11 @@
import com.android.tools.r8.utils.Pair;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.function.Predicate;
public class DexType extends DexReference implements PresortedComparable<DexType> {
@@ -60,23 +62,26 @@
}
public boolean classInitializationMayHaveSideEffects(AppView<?> appView) {
- return classInitializationMayHaveSideEffects(appView, Predicates.alwaysFalse());
+ return classInitializationMayHaveSideEffects(
+ appView, Predicates.alwaysFalse(), Sets.newIdentityHashSet());
}
public boolean classInitializationMayHaveSideEffects(
- AppView<?> appView, Predicate<DexType> ignore) {
+ AppView<?> appView, Predicate<DexType> ignore, Set<DexType> seen) {
DexClass clazz = appView.definitionFor(this);
- return clazz == null || clazz.classInitializationMayHaveSideEffects(appView, ignore);
+ return clazz == null || clazz.classInitializationMayHaveSideEffects(appView, ignore, seen);
}
public boolean initializationOfParentTypesMayHaveSideEffects(AppView<?> appView) {
- return initializationOfParentTypesMayHaveSideEffects(appView, Predicates.alwaysFalse());
+ return initializationOfParentTypesMayHaveSideEffects(
+ appView, Predicates.alwaysFalse(), Sets.newIdentityHashSet());
}
public boolean initializationOfParentTypesMayHaveSideEffects(
- AppView<?> appView, Predicate<DexType> ignore) {
+ AppView<?> appView, Predicate<DexType> ignore, Set<DexType> seen) {
DexClass clazz = appView.definitionFor(this);
- return clazz == null || clazz.initializationOfParentTypesMayHaveSideEffects(appView, ignore);
+ return clazz == null
+ || clazz.initializationOfParentTypesMayHaveSideEffects(appView, ignore, seen);
}
public boolean isAlwaysNull(AppView<AppInfoWithLiveness> appView) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
index 39b6fb8..c8d6abc 100644
--- a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.UnknownValue;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.google.common.collect.Sets;
import java.util.Collections;
import java.util.List;
@@ -128,7 +129,8 @@
if (field.holder.classInitializationMayHaveSideEffects(
appView,
// Types that are a super type of `context` are guaranteed to be initialized already.
- type -> appView.isSubtype(context, type).isTrue())) {
+ type -> appView.isSubtype(context, type).isTrue(),
+ Sets.newIdentityHashSet())) {
return AbstractError.top();
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
index 6ba4a3a..3a7969a 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
@@ -24,6 +24,7 @@
import com.android.tools.r8.ir.optimize.InliningConstraints;
import com.android.tools.r8.ir.optimize.inliner.WhyAreYouNotInliningReporter;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.google.common.collect.Sets;
import java.util.List;
import java.util.function.Predicate;
@@ -197,7 +198,8 @@
appView,
// Types that are a super type of `context` are guaranteed to be initialized
// already.
- type -> appView.isSubtype(context, type).isTrue());
+ type -> appView.isSubtype(context, type).isTrue(),
+ Sets.newIdentityHashSet());
}
return targetMayHaveSideEffects;
diff --git a/src/main/java/com/android/tools/r8/ir/code/NewInstance.java b/src/main/java/com/android/tools/r8/ir/code/NewInstance.java
index aed4b9e..cd88da6 100644
--- a/src/main/java/com/android/tools/r8/ir/code/NewInstance.java
+++ b/src/main/java/com/android/tools/r8/ir/code/NewInstance.java
@@ -24,6 +24,7 @@
import com.android.tools.r8.ir.conversion.DexBuilder;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.InliningConstraints;
+import com.google.common.collect.Sets;
public class NewInstance extends Instruction {
@@ -188,7 +189,8 @@
if (definition.classInitializationMayHaveSideEffects(
appView,
// Types that are a super type of `context` are guaranteed to be initialized already.
- type -> appView.isSubtype(context, type).isTrue())) {
+ type -> appView.isSubtype(context, type).isTrue(),
+ Sets.newIdentityHashSet())) {
return true;
}
@@ -228,7 +230,8 @@
return clazz.classInitializationMayHaveSideEffects(
appView,
// Types that are a super type of `context` are guaranteed to be initialized already.
- type -> appView.isSubtype(context, type).isTrue());
+ type -> appView.isSubtype(context, type).isTrue(),
+ Sets.newIdentityHashSet());
} else {
// In D8, this instruction may trigger class initialization if the holder of the field is
// different from the current context.
diff --git a/src/main/java/com/android/tools/r8/ir/code/StaticGet.java b/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
index f59eb4b..08285c5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
+++ b/src/main/java/com/android/tools/r8/ir/code/StaticGet.java
@@ -27,6 +27,7 @@
import com.android.tools.r8.ir.conversion.DexBuilder;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.InliningConstraints;
+import com.google.common.collect.Sets;
import java.util.Set;
public class StaticGet extends FieldInstruction {
@@ -236,7 +237,8 @@
return holder.classInitializationMayHaveSideEffects(
appView,
// Types that are a super type of `context` are guaranteed to be initialized already.
- type -> appView.isSubtype(context, type).isTrue());
+ type -> appView.isSubtype(context, type).isTrue(),
+ Sets.newIdentityHashSet());
} else {
// In D8, this instruction may trigger class initialization if the holder of the field is
// different from the current context.
diff --git a/src/main/java/com/android/tools/r8/ir/code/StaticPut.java b/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
index 63e5c10..c9bab8b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
+++ b/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
@@ -28,6 +28,7 @@
import com.android.tools.r8.ir.regalloc.RegisterAllocator;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.ProguardMemberRule;
+import com.google.common.collect.Sets;
public class StaticPut extends FieldInstruction {
@@ -235,7 +236,8 @@
return holder.classInitializationMayHaveSideEffects(
appView,
// Types that are a super type of `context` are guaranteed to be initialized already.
- type -> appView.isSubtype(context, type).isTrue());
+ type -> appView.isSubtype(context, type).isTrue(),
+ Sets.newIdentityHashSet());
} else {
// In D8, this instruction may trigger class initialization if the holder of the field is
// different from the current context.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
index 5577a8c..f362ef7 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
@@ -186,7 +186,8 @@
if (newInstance.clazz.classInitializationMayHaveSideEffects(
appView,
// Types that are a super type of `context` are guaranteed to be initialized already.
- type -> appView.isSubtype(context, type).isTrue())) {
+ type -> appView.isSubtype(context, type).isTrue(),
+ Sets.newIdentityHashSet())) {
killAllActiveFields();
}
} else {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 9231d05..f076b41 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -160,7 +160,8 @@
if (eligibleClass.classInitializationMayHaveSideEffects(
appView,
// Types that are a super type of the current context are guaranteed to be initialized.
- type -> appView.isSubtype(method.method.holder, type).isTrue())) {
+ type -> appView.isSubtype(method.method.holder, type).isTrue(),
+ Sets.newIdentityHashSet())) {
return EligibilityStatus.HAS_CLINIT;
}
return EligibilityStatus.ELIGIBLE;
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 783862a..8629473 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -420,7 +420,8 @@
// We basically can't move the clinit, since it is not called when implementing classes have
// their clinit called - except when the interface has a default method.
if ((clazz.hasClassInitializer() && targetClass.hasClassInitializer())
- || targetClass.classInitializationMayHaveSideEffects(appView, type -> type == clazz.type)
+ || targetClass.classInitializationMayHaveSideEffects(
+ appView, type -> type == clazz.type, Sets.newIdentityHashSet())
|| (clazz.isInterface() && clazz.classInitializationMayHaveSideEffects(appView))) {
// TODO(herhut): Handle class initializers.
if (Log.ENABLED) {