During shrinking, emit warning if they are partial types

- For each partial types that are kept, a warning is generated and
a conservative shrinking is done by keeping all methods member for
partial types.

Bug: 20135591

Change-Id: Ibce494bf32d84e911a22fbdbc00a6efd5de6a713

4 files changed, 77 insertions, 61 deletions

diff --git a/jack/src/com/android/jack/analysis/tracer/Tracer.java b/jack/src/com/android/jack/analysis/tracer/Tracer.java
index 90f7344..d4d03d5 100644
--- a/jack/src/com/android/jack/analysis/tracer/Tracer.java
+++ b/jack/src/com/android/jack/analysis/tracer/Tracer.java
@@ -59,6 +59,8 @@ import com.android.jack.ir.ast.JVariable;
 import com.android.jack.ir.ast.JVisitor;
 import com.android.jack.ir.ast.marker.ThrownExceptionMarker;
 import com.android.jack.lookup.JMethodLookupException;
+import com.android.jack.reporting.Reporter.Severity;
+import com.android.jack.shrob.shrink.PartialTypeHierarchy;
 import com.android.sched.item.Description;
 import com.android.sched.marker.LocalMarkerManager;
 import com.android.sched.util.log.LoggerFactory;
@@ -193,15 +195,23 @@ public class Tracer extends JVisitor {
         }
       }
 
+      PartialTypeHierarchy pth = t.getMarker(PartialTypeHierarchy.class);
+      if (pth != null) {
+        Jack.getSession().getReporter().report(Severity.NON_FATAL, pth);
+      }
       for (JMethod method : t.getMethods()) {
         // Clinit and constructor without parameters must always be trace without taking into
         // account seed.
         if ((JMethod.isClinit(method) || isNullaryConstructor(method))) {
           trace(method);
-        } else if (brush.startTraceSeed(method)) {
-          trace(method);
-          brush.setMustTraceOverridingMethods(method);
-          brush.endTraceSeed(method);
+        } else {
+          // To be safe, Jack is conservative when there is partial type hierarchy.
+          // It considers all methods of the type as seed.
+          if (brush.startTraceSeed(method) || pth != null) {
+            trace(method);
+            brush.setMustTraceOverridingMethods(method);
+            brush.endTraceSeed(method);
+          }
         }
       }
 
diff --git a/jack/src/com/android/jack/shrob/shrink/KeeperBrush.java b/jack/src/com/android/jack/shrob/shrink/KeeperBrush.java
index e99379f..fdb2a83 100644
--- a/jack/src/com/android/jack/shrob/shrink/KeeperBrush.java
+++ b/jack/src/com/android/jack/shrob/shrink/KeeperBrush.java
@@ -27,6 +27,7 @@ import com.android.jack.ir.ast.JInterface;
 import com.android.jack.ir.ast.JMethod;
 import com.android.jack.ir.ast.JNode;
 import com.android.jack.ir.ast.JPhantomClass;
+import com.android.jack.ir.ast.JPhantomClassOrInterface;
 import com.android.jack.ir.ast.JPhantomInterface;
 import com.android.jack.shrob.seed.SeedMarker;
 import com.android.jack.shrob.spec.KeepModifier;
