Version 2.0.39

Cherry pick: Mark types with dependent instance constructor as instantiated
CL: https://r8-review.googlesource.com/c/r8/+/49085

Cherry pick: Also consider referenced fields in if rule evaluator
CL: https://r8-review.googlesource.com/c/r8/+/49089

Bug: 150189783, 149729626, 147972078
Change-Id: I52855f85275708631a765f2c806cda7db82fd9d7
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 543adc6..53d04aa 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 = "2.0.38";
+  public static final String LABEL = "2.0.39";
 
   private Version() {
   }
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 6d418cd..9a33da2 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1240,6 +1240,8 @@
       annotations.forEach(annotation -> handleAnnotation(holder, annotation));
     }
 
+    rootSet.forEachDependentInstanceConstructor(
+        holder, appView, this::enqueueHolderWithDependentInstanceConstructor);
     rootSet.forEachDependentStaticMember(holder, appView, this::enqueueDependentItem);
     compatEnqueueHolderIfDependentNonStaticMember(
         holder, rootSet.getDependentKeepClassCompatRule(holder.getType()));
@@ -1294,6 +1296,13 @@
     internalEnqueueRootItem(consequent, reasons, precondition);
   }
 
+  private void enqueueHolderWithDependentInstanceConstructor(
+      DexProgramClass clazz,
+      DexEncodedMethod instanceInitializer,
+      Set<ProguardKeepRuleBase> reasons) {
+    enqueueRootItem(clazz, reasons);
+  }
+
   private void processAnnotations(DexDefinition holder, DexAnnotation[] annotations) {
     for (DexAnnotation annotation : annotations) {
       processAnnotation(holder, annotation);
@@ -1896,6 +1905,11 @@
     }
   }
 
+  public boolean isFieldReferenced(DexEncodedField field) {
+    FieldAccessInfoImpl info = fieldAccessInfoCollection.get(field.field);
+    return info != null;
+  }
+
   public boolean isFieldLive(DexEncodedField field) {
     return liveFields.contains(field);
   }
@@ -1927,6 +1941,18 @@
     return directAndIndirectlyInstantiatedTypes.contains(clazz);
   }
 
