Keep static members matched by -keepclassmembers on kept classes
When a class is kept but not instantiated, the -keepclassmembers rules
for static members will now be applied. Before -keepclassmembers rules
where only applied if a class was instantitiated.
Bug: 69028743
Change-Id: I846db05b1a978069072b136d11af00d550692eea
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 215ea88..5b79fce 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -363,6 +363,9 @@
annotations.forEach(this::handleAnnotationOfLiveType);
}
+ // Add all dependent static members to the workqueue.
+ enqueueRootItems(rootSet.getDependentStaticMembers(type));
+
// For Proguard compatibility mark default initializer for live type as live.
if (options.forceProguardCompatibility) {
if (holder.hasDefaultInitializer()) {
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 bb8c08a..53f5fb7 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -38,10 +38,12 @@
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
+import java.util.stream.Collectors;
public class RootSetBuilder {
@@ -582,6 +584,23 @@
.unmodifiableMap(dependentNoShrinking.getOrDefault(item, Collections.emptyMap()));
}
+ private boolean isStaticMember(Entry<DexItem, ProguardKeepRule> entry) {
+ if (entry.getKey() instanceof DexEncodedMethod) {
+ return ((DexEncodedMethod) entry.getKey()).accessFlags.isStatic();
+ }
+ if (entry.getKey() instanceof DexEncodedField) {
+ return ((DexEncodedField) entry.getKey()).accessFlags.isStatic();
+ }
+ return false;
+ }
+
+ Map<DexItem, ProguardKeepRule> getDependentStaticMembers(DexItem item) {
+ assert item instanceof DexType;
+ return getDependentItems(item).entrySet().stream()
+ .filter(this::isStaticMember)
+ .collect(Collectors.toMap(Entry::getKey, Entry::getValue));
+ }
+
@Override
public String toString() {
StringBuilder builder = new StringBuilder();
diff --git a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/KeepClassMembersTest.java b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/KeepClassMembersTest.java
new file mode 100644
index 0000000..7904400
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/KeepClassMembersTest.java
@@ -0,0 +1,72 @@
+// Copyright (c) 2017, 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.keepclassmembers;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+
+public class KeepClassMembersTest extends TestBase {
+
+ public void runTest(Class mainClass, Class<?> staticClass,
+ boolean forceProguardCompatibility) throws Exception {
+ boolean staticClassHasDefaultConstructor = true;
+ try {
+ staticClass.getDeclaredConstructor();
+ } catch (NoSuchMethodException e) {
+ staticClassHasDefaultConstructor = false;
+ }
+ String proguardConfig = String.join("\n", ImmutableList.of(
+ "-keepclassmembers class **.PureStatic* {",
+ " public static int b;",
+ " public static int getA();",
+ " public int getI();",
+ "}",
+ "-keep class **.MainUsing* {",
+ " public static void main(java.lang.String[]);",
+ "}",
+ "-dontoptimize", "-dontobfuscate"
+ ));
+ DexInspector inspector = new DexInspector(
+ compileWithR8(ImmutableList.of(mainClass, staticClass), proguardConfig,
+ options -> options.forceProguardCompatibility = forceProguardCompatibility));
+ assertTrue(inspector.clazz(mainClass).isPresent());
+ ClassSubject staticClassSubject = inspector.clazz(staticClass);
+ assertTrue(staticClassSubject.isPresent());
+ assertTrue(staticClassSubject.method("int", "getA", ImmutableList.of()).isPresent());
+ assertFalse(staticClassSubject.method("int", "getB", ImmutableList.of()).isPresent());
+ assertTrue(staticClassSubject.field("int", "a").isPresent());
+ assertTrue(staticClassSubject.field("int", "b").isPresent());
+ assertFalse(staticClassSubject.field("int", "c").isPresent());
+ // Force Proguard compatibility keeps the default constructor if present and then assumes
+ // instantiated, hence keeps the instance method as well.
+ assertEquals(forceProguardCompatibility && staticClassHasDefaultConstructor,
+ staticClassSubject.init(ImmutableList.of()).isPresent());
+ assertEquals(forceProguardCompatibility && staticClassHasDefaultConstructor,
+ staticClassSubject.method("int", "getI", ImmutableList.of()).isPresent());
+ assertEquals(forceProguardCompatibility && staticClassHasDefaultConstructor,
+ staticClassSubject.field("int", "i").isPresent());
+ assertFalse(staticClassSubject.method("int", "getJ", ImmutableList.of()).isPresent());
+ assertFalse(staticClassSubject.field("int", "j").isPresent());
+ }
+
+ @Test
+ public void regress69028743() throws Exception {
+ runTest(MainUsingWithDefaultConstructor.class,
+ PureStaticClassWithDefaultConstructor.class, false);
+ runTest(MainUsingWithDefaultConstructor.class,
+ PureStaticClassWithDefaultConstructor.class, true);
+ runTest(MainUsingWithoutDefaultConstructor.class,
+ PureStaticClassWithoutDefaultConstructor.class, false);
+ runTest(MainUsingWithoutDefaultConstructor.class,
+ PureStaticClassWithoutDefaultConstructor.class, true);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/MainUsingWithDefaultConstructor.java b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/MainUsingWithDefaultConstructor.java
new file mode 100644
index 0000000..9c7da5d
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/MainUsingWithDefaultConstructor.java
@@ -0,0 +1,12 @@
+// Copyright (c) 2017, 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.keepclassmembers;
+
+public class MainUsingWithDefaultConstructor {
+
+ public static void main(String[] args) {
+ PureStaticClassWithDefaultConstructor.setA(1);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/MainUsingWithoutDefaultConstructor.java b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/MainUsingWithoutDefaultConstructor.java
new file mode 100644
index 0000000..2b1b5ef
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/MainUsingWithoutDefaultConstructor.java
@@ -0,0 +1,12 @@
+// Copyright (c) 2017, 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.keepclassmembers;
+
+public class MainUsingWithoutDefaultConstructor {
+
+ public static void main(String[] args) {
+ PureStaticClassWithoutDefaultConstructor.setA(1);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/PureStaticClassWithDefaultConstructor.java b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/PureStaticClassWithDefaultConstructor.java
new file mode 100644
index 0000000..bf45469
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/PureStaticClassWithDefaultConstructor.java
@@ -0,0 +1,49 @@
+// Copyright (c) 2017, 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.keepclassmembers;
+
+public class PureStaticClassWithDefaultConstructor {
+ public static int a;
+ public static int b;
+ public static int c;
+
+ public int i;
+ public int j;
+
+ private PureStaticClassWithDefaultConstructor() {
+ }
+
+ public static int getA() {
+ return a;
+ }
+
+ public static void setA(int value) {
+ a = value;
+ }
+
+ public static int getB() {
+ return b;
+ }
+
+ public static void setB(int value) {
+ b = value;
+ }
+
+ public int getI() {
+ return i;
+ }
+
+ public void setI(int value) {
+ i = value;
+ }
+
+ public int getJ() {
+ return j;
+ }
+
+ public void setJ(int value) {
+ j = value;
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/PureStaticClassWithoutDefaultConstructor.java b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/PureStaticClassWithoutDefaultConstructor.java
new file mode 100644
index 0000000..7537f81
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/PureStaticClassWithoutDefaultConstructor.java
@@ -0,0 +1,51 @@
+// Copyright (c) 2017, 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.keepclassmembers;
+
+public class PureStaticClassWithoutDefaultConstructor {
+
+ public static int a;
+ public static int b;
+ public static int c;
+
+ public int i;
+ public int j;
+
+ private PureStaticClassWithoutDefaultConstructor(int i) {
+ a = i;
+ }
+
+ public static int getA() {
+ return a;
+ }
+
+ public static void setA(int value) {
+ a = value;
+ }
+
+ public static int getB() {
+ return b;
+ }
+
+ public static void setB(int value) {
+ b = value;
+ }
+
+ public int getI() {
+ return i;
+ }
+
+ public void setI(int value) {
+ i = value;
+ }
+
+ public int getJ() {
+ return j;
+ }
+
+ public void setJ(int value) {
+ j = value;
+ }
+}
