Do not mark initializers with @AssumeMayHaveSideEffects as trivial

Bug: 142772856
Change-Id: I640118222c200b71b7d0b611779bdd5d1118b9bb
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index b7f7a4d..4935103 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -205,7 +205,6 @@
             .map(prefix -> "L" + DescriptorUtils.getPackageBinaryNameFromJavaType(prefix))
             .map(options.itemFactory::createString)
             .collect(Collectors.toList());
-    this.methodOptimizationInfoCollector = new MethodOptimizationInfoCollector(appView);
     if (options.isDesugaredLibraryCompilation()) {
       // Specific L8 Settings.
       // BackportedMethodRewriter is needed for retarget core library members and backports.
@@ -241,6 +240,7 @@
       this.stringSwitchRemover = null;
       this.desugaredLibraryAPIConverter = null;
       this.serviceLoaderRewriter = null;
+      this.methodOptimizationInfoCollector = null;
       return;
     }
     this.lambdaRewriter = options.enableDesugaring ? new LambdaRewriter(appView, this) : null;
@@ -295,6 +295,8 @@
       this.outliner = new Outliner(appViewWithLiveness, this);
       this.memberValuePropagation =
           options.enableValuePropagation ? new MemberValuePropagation(appViewWithLiveness) : null;
+      this.methodOptimizationInfoCollector =
+          new MethodOptimizationInfoCollector(appViewWithLiveness);
       if (options.isMinifying()) {
         this.identifierNameStringMarker = new IdentifierNameStringMarker(appViewWithLiveness);
       } else {
@@ -330,6 +332,7 @@
       this.d8NestBasedAccessDesugaring =
           options.shouldDesugarNests() ? new D8NestBasedAccessDesugaring(appView) : null;
       this.serviceLoaderRewriter = null;
+      this.methodOptimizationInfoCollector = null;
     }
     this.stringSwitchRemover =
         options.isStringSwitchConversionEnabled()
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
index e23fa01..8cbc644 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -61,11 +61,11 @@
 import java.util.function.Function;
 
 public class MethodOptimizationInfoCollector {
-  private final AppView<?> appView;
+  private final AppView<AppInfoWithLiveness> appView;
   private final InternalOptions options;
   private final DexItemFactory dexItemFactory;
 
-  public MethodOptimizationInfoCollector(AppView<?> appView) {
+  public MethodOptimizationInfoCollector(AppView<AppInfoWithLiveness> appView) {
     this.appView = appView;
     this.options = appView.options();
     this.dexItemFactory = appView.dexItemFactory();
@@ -264,7 +264,9 @@
 
   private void identifyTrivialInitializer(
       DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
-    if (!method.isInstanceInitializer() && !method.isClassInitializer()) {
+    assert !appView.appInfo().isPinned(method.method);
+
+    if (!method.isInitializer()) {
       return;
     }
 
@@ -272,8 +274,13 @@
       return;
     }
 
+    if (appView.appInfo().mayHaveSideEffects.containsKey(method.method)) {
+      return;
+    }
+
     DexClass clazz = appView.appInfo().definitionFor(method.method.holder);
     if (clazz == null) {
+      assert false;
       return;
     }
 
diff --git a/src/test/java/com/android/tools/r8/shaking/librarymethodoverride/LibraryMethodOverrideTest.java b/src/test/java/com/android/tools/r8/shaking/librarymethodoverride/LibraryMethodOverrideTest.java
index ca354f9..a5fbc59 100644
--- a/src/test/java/com/android/tools/r8/shaking/librarymethodoverride/LibraryMethodOverrideTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/librarymethodoverride/LibraryMethodOverrideTest.java
@@ -8,7 +8,6 @@
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 
-import com.android.tools.r8.AssumeMayHaveSideEffects;
 import com.android.tools.r8.NeverClassInline;
 import com.android.tools.r8.NeverMerge;
 import com.android.tools.r8.TestBase;
@@ -29,7 +28,7 @@
 
   @Parameterized.Parameters(name = "{0}")
   public static TestParametersCollection data() {
-    return getTestParameters().withAllRuntimes().build();
+    return getTestParameters().withAllRuntimesAndApiLevels().build();
   }
 
   public LibraryMethodOverrideTest(TestParameters parameters) {
@@ -44,8 +43,7 @@
         .addOptionsModification(options -> options.enableTreeShakingOfLibraryMethodOverrides = true)
         .enableClassInliningAnnotations()
         .enableMergeAnnotations()
-        .enableSideEffectAnnotations()
-        .setMinApi(parameters.getRuntime())
+        .setMinApi(parameters.getApiLevel())
         .compile()
         .inspect(this::verifyOutput)
         .run(parameters.getRuntime(), TestClass.class)
@@ -68,7 +66,16 @@
     for (Class<?> nonEscapingClass : nonEscapingClasses) {
       ClassSubject classSubject = inspector.clazz(nonEscapingClass);
       assertThat(classSubject, isPresent());
-      assertThat(classSubject.uniqueMethodWithName("toString"), not(isPresent()));
+
+      // TODO(b/142772856): None of the non-escaping classes should have a toString() method. It is
+      //  a requirement that the instance initializers are considered trivial for this to work,
+      //  though, even when they have a side effect (as long as the receiver does not escape via the
+      //  side effecting instruction).
+      if (nonEscapingClass == DoesNotEscapeWithSubThatDoesNotOverrideSub.class) {
+        assertThat(classSubject.uniqueMethodWithName("toString"), not(isPresent()));
+      } else {
+        assertThat(classSubject.uniqueMethodWithName("toString"), isPresent());
+      }
     }
   }
 
@@ -128,8 +135,12 @@
   @NeverClassInline
   static class DoesNotEscape {
 
-    @AssumeMayHaveSideEffects
-    DoesNotEscape() {}
+    // TODO(b/142772856): Should be classified as a trivial instance initializer although it has a
+    //  side effect.
+    DoesNotEscape() {
+      // Side effect to ensure that the constructor is not removed from main().
+      System.out.print("");
+    }
 
     @Override
     public String toString() {
@@ -140,8 +151,12 @@
   @NeverClassInline
   static class DoesNotEscapeWithSubThatDoesNotOverride {
 
-    @AssumeMayHaveSideEffects
-    DoesNotEscapeWithSubThatDoesNotOverride() {}
+    // TODO(b/142772856): Should be classified as a trivial instance initializer although it has a
+    //  side effect.
+    DoesNotEscapeWithSubThatDoesNotOverride() {
+      // Side effect to ensure that the constructor is not removed from main().
+      System.out.print("");
+    }
 
     @Override
     public String toString() {
@@ -153,15 +168,23 @@
   static class DoesNotEscapeWithSubThatDoesNotOverrideSub
       extends DoesNotEscapeWithSubThatDoesNotOverride {
 
-    @AssumeMayHaveSideEffects
-    DoesNotEscapeWithSubThatDoesNotOverrideSub() {}
+    // TODO(b/142772856): Should be classified as a trivial instance initializer although it has a
+    //  side effect.
+    DoesNotEscapeWithSubThatDoesNotOverrideSub() {
+      // Side effect to ensure that the constructor is not removed from main().
+      System.out.print("");
+    }
   }
 
   @NeverClassInline
   static class DoesNotEscapeWithSubThatOverrides {
 
-    @AssumeMayHaveSideEffects
-    DoesNotEscapeWithSubThatOverrides() {}
+    // TODO(b/142772856): Should be classified as a trivial instance initializer although it has a
+    //  side effect.
+    DoesNotEscapeWithSubThatOverrides() {
+      // Side effect to ensure that the constructor is not removed from main().
+      System.out.print("");
+    }
 
     @Override
     public String toString() {
@@ -172,8 +195,12 @@
   @NeverClassInline
   static class DoesNotEscapeWithSubThatOverridesSub extends DoesNotEscapeWithSubThatOverrides {
 
-    @AssumeMayHaveSideEffects
-    DoesNotEscapeWithSubThatOverridesSub() {}
+    // TODO(b/142772856): Should be classified as a trivial instance initializer although it has a
+    //  side effect.
+    DoesNotEscapeWithSubThatOverridesSub() {
+      // Side effect to ensure that the constructor is not removed from main().
+      System.out.print("");
+    }
 
     @Override
     public String toString() {
@@ -188,8 +215,12 @@
   @NeverClassInline
   static class DoesNotEscapeWithSubThatOverridesAndEscapes {
 
-    @AssumeMayHaveSideEffects
-    DoesNotEscapeWithSubThatOverridesAndEscapes() {}
+    // TODO(b/142772856): Should be classified as a trivial instance initializer although it has a
+    //  side effect.
+    DoesNotEscapeWithSubThatOverridesAndEscapes() {
+      // Side effect to ensure that the constructor is not removed from main().
+      System.out.print("");
+    }
 
     @Override
     public String toString() {
@@ -201,8 +232,12 @@
   static class DoesNotEscapeWithSubThatOverridesAndEscapesSub
       extends DoesNotEscapeWithSubThatOverridesAndEscapes {
 
-    @AssumeMayHaveSideEffects
-    DoesNotEscapeWithSubThatOverridesAndEscapesSub() {}
+    // TODO(b/142772856): Should be classified as a trivial instance initializer although it has a
+    //  side effect.
+    DoesNotEscapeWithSubThatOverridesAndEscapesSub() {
+      // Side effect to ensure that the constructor is not removed from main().
+      System.out.print("");
+    }
 
     @Override
     public String toString() {