+  public boolean isMethodLive(DexEncodedMethod method) {
+    return liveMethods.contains(method);
+  }
+
+  public boolean isMethodTargeted(DexEncodedMethod method) {
+    return targetedMethods.contains(method);
+  }
+
+  public boolean isTypeLive(DexProgramClass clazz) {
+    return liveTypes.contains(clazz);
+  }
+
   // Package protected due to entry point from worklist.
   void markInstanceFieldAsReachable(DexEncodedField encodedField, KeepReason reason) {
     DexField field = encodedField.field;
@@ -2372,15 +2398,7 @@
           RootSetBuilder consequentSetBuilder = new RootSetBuilder(appView);
           IfRuleEvaluator ifRuleEvaluator =
               new IfRuleEvaluator(
-                  appView,
-                  executorService,
-                  activeIfRules,
-                  liveFields.getItems(),
-                  liveMethods.getItems(),
-                  liveTypes.getItems(),
-                  mode,
-                  consequentSetBuilder,
-                  targetedMethods.getItems());
+                  appView, this, executorService, activeIfRules, consequentSetBuilder);
           addConsequentRootSet(ifRuleEvaluator.run(), false);
           if (!workList.isEmpty()) {
             continue;
diff --git a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
index 9c6889e..c1dabc8 100644
--- a/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
+++ b/src/main/java/com/android/tools/r8/shaking/IfRuleEvaluator.java
@@ -13,7 +13,6 @@
 import com.android.tools.r8.graph.DexProgramClass;
 import com.android.tools.r8.graph.DexReference;
 import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.shaking.Enqueuer.Mode;
 import com.android.tools.r8.shaking.RootSetBuilder.ConsequentRootSet;
 import com.android.tools.r8.utils.ThreadUtils;
 import com.google.common.base.Equivalence.Wrapper;
@@ -34,35 +33,23 @@
 public class IfRuleEvaluator {
 
   private final AppView<? extends AppInfoWithSubtyping> appView;
+  private final Enqueuer enqueuer;
   private final ExecutorService executorService;
   private final List<Future<?>> futures = new ArrayList<>();
   private final Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> ifRules;
-  private final Set<DexEncodedField> liveFields;
-  private final Set<DexEncodedMethod> liveMethods;
-  private final Set<DexProgramClass> liveTypes;
-  private final Mode mode;
   private final RootSetBuilder rootSetBuilder;
-  private final Set<DexEncodedMethod> targetedMethods;
 
   IfRuleEvaluator(
       AppView<? extends AppInfoWithSubtyping> appView,
+      Enqueuer enqueuer,
       ExecutorService executorService,
       Map<Wrapper<ProguardIfRule>, Set<ProguardIfRule>> ifRules,
-      Set<DexEncodedField> liveFields,
-      Set<DexEncodedMethod> liveMethods,
-      Set<DexProgramClass> liveTypes,
-      Mode mode,
-      RootSetBuilder rootSetBuilder,
-      Set<DexEncodedMethod> targetedMethods) {
+      RootSetBuilder rootSetBuilder) {
     this.appView = appView;
+    this.enqueuer = enqueuer;
     this.executorService = executorService;
     this.ifRules = ifRules;
-    this.liveFields = liveFields;
-    this.liveMethods = liveMethods;
-    this.liveTypes = liveTypes;
-    this.mode = mode;
     this.rootSetBuilder = rootSetBuilder;
-    this.targetedMethods = targetedMethods;
   }
 
   public ConsequentRootSet run() throws ExecutionException {
@@ -188,7 +175,7 @@
     // A type is effectively live if (1) it is truly live, (2) the value of one of its fields has
     // been inlined by the member value propagation, or (3) the return value of one of its methods
     // has been forwarded by the member value propagation.
-    if (liveTypes.contains(clazz)) {
+    if (enqueuer.isTypeLive(clazz)) {
       return true;
     }
     for (DexEncodedField field : clazz.fields()) {
@@ -243,15 +230,15 @@
         filteredMembers,
         targetClass.fields(
             f ->
-                (liveFields.contains(f) || f.getOptimizationInfo().valueHasBeenPropagated())
+                (enqueuer.isFieldReferenced(f) || f.getOptimizationInfo().valueHasBeenPropagated())
                     && appView.graphLense().getOriginalFieldSignature(f.field).holder
                         == sourceClass.type));
     Iterables.addAll(
         filteredMembers,
         targetClass.methods(
             m ->
-                (liveMethods.contains(m)
-                        || targetedMethods.contains(m)
+                (enqueuer.isMethodLive(m)
+                        || enqueuer.isMethodTargeted(m)
                         || m.getOptimizationInfo().returnValueHasBeenPropagated())
                     && appView.graphLense().getOriginalMethodSignature(m.method).holder
                         == sourceClass.type));
@@ -294,7 +281,7 @@
     DexItemFactory dexItemFactory = appView.dexItemFactory();
     ProguardIfRule materializedRule = rule.materialize(dexItemFactory, preconditions);
 
-    if (mode.isInitialTreeShaking() && !rule.isUsed()) {
+    if (enqueuer.getMode().isInitialTreeShaking() && !rule.isUsed()) {
       // We need to abort class inlining of classes that could be matched by the condition of this
       // -if rule.
       ClassInlineRule neverClassInlineRuleForCondition =
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 8555b93..f5fc58f 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -1352,6 +1352,22 @@
               });
     }
 
+    public void forEachDependentInstanceConstructor(
+        DexProgramClass clazz,
+        AppView<?> appView,
+        Consumer3<DexProgramClass, DexEncodedMethod, Set<ProguardKeepRuleBase>> fn) {
+      getDependentItems(clazz)
+          .forEach(
+              (reference, reasons) -> {
+                DexDefinition definition = appView.definitionFor(reference);
+                if (definition != null
+                    && definition.isDexEncodedMethod()
+                    && definition.asDexEncodedMethod().isInstanceInitializer()) {
+                  fn.accept(clazz, definition.asDexEncodedMethod(), reasons);
+                }
+              });
+    }
+
     public void copy(DexReference original, DexReference rewritten) {
       if (noShrinking.containsKey(original)) {
         noShrinking.put(rewritten, noShrinking.get(original));
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index 84fc2ee..dd3d56f 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -188,6 +188,10 @@
     return addKeepRules("-keepattributes " + String.join(",", attributes));
   }
 
+  public T addKeepRuntimeVisibleAnnotations() {
+    return addKeepAttributes("RuntimeVisibleAnnotations");
+  }
+
   public T addKeepAllAttributes() {
     return addKeepAttributes("*");
   }
diff --git a/src/test/java/com/android/tools/r8/compatproguard/CompatKeepClassMemberNamesTestRunner.java b/src/test/java/com/android/tools/r8/compatproguard/CompatKeepClassMemberNamesTestRunner.java
index c7ffc01..412311e 100644
--- a/src/test/java/com/android/tools/r8/compatproguard/CompatKeepClassMemberNamesTestRunner.java
+++ b/src/test/java/com/android/tools/r8/compatproguard/CompatKeepClassMemberNamesTestRunner.java
@@ -3,6 +3,8 @@
 // BSD-style license that can be found in the LICENSE file.
 package com.android.tools.r8.compatproguard;
 
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.AllOf.allOf;
 import static org.hamcrest.core.StringContains.containsString;
 import static org.junit.Assert.assertFalse;
@@ -175,8 +177,8 @@
               assertTrue(inspector.clazz(MAIN_CLASS).isPresent());
               assertTrue(inspector.clazz(BAR_CLASS).isPresent());
               assertBarGetInstanceIsNotInlined(inspector);
-              assertFalse(inspector.clazz(BAR_CLASS).uniqueFieldWithName("i").isPresent());
-              assertFalse(inspector.clazz(BAR_CLASS).uniqueMethodWithName("<init>").isPresent());
+              assertThat(inspector.clazz(BAR_CLASS).init(), isPresent());
+              assertThat(inspector.clazz(BAR_CLASS).uniqueFieldWithName("i"), isPresent());
             });
   }
 
@@ -286,8 +288,8 @@
               assertTrue(inspector.clazz(MAIN_CLASS).isPresent());
               assertTrue(inspector.clazz(BAR_CLASS).isPresent());
               assertBarGetInstanceIsNotInlined(inspector);
-              assertFalse(inspector.clazz(BAR_CLASS).uniqueFieldWithName("i").isPresent());
-              assertFalse(inspector.clazz(BAR_CLASS).uniqueMethodWithName("<init>").isPresent());
+              assertThat(inspector.clazz(BAR_CLASS).uniqueMethodWithName("<init>"), isPresent());
+              assertThat(inspector.clazz(BAR_CLASS).uniqueFieldWithName("i"), isPresent());
             });
   }
 