@@ -37,6 +38,9 @@ import com.android.sched.util.config.HasKeyId;
 import com.android.sched.util.config.ThreadConfig;
 import com.android.sched.util.config.id.BooleanPropertyId;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import javax.annotation.Nonnull;
 
 /**
@@ -127,33 +131,41 @@ public class KeeperBrush extends AbstractTracerBrush<KeepMarker> {
   public boolean startTrace(@Nonnull JDefinedClassOrInterface type) {
     boolean traceType = markIfNecessary(type);
     if (traceType) {
+      List<JPhantomClassOrInterface> unknownTypes = new ArrayList<JPhantomClassOrInterface>();
+
       if (type instanceof JDefinedClass) {
-        verifyHierarchy((JDefinedClass) type);
+        findUnknownTypes((JDefinedClass) type, unknownTypes);
       } else {
         assert type instanceof JDefinedInterface;
-        verifyImplementedInterfaces(type);
+        findUnknownTypes(type.getImplements(), unknownTypes);
+      }
+
+      if (!unknownTypes.isEmpty()) {
+        type.addMarker(new PartialTypeHierarchy(type, unknownTypes));
       }
     }
     return traceType;
   }
 
-  private void verifyHierarchy(@Nonnull JDefinedClass t) {
+  private void findUnknownTypes(@Nonnull JDefinedClass t,
+      @Nonnull List<JPhantomClassOrInterface> unknownTypes) {
     JClass superClass = t.getSuperClass();
     if (superClass instanceof JPhantomClass) {
-      t.addMarker(new PartialTypeHierarchy((JPhantomClass) superClass));
+      unknownTypes.add((JPhantomClass) superClass);
     } else if (superClass != null) {
-      verifyHierarchy((JDefinedClass) superClass);
+      findUnknownTypes((JDefinedClass) superClass, unknownTypes);
     }
-    verifyImplementedInterfaces(t);
+    findUnknownTypes(t.getImplements(), unknownTypes);
   }
 
-  private void verifyImplementedInterfaces(@Nonnull JDefinedClassOrInterface t) {
-    for (JInterface jInterface : t.getImplements()) {
+  private void findUnknownTypes(@Nonnull List<JInterface> interfaces,
+      @Nonnull List<JPhantomClassOrInterface> unknownTypes) {
+    for (JInterface jInterface : interfaces) {
       if (jInterface instanceof JPhantomInterface) {
-        t.addMarker(new PartialTypeHierarchy((JPhantomInterface) jInterface));
+        unknownTypes.add((JPhantomInterface) jInterface);
       } else {
         assert jInterface instanceof JDefinedInterface;
-        verifyImplementedInterfaces((JDefinedInterface) jInterface);
+        findUnknownTypes(((JDefinedInterface) jInterface).getImplements(), unknownTypes);
       }
     }
   }
diff --git a/jack/src/com/android/jack/shrob/shrink/MethodShrinker.java b/jack/src/com/android/jack/shrob/shrink/MethodShrinker.java
index fd22180..7b6e302 100644
--- a/jack/src/com/android/jack/shrob/shrink/MethodShrinker.java
+++ b/jack/src/com/android/jack/shrob/shrink/MethodShrinker.java
@@ -17,10 +17,7 @@
 package com.android.jack.shrob.shrink;
 
 import com.android.jack.Jack;
-import com.android.jack.JackAbortException;
 import com.android.jack.ir.ast.JMethod;
-import com.android.jack.ir.ast.JPhantomClassOrInterface;
-import com.android.jack.reporting.Reporter.Severity;
 import com.android.jack.transformations.request.Remove;
 import com.android.jack.transformations.request.TransformationRequest;
 import com.android.sched.item.Description;
@@ -43,34 +40,6 @@ import javax.annotation.Nonnull;
 @Constraint(need = {KeepMarker.class, PartialTypeHierarchy.class})
 public class MethodShrinker implements RunnableSchedulable<JMethod> {
 
-  private static final class UnknownReferencedTypeException extends Exception {
-
-    private static final long serialVersionUID = 1L;
-
-    @Nonnull
-    private final JPhantomClassOrInterface unknownType;
-
-    @Nonnull
-    private final JMethod shrinkedMethod;
-
-    private UnknownReferencedTypeException(@Nonnull JPhantomClassOrInterface unknownType,
-        @Nonnull JMethod shrinkedMethod) {
-      this.unknownType = unknownType;
-      this.shrinkedMethod = shrinkedMethod;
-    }
-
-    @Override
-    @Nonnull
-    public String getMessage() {
-      return "Unknown referenced type '"
-          + Jack.getUserFriendlyFormatter().getName(unknownType)
-          + "' while trying to remove method '"
-          + Jack.getUserFriendlyFormatter().getName(shrinkedMethod)
-          + "' from '"
-          + Jack.getUserFriendlyFormatter().getName(shrinkedMethod.getEnclosingType()) + "'";
-    }
-  }
-
   @Nonnull
   private static final Logger logger = LoggerFactory.getLogger();
 
@@ -81,21 +50,14 @@ public class MethodShrinker implements RunnableSchedulable<JMethod> {
   public synchronized void run(@Nonnull JMethod method) throws Exception {
     boolean toRemove = !method.containsMarker(KeepMarker.class);
     if (toRemove) {
+      assert method.getEnclosingType().getMarker(PartialTypeHierarchy.class) == null;
       TransformationRequest request = new TransformationRequest(method);
       request.append(new Remove(method));
       request.commit();
       logger.log(Level.INFO, "Removed method {0} from {1}", new Object[] {
           Jack.getUserFriendlyFormatter().getName(method),
           Jack.getUserFriendlyFormatter().getName(method.getEnclosingType())});
-      PartialTypeHierarchy pth = method.getEnclosingType().getMarker(PartialTypeHierarchy.class);
-      if (pth != null)  {
-        ShrinkingException reportable = new ShrinkingException(
-            new UnknownReferencedTypeException(pth.getUnknownType(), method));
-        Jack.getSession().getReporter().report(Severity.FATAL, reportable);
-        throw new JackAbortException(reportable);
-      }
     }
     tracer.getStatistic(ShrinkStatistic.METHODS_REMOVED).add(toRemove);
   }
-
 }
diff --git a/jack/src/com/android/jack/shrob/shrink/PartialTypeHierarchy.java b/jack/src/com/android/jack/shrob/shrink/PartialTypeHierarchy.java
index fd1105b..c08d40c 100644
--- a/jack/src/com/android/jack/shrob/shrink/PartialTypeHierarchy.java
+++ b/jack/src/com/android/jack/shrob/shrink/PartialTypeHierarchy.java
@@ -16,26 +16,42 @@
 
 package com.android.jack.shrob.shrink;
 
+import com.google.common.base.Function;
+import com.google.common.base.Joiner;
+import com.google.common.collect.Iterables;
+
+import com.android.jack.Jack;
 import com.android.jack.ir.ast.JDefinedClassOrInterface;
 import com.android.jack.ir.ast.JPhantomClassOrInterface;
+import com.android.jack.reporting.Reportable;
 import com.android.sched.item.Description;
 import com.android.sched.marker.DynamicValidOn;
 import com.android.sched.marker.Marker;
 
+import java.util.List;
+
 import javax.annotation.Nonnull;
 
 /**
- * Indicates that the type hierarchy is partial, {@link JDefinedClassOrInterface} contains reference
- * to types which does not belong to classpath.
+ * Indicates that the type hierarchy is partial, {@link JDefinedClassOrInterface} contains
+ * references to types which does not belong to classpath.
  */
 @Description("Indicates that the type hierarchy is partial.")
