Version 1.5.71 Cherry-pick: Use a seen set when computing if class initialization has side effects. CL: https://r8-review.googlesource.com/c/r8/+/48860 Bug: 147865209 Change-Id: I8edf12a2d17d0b147605bfd2306b76a4d97abe05
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java index a65d514..04f5f2f 100644 --- a/src/main/java/com/android/tools/r8/Version.java +++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@ // This field is accessed from release scripts using simple pattern matching. // Therefore, changing this field could break our release scripts. - public static final String LABEL = "1.5.70"; + public static final String LABEL = "1.5.71"; private Version() { }
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 3429bb3..2083ed0 100644 --- a/src/main/java/com/android/tools/r8/graph/DexClass.java +++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -745,11 +745,15 @@ } public boolean classInitializationMayHaveSideEffects(DexDefinitionSupplier definitions) { - return classInitializationMayHaveSideEffects(definitions, Predicates.alwaysFalse()); + return classInitializationMayHaveSideEffects( + definitions, Predicates.alwaysFalse(), Sets.newIdentityHashSet()); } public boolean classInitializationMayHaveSideEffects( - DexDefinitionSupplier definitions, Predicate<DexType> ignore) { + DexDefinitionSupplier definitions, Predicate<DexType> ignore, Set<DexType> seen) { + if (!seen.add(type)) { + return false; + } if (ignore.test(type) || definitions.dexItemFactory().libraryTypesWithoutStaticInitialization.contains(type)) { return false; @@ -760,7 +764,7 @@ if (defaultValuesForStaticFieldsMayTriggerAllocation()) { return true; } - return initializationOfParentTypesMayHaveSideEffects(definitions, ignore); + return initializationOfParentTypesMayHaveSideEffects(definitions, ignore, seen); } public Iterable<DexType> allImmediateSupertypes() { @@ -773,17 +777,19 @@ } public boolean initializationOfParentTypesMayHaveSideEffects(DexDefinitionSupplier definitions) { - return initializationOfParentTypesMayHaveSideEffects(definitions, Predicates.alwaysFalse()); + return initializationOfParentTypesMayHaveSideEffects( + definitions, Predicates.alwaysFalse(), Sets.newIdentityHashSet()); } public boolean initializationOfParentTypesMayHaveSideEffects( - DexDefinitionSupplier definitions, Predicate<DexType> ignore) { + DexDefinitionSupplier definitions, Predicate<DexType> ignore, Set<DexType> seen) { for (DexType iface : interfaces.values) { - if (iface.classInitializationMayHaveSideEffects(definitions, ignore)) { + if (iface.classInitializationMayHaveSideEffects(definitions, ignore, seen)) { return true; } } - if (superType != null && superType.classInitializationMayHaveSideEffects(definitions, ignore)) { + if (superType != null + && superType.classInitializationMayHaveSideEffects(definitions, ignore, seen)) { return true; } return false;
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 d42caf4..d88ad91 100644 --- a/src/main/java/com/android/tools/r8/graph/DexType.java +++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -18,7 +18,9 @@ import com.android.tools.r8.utils.DescriptorUtils; import com.android.tools.r8.utils.InternalOptions.OutlineOptions; import com.google.common.base.Predicates; +import com.google.common.collect.Sets; import java.util.Arrays; +import java.util.Set; import java.util.function.Predicate; public class DexType extends DexReference implements PresortedComparable<DexType> { @@ -46,23 +48,26 @@ } public boolean classInitializationMayHaveSideEffects(DexDefinitionSupplier definitions) { - return classInitializationMayHaveSideEffects(definitions, Predicates.alwaysFalse()); + return classInitializationMayHaveSideEffects( + definitions, Predicates.alwaysFalse(), Sets.newIdentityHashSet()); } public boolean classInitializationMayHaveSideEffects( - DexDefinitionSupplier definitions, Predicate<DexType> ignore) { + DexDefinitionSupplier definitions, Predicate<DexType> ignore, Set<DexType> seen) { DexClass clazz = definitions.definitionFor(this); - return clazz == null || clazz.classInitializationMayHaveSideEffects(definitions, ignore); + return clazz == null || clazz.classInitializationMayHaveSideEffects(definitions, ignore, seen); } public boolean initializationOfParentTypesMayHaveSideEffects(AppInfo appInfo) { - return initializationOfParentTypesMayHaveSideEffects(appInfo, Predicates.alwaysFalse()); + return initializationOfParentTypesMayHaveSideEffects( + appInfo, Predicates.alwaysFalse(), Sets.newIdentityHashSet()); } public boolean initializationOfParentTypesMayHaveSideEffects( - AppInfo appInfo, Predicate<DexType> ignore) { + AppInfo appInfo, Predicate<DexType> ignore, Set<DexType> seen) { DexClass clazz = appInfo.definitionFor(this); - return clazz == null || clazz.initializationOfParentTypesMayHaveSideEffects(appInfo, ignore); + return clazz == null + || clazz.initializationOfParentTypesMayHaveSideEffects(appInfo, 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 25a9590..b22345f 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
@@ -10,6 +10,7 @@ import com.android.tools.r8.graph.DexField; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.ir.analysis.AbstractError; +import com.google.common.collect.Sets; import java.util.Collections; import java.util.List; @@ -86,7 +87,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 93e98db..58fe130 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
@@ -22,6 +22,7 @@ import com.android.tools.r8.ir.optimize.InliningConstraints; import com.android.tools.r8.ir.optimize.InliningOracle; import com.android.tools.r8.shaking.AppInfoWithLiveness; +import com.google.common.collect.Sets; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -180,7 +181,8 @@ appView.appInfo(), // 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 1853bc9..f45485e 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
@@ -21,6 +21,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 { @@ -159,7 +160,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; }
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 0bc0284..e47d131 100644 --- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java +++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -416,7 +416,8 @@ } DexClass targetClass = appInfo.definitionFor(appInfo.getSingleSubtype(clazz.type)); if ((clazz.hasClassInitializer() && targetClass.hasClassInitializer()) - || targetClass.classInitializationMayHaveSideEffects(appInfo, type -> type == clazz.type)) { + || targetClass.classInitializationMayHaveSideEffects( + appInfo, type -> type == clazz.type, Sets.newIdentityHashSet())) { // TODO(herhut): Handle class initializers. if (Log.ENABLED) { AbortReason.STATIC_INITIALIZERS.printLogMessageForClass(clazz);