Version 1.5.25

Cherry-pick: Apply -assumenosideeffects to overridden methods.
CL: https://r8-review.googlesource.com/c/r8/+/38000

Bug: 130804193
Change-Id: I1523a781f5200f25995cab36f1c0be6ec44c14b4
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 43fff83..12d6f8a 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.24";
+  public static final String LABEL = "1.5.25";
 
   private Version() {
   }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index 44cdaf7..615b4c1 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -59,6 +59,20 @@
       this.type = type;
       this.rule = rule;
     }
+
+    @Override
+    public boolean equals(Object other) {
+      if (!(other instanceof ProguardMemberRuleLookup)) {
+        return false;
+      }
+      ProguardMemberRuleLookup otherLookup = (ProguardMemberRuleLookup) other;
+      return type == otherLookup.type && rule == otherLookup.rule;
+    }
+
+    @Override
+    public int hashCode() {
+      return type.ordinal() * 31 + rule.hashCode();
+    }
   }
 
   public MemberValuePropagation(AppView<AppInfoWithLiveness> appView) {
@@ -204,10 +218,15 @@
     if (!invokedHolder.isClassType()) {
       return;
     }
-    // TODO(b/130804193): search for all call targets and apply -assumenosideeffects if one of
-    //   call targets has a matching rule?
     DexEncodedMethod definition = current.lookupSingleTarget(appView, callingContext);
     ProguardMemberRuleLookup lookup = lookupMemberRule(definition);
+    if (lookup == null) {
+      // Since -assumenosideeffects rules are always applied to all overriding methods, we can
+      // simply check the target method, although this may be different from the dynamically
+      // targeted method.
+      DexEncodedMethod target = appView.definitionFor(invokedMethod);
+      lookup = lookupMemberRule(target);
+    }
     boolean invokeReplaced = false;
     if (lookup != null) {
       boolean outValueNullOrNotUsed = current.outValue() == null || !current.outValue().isUsed();
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index c32d23f..40152ab 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -9,6 +9,7 @@
 import com.android.tools.r8.dex.Constants;
 import com.android.tools.r8.errors.Unreachable;
 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.DexAnnotation;
 import com.android.tools.r8.graph.DexAnnotationSet;
@@ -38,11 +39,13 @@
 import com.google.common.collect.Sets;
 import com.google.common.collect.Streams;
 import java.io.PrintStream;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.IdentityHashMap;
@@ -181,6 +184,10 @@
       } else if (rule instanceof ProguardAssumeMayHaveSideEffectsRule
           || rule instanceof ProguardAssumeNoSideEffectRule) {
         markMatchingVisibleMethods(clazz, memberKeepRules, rule, null, true);
+        if (appView.appInfo().hasSubtyping()) {
+          markMatchingOverriddenMethods(
+              appView.appInfo().withSubtyping(), clazz, memberKeepRules, rule, null, true);
+        }
         markMatchingVisibleFields(clazz, memberKeepRules, rule, null, true);
       } else if (rule instanceof ClassMergingRule) {
         if (allRulesSatisfied(memberKeepRules, clazz)) {
@@ -488,7 +495,8 @@
       Map<Predicate<DexDefinition>, DexDefinition> preconditionSupplier,
       boolean includeLibraryClasses) {
     Set<Wrapper<DexMethod>> methodsMarked =
-        options.forceProguardCompatibility ? null : new HashSet<>();
+        options.forceProguardCompatibility || rule instanceof ProguardAssumeNoSideEffectRule
+            ? null : new HashSet<>();
     DexClass startingClass = clazz;
     while (clazz != null) {
       if (!includeLibraryClasses && clazz.isNotProgramClass()) {
@@ -515,6 +523,41 @@
     }
   }
 
+  private void markMatchingOverriddenMethods(
+      AppInfoWithSubtyping appInfoWithSubtyping,
+      DexClass clazz,
+      Collection<ProguardMemberRule> memberKeepRules,
+      ProguardConfigurationRule rule,
+      Map<Predicate<DexDefinition>, DexDefinition> preconditionSupplier,
+      boolean onlyIncludeProgramClasses) {
+    Set<DexType> visited = new HashSet<>();
+    Deque<DexType> worklist = new ArrayDeque<>();
+    // Intentionally skip the current `clazz`, assuming it's covered by markMatchingVisibleMethods.
+    worklist.addAll(appInfoWithSubtyping.allImmediateSubtypes(clazz.type));
+
+    while (!worklist.isEmpty()) {
+      DexType currentType = worklist.poll();
+      if (!visited.add(currentType)) {
+        continue;
+      }
+      DexClass currentClazz = appView.definitionFor(currentType);
+      if (currentClazz == null) {
+        continue;
+      }
+      if (!onlyIncludeProgramClasses && currentClazz.isNotProgramClass()) {
+        continue;
+      }
+      currentClazz
+          .virtualMethods()
+          .forEach(
+              method -> {
+                DexDefinition precondition = testAndGetPrecondition(method, preconditionSupplier);
+                markMethod(method, memberKeepRules, null, rule, precondition);
+              });
+      worklist.addAll(appInfoWithSubtyping.allImmediateSubtypes(currentClazz.type));
+    }
+  }
+
   private void markMatchingMethods(
       DexClass clazz,
       Collection<ProguardMemberRule> memberKeepRules,
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
index 804fb9c..1d54c3c 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
@@ -100,12 +100,6 @@
       throw new Unreachable();
     }
 
-    private static final String OUTPUT_WITH_INFO = StringUtils.lines(
-        "TestClass: message1",
-        "TestClass: message2",
-        "[TestClass] message3",
-        "The end"
-    );
     private static final String OUTPUT_WITH_PARTIAL_INFO = StringUtils.lines(
         "TestClass: message2",
         "The end"
@@ -121,8 +115,7 @@
       switch (this) {
         case RULE_THAT_DIRECTLY_REFERS_CLASS:
         case RULE_THAT_DIRECTLY_REFERS_INTERFACE:
-          // TODO(b/130804193): implicitly mark all call targets?
-          return OUTPUT_WITH_INFO;
+          return OUTPUT_WITHOUT_INFO;
         case RULE_WITH_IMPLEMENTS:
           return OUTPUT_WITH_PARTIAL_INFO;
         default:
@@ -136,37 +129,31 @@
 
       MethodSubject mainMethod = main.mainMethod();
       assertThat(mainMethod, isPresent());
-      int expectedInfoCallsInMainMethod = 0;
-      // TODO(b/130804193): implicitly mark all call targets?
-      if (isR8) {
-        switch (this) {
-          case RULE_THAT_DIRECTLY_REFERS_CLASS:
-          case RULE_THAT_DIRECTLY_REFERS_INTERFACE:
-            expectedInfoCallsInMainMethod = 2;
-            break;
-          case RULE_WITH_IMPLEMENTS:
-            expectedInfoCallsInMainMethod = 0;
-            break;
-          default:
-            throw new Unreachable();
-        }
-      }
       assertEquals(
-          expectedInfoCallsInMainMethod,
+          0,
           Streams.stream(mainMethod.iterateInstructions(
               i -> i.isInvoke() && i.getMethod().name.toString().equals("info"))).count());
 
       MethodSubject testInvokeInterface = main.uniqueMethodWithName("testInvokeInterface");
-      int expectedInfoCallsInInvokeInterfaceMethod = 0;
-      // Inlining of testInvokeInterface is out of control if !isR8.
       if (isR8) {
-        assertThat(testInvokeInterface, isPresent());
-        // TODO(b/130804193): implicitly mark all call targets?
-        expectedInfoCallsInInvokeInterfaceMethod = 1;
-        assertEquals(
-            expectedInfoCallsInInvokeInterfaceMethod,
-            Streams.stream(testInvokeInterface.iterateInstructions(
-                i -> i.isInvoke() && i.getMethod().name.toString().equals("info"))).count());
+        switch (this) {
+          case RULE_THAT_DIRECTLY_REFERS_CLASS:
+          case RULE_THAT_DIRECTLY_REFERS_INTERFACE:
+            assertThat(testInvokeInterface, not(isPresent()));
+            break;
+          case RULE_WITH_IMPLEMENTS:
+            assertThat(testInvokeInterface, isPresent());
+            // TODO(b/130804193): upwards propagation of member rules.
+            assertEquals(
+                1,
+                Streams.stream(testInvokeInterface.iterateInstructions(
+                    i -> i.isInvoke() && i.getMethod().name.toString().equals("info"))).count());
+            break;
+          default:
+            throw new Unreachable();
+        }
+      } else {
+        assertThat(testInvokeInterface, not(isPresent()));
       }
 
       FieldSubject tag = main.uniqueFieldWithName("TAG");