-public class PartialTypeHierarchy implements Marker {
+public class PartialTypeHierarchy implements Marker, Reportable {
+
+  @Nonnull
+  private static final Joiner typeNameJoiner = Joiner.on(", ");
+
+  @Nonnull
+  private final List<JPhantomClassOrInterface> unknownTypes;
 
   @Nonnull
-  private final JPhantomClassOrInterface unknownType;
+  private final JDefinedClassOrInterface definedType;
 
-  public PartialTypeHierarchy(@Nonnull JPhantomClassOrInterface unknownType) {
-    this.unknownType = unknownType;
+  public PartialTypeHierarchy(@Nonnull JDefinedClassOrInterface definedType,
+      @Nonnull List<JPhantomClassOrInterface> unknownTypes) {
+    this.definedType = definedType;
+    this.unknownTypes = unknownTypes;
   }
 
   @DynamicValidOn
@@ -48,8 +64,24 @@ public class PartialTypeHierarchy implements Marker {
     return this;
   }
 
+  @Override
+  @Nonnull
+  public String getMessage() {
+    return "Shrinking: force to keep members of '"
+        + Jack.getUserFriendlyFormatter().getName(definedType)
+        + "' due to unknown referenced types "
+        + typeNameJoiner.join(
+            Iterables.transform(unknownTypes, new Function<JPhantomClassOrInterface, String>() {
+              @Override
+              public String apply(JPhantomClassOrInterface arg0) {
+                return Jack.getUserFriendlyFormatter().getName(arg0);
+              }
+            }));
+  }
+
+  @Override
   @Nonnull
-  public JPhantomClassOrInterface getUnknownType() {
-    return unknownType;
+  public ProblemLevel getDefaultProblemLevel() {
+    return ProblemLevel.WARNING;
   }
 }