diff --git a/src/test/java/com/android/tools/r8/shaking/annotations/B149729626.java b/src/test/java/com/android/tools/r8/shaking/annotations/B149729626.java
new file mode 100644
index 0000000..e0c497c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/annotations/B149729626.java
@@ -0,0 +1,114 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.shaking.annotations;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class B149729626 extends TestBase {
+
+  private final TestParameters parameters;
+
+  @Parameters(name = "{0}")
+  public static TestParametersCollection params() {
+    return getTestParameters().withAllRuntimesAndApiLevels().build();
+  }
+
+  public B149729626(TestParameters parameters) {
+    this.parameters = parameters;
+  }
+
+  @Test
+  public void testR8WithKeepClassMembersRule() throws Exception {
+    testForR8(parameters.getBackend())
+        .addInnerClasses(B149729626.class)
+        .addKeepMainRule(TestClass.class)
+        .addKeepRules(
+            "-keepclassmembers @" + Marker.class.getTypeName() + " class * {",
+            "  <init>(...);",
+            "}",
+            // TODO(b/149729626): Should not be required.
+            "-keep class " + TestClass.class.getTypeName() + " { void makeMarkerLive(); }")
+        // TODO(b/149729626): Should not be required.
+        .addKeepRuntimeVisibleAnnotations()
+        .setMinApi(parameters.getApiLevel())
+        .compile()
+        .inspect(this::inspect)
+        .run(parameters.getRuntime(), TestClass.class)
+        .assertSuccess();
+  }
+
+  @Test
+  public void testR8WithIfRule() throws Exception {
+    testForR8(parameters.getBackend())
+        .addInnerClasses(B149729626.class)
+        .addKeepMainRule(TestClass.class)
+        .addKeepRules(
+            "-if @" + Marker.class.getTypeName() + " class *",
+            "-keep class <1> {",
+            "  <init>(...);",
+            "}",
+            // TODO(b/149729626): Should not be required.
+            "-keep class " + TestClass.class.getTypeName() + " { void makeMarkerLive(); }")
+        // TODO(b/149729626): Should not be required.
+        .addKeepRuntimeVisibleAnnotations()
+        .setMinApi(parameters.getApiLevel())
+        .compile()
+        .inspect(this::inspect)
+        .run(parameters.getRuntime(), TestClass.class)
+        .assertSuccess();
+  }
+
+  @Test
+  public void testCompat() throws Exception {
+    testForR8Compat(parameters.getBackend())
+        .addInnerClasses(B149729626.class)
+        .addKeepMainRule(TestClass.class)
+        .setMinApi(parameters.getApiLevel())
+        .compile()
+        .inspect(this::inspect)
+        .run(parameters.getRuntime(), TestClass.class)
+        .assertSuccess();
+  }
+
+  private void inspect(CodeInspector inspector) {
+    ClassSubject markedClassSubject = inspector.clazz(Marked.class);
+    assertThat(markedClassSubject, isPresent());
+    assertThat(markedClassSubject.init(), isPresent());
+  }
+
+  static class TestClass {
+
+    public static void main(String[] args) {
+      System.out.println(Marked.class);
+    }
+
+    static void makeMarkerLive() {
+      System.out.println(Marker.class);
+    }
+  }
+
+  @Target(ElementType.TYPE)
+  @Retention(RetentionPolicy.RUNTIME)
+  @interface Marker {}
+
+  @Marker
+  static class Marked {}
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/ConditionalKeepIfKeptTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/ConditionalKeepIfKeptTest.java
index 50443fc..790ea8d 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/ConditionalKeepIfKeptTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/ConditionalKeepIfKeptTest.java
@@ -8,6 +8,7 @@
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import com.android.tools.r8.TestBase;
 import com.android.tools.r8.TestParameters;
@@ -62,9 +63,9 @@
               ClassSubject classSubject = inspector.clazz(StaticallyReferenced.class);
               assertThat(classSubject, isPresent());
               assertEquals(0, classSubject.allFields().size());
-              // TODO(b/132318799): Full mode no-marker should not keep <init>() when not specified.
-              assertEquals(useMarker ? 0 : 1, classSubject.allMethods().size());
-              assertEquals(!useMarker, classSubject.init().isPresent());
+              // TODO(b/132318799): Should not keep <init>() when not specified.
+              assertEquals(1, classSubject.allMethods().size());
+              assertTrue(classSubject.init().isPresent());
             });
   }
 
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/KeepClassesWithAnnotatedFieldsTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/KeepClassesWithAnnotatedFieldsTest.java
new file mode 100644
index 0000000..4b59d05
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/KeepClassesWithAnnotatedFieldsTest.java
@@ -0,0 +1,83 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.shaking.ifrule;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverPropagateValue;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class KeepClassesWithAnnotatedFieldsTest extends TestBase {
+
+  private final TestParameters parameters;
+
+  @Parameterized.Parameters(name = "{0}")
+  public static TestParametersCollection data() {
+    return getTestParameters().withAllRuntimesAndApiLevels().build();
+  }
+
+  public KeepClassesWithAnnotatedFieldsTest(TestParameters parameters) {
+    this.parameters = parameters;
+  }
+
+  @Test
+  public void test() throws Exception {
+    testForR8(parameters.getBackend())
+        .addInnerClasses(KeepClassesWithAnnotatedFieldsTest.class)
+        .addKeepMainRule(TestClass.class)
+        .addKeepRules(
+            "-if class * { @" + typeName(MyAnnotation.class) + " <fields>; }",
+            "-keep class <1> { <init>(); }",
+            "-keepclassmembers class * {",
+            "  @" + typeName(MyAnnotation.class) + " <fields>;",
+            "}")
+        .addKeepRuntimeVisibleAnnotations()
+        .enableInliningAnnotations()
+        .enableMemberValuePropagationAnnotations()
+        .setMinApi(parameters.getApiLevel())
+        .compile()
+        .run(parameters.getRuntime(), TestClass.class)
+        .assertSuccessWithOutputLines("Hello world!");
+  }
+
+  static class TestClass {
+
+    public static void main(String[] args) throws Exception {
+      DataClass obj = (DataClass) getDataClass().getDeclaredConstructor().newInstance();
+      setField(obj, "Hello world!");
+      System.out.println(obj.field);
+    }
+
+    @NeverInline
+    @NeverPropagateValue
+    static Class<?> getDataClass() {
+      return DataClass.class;
+    }
+
+    @NeverInline
+    static void setField(Object object, String value) throws Exception {
+      getDataClass().getDeclaredField("field").set(object, value);
+    }
+  }
+
+  static class DataClass {
+
+    @MyAnnotation
+    String field;
+  }
+
+  @Retention(RetentionPolicy.RUNTIME)
+  @Target(ElementType.FIELD)
+  @interface MyAnnotation {}
+}