Check if methods are overriding if they are seen when marking live

Bug: 182456011
Bug: 182458496
Bug: 182444403
Change-Id: I26a6a30cd7d42f05e9121876417ebf3ba2892ba1

3 files changed, 30 insertions, 12 deletions

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 331466be2..8c7060583 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -5,11 +5,13 @@ package com.android.tools.r8.shaking;
 
 import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
 import static com.android.tools.r8.graph.FieldAccessInfoImpl.MISSING_FIELD_ACCESS_INFO;
+import static com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult.isOverriding;
 import static com.android.tools.r8.ir.desugar.LambdaDescriptor.isLambdaMetafactoryMethod;
 import static com.android.tools.r8.ir.optimize.enums.UnboxedEnumMemberRelocator.ENUM_UNBOXING_UTILITY_CLASS_SUFFIX;
 import static com.android.tools.r8.naming.IdentifierNameStringUtils.identifyIdentifier;
 import static com.android.tools.r8.naming.IdentifierNameStringUtils.isReflectionMethod;
 import static com.android.tools.r8.shaking.AnnotationRemover.shouldKeepAnnotation;
+import static com.android.tools.r8.utils.FunctionUtils.ignoreArgument;
 
 import com.android.tools.r8.Diagnostic;
 import com.android.tools.r8.cf.code.CfInstruction;
@@ -140,7 +142,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Deque;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.IdentityHashMap;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -2363,11 +2364,13 @@ public class Enqueuer {
    */
   private void transitionMethodsForInstantiatedObject(
       InstantiatedObject instantiation, DexClass clazz, List<DexType> interfaces) {
-    Set<Wrapper<DexMethod>> seen = new HashSet<>();
+    Map<Wrapper<DexMethod>, List<DexEncodedMethod>> seen = new HashMap<>();
     WorkList<DexType> worklist = WorkList.newIdentityWorkList();
     worklist.addIfNotSeen(interfaces);
     // First we lookup and mark all targets on the instantiated class for each reachable method in
     // the super chain (inclusive).
+    // TODO(b/182458496): Consider changing this algorithm to a version that collects all reachable
+    //  methods and then does lookup.
     DexClass initialClass = clazz;
     while (clazz != null) {
       if (clazz.isProgramClass()) {
@@ -2405,14 +2408,25 @@ public class Enqueuer {
   private void markProgramMethodOverridesAsLive(
       InstantiatedObject instantiation,
       DexClass initialClass,
-      DexProgramClass superClass,
-      Set<Wrapper<DexMethod>> seenMethods) {
-    for (DexMethod method : getReachableVirtualTargets(superClass)) {
-      assert method.holder == superClass.type;
+      DexProgramClass currentClass,
+      Map<Wrapper<DexMethod>, List<DexEncodedMethod>> seenMethods) {
+    for (DexMethod method : getReachableVirtualTargets(currentClass)) {
+      assert method.holder == currentClass.type;
       Wrapper<DexMethod> signature = MethodSignatureEquivalence.get().wrap(method);
-      if (!seenMethods.contains(signature)) {
+      // TODO(b/182456011): Could reachableVirtalTarget be dex encoded methods.
+      DexEncodedMethod currentTarget = definitionFor(method, currentClass);
+      List<DexEncodedMethod> potentialOverrides =
+          seenMethods.computeIfAbsent(signature, ignoreArgument(ArrayList::new));
+      boolean notOverridingCurrentTarget = potentialOverrides.isEmpty();
+      for (DexEncodedMethod potentialOverride : potentialOverrides) {
+        if (!isOverriding(currentTarget, potentialOverride)) {
+          notOverridingCurrentTarget = true;
+          break;
+        }
+      }
+      if (notOverridingCurrentTarget) {
         SingleResolutionResult resolution =
-            appInfo.resolveMethodOn(superClass, method).asSingleResolution();
+            appInfo.resolveMethodOn(currentClass, method).asSingleResolution();
         assert resolution != null;
         assert resolution.getResolvedHolder().isProgramClass();
         if (resolution != null) {
@@ -2420,11 +2434,11 @@ public class Enqueuer {
               || resolution
                   .isAccessibleForVirtualDispatchFrom(initialClass.asProgramClass(), appInfo)
                   .isTrue()) {
-            seenMethods.add(signature);
+            potentialOverrides.add(currentTarget);
           }
           if (resolution.getResolvedHolder().isProgramClass()) {
             markLiveOverrides(
-                instantiation, superClass, resolution.getResolutionPair().asProgramMethod());
+                instantiation, currentClass, resolution.getResolutionPair().asProgramMethod());
           }
         }
       }
diff --git a/src/main/java/com/android/tools/r8/utils/FunctionUtils.java b/src/main/java/com/android/tools/r8/utils/FunctionUtils.java
index 3338dde56..acd5cb3c2 100644
--- a/src/main/java/com/android/tools/r8/utils/FunctionUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/FunctionUtils.java
@@ -7,6 +7,7 @@ package com.android.tools.r8.utils;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.function.Supplier;
 
 public class FunctionUtils {
 
@@ -28,4 +29,8 @@ public class FunctionUtils {
       func.apply(t).accept(argument);
     }
   }
+
+  public static <T, R> Function<T, R> ignoreArgument(Supplier<R> supplier) {
+    return ignore -> supplier.get();
+  }
 }
diff --git a/src/test/java/com/android/tools/r8/shaking/AbstractSuperClassLiveMethodTest.java b/src/test/java/com/android/tools/r8/shaking/AbstractSuperClassLiveMethodTest.java
index 8cdd48ea6..f9583f768 100644
--- a/src/test/java/com/android/tools/r8/shaking/AbstractSuperClassLiveMethodTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/AbstractSuperClassLiveMethodTest.java
@@ -69,8 +69,7 @@ public class AbstractSuperClassLiveMethodTest extends TestBase {
         .applyIf(
             parameters.isDexRuntime() && parameters.getDexRuntimeVersion().isDalvik(),
             r -> r.assertSuccessWithOutputLines(EXPECTED_DALVIK),
-            // TODO(b/182444403): Should succeed with EXPECTED.
-            r -> r.assertFailureWithErrorThatThrows(AbstractMethodError.class));
+            r -> r.assertSuccessWithOutputLines(EXPECTED));
   }
 
   @NoVerticalClassMerging