Keep more annotations/attributes for inner/outer class relationship
In default/full R8 mode the annotations/attributes related to
inner/outer class relationship is only retained for kept classes. If
only one of the inner or the outer class is kept this change will the
annotations/attributes for both the inner and the outer class.
Bug: 117464680
Change-Id: I5f916881dd8272cf78fa07a311bbd68b3b4fb946
diff --git a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
index 0c47df1..d194bb2 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -11,8 +11,10 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.InnerClassAttribute;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
+import java.util.List;
public class AnnotationRemover {
@@ -135,11 +137,36 @@
field.annotations = field.annotations.keepIf(this::filterAnnotations);
}
+ private boolean enclosingMethodPinned(DexClass clazz) {
+ return clazz.getEnclosingMethod() != null
+ && clazz.getEnclosingMethod().getEnclosingClass() != null
+ && appInfo.isPinned(clazz.getEnclosingMethod().getEnclosingClass());
+ }
+
+ private boolean innerClassPinned(DexClass clazz) {
+ List<InnerClassAttribute> innerClasses = clazz.getInnerClasses();
+ for (InnerClassAttribute innerClass : innerClasses) {
+ if (appInfo.isPinned(innerClass.getInner())) {
+ return true;
+ }
+ if (appInfo.isPinned(innerClass.getOuter())) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private void stripAttributes(DexProgramClass clazz) {
// If [clazz] is mentioned by a keep rule, it could be used for reflection, and we therefore
// need to keep the enclosing method and inner classes attributes, if requested. In Proguard
// compatibility mode we keep these attributes independent of whether the given class is kept.
- if (appInfo.isPinned(clazz.type) || options.forceProguardCompatibility) {
+ // To ensure reflection from both inner to outer and and outer to inner for kept classes - even
+ // if only one side is kept - keep the attributes is any class mentioned in these attributes
+ // is kept.
+ if (appInfo.isPinned(clazz.type)
+ || enclosingMethodPinned(clazz)
+ || innerClassPinned(clazz)
+ || options.forceProguardCompatibility) {
if (!keep.enclosingMethod) {
clazz.clearEnclosingMethod();
}
diff --git a/src/test/java/com/android/tools/r8/shaking/innerclassesenclosingmethod/KeepInnerClassesEnclosingMethodAnnotationsTest.java b/src/test/java/com/android/tools/r8/shaking/innerclassesenclosingmethod/KeepInnerClassesEnclosingMethodAnnotationsTest.java
new file mode 100644
index 0000000..474194e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/innerclassesenclosingmethod/KeepInnerClassesEnclosingMethodAnnotationsTest.java
@@ -0,0 +1,141 @@
+// Copyright (c) 2018, 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.innerclassesenclosingmethod;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isMemberClass;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.shaking.innerclassesenclosingmethod.testclasses.Outer;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+
+class Main {
+
+ public static void main(String[] args) {
+ Class<?>[] declaredClasses = Outer.class.getDeclaredClasses();
+ if (declaredClasses.length == 0) {
+ System.out.println("No declared classes");
+ } else {
+ for (int i = 0; i < declaredClasses.length; i++) {
+ System.out.println("Declared class: " + declaredClasses[i].getName());
+ }
+ }
+ if (Outer.Inner.class.getDeclaringClass() == null) {
+ System.out.println("No declaring classes");
+ } else {
+ System.out.println("Declaring class: " + Outer.Inner.class.getDeclaringClass().getName());
+ }
+ }
+}
+
+public class KeepInnerClassesEnclosingMethodAnnotationsTest extends TestBase {
+ private static class TestResult {
+
+ public final AndroidApp app;
+ public final CodeInspector inspector;
+ public final ClassSubject outer;
+ public final ClassSubject inner;
+
+ TestResult(AndroidApp app) throws Throwable {
+ this.app = app;
+ this.inspector = new CodeInspector(app);
+ this.outer = inspector.clazz(Outer.class);
+ this.inner = inspector.clazz(Outer.Inner.class);
+ assertThat(outer, isPresent());
+ assertThat(inner, isPresent());
+ }
+ }
+
+ private TestResult runTest(List<String> proguardConfiguration) throws Throwable {
+ return new TestResult(
+ ToolHelper.runR8(
+ R8Command.builder()
+ .addProgramFiles(ToolHelper.getClassFilesForTestPackage(Outer.class.getPackage()))
+ .addClassProgramData(ToolHelper.getClassAsBytes(Main.class), Origin.unknown())
+ .addProguardConfiguration(proguardConfiguration, Origin.unknown())
+ .setProgramConsumer(emptyConsumer(Backend.DEX))
+ .build()));
+ }
+
+ private void noInnerClassesEnclosingMethodInformation(TestResult result) throws Throwable {
+ List<String> lines = StringUtils.splitLines(runOnArt(result.app, Main.class));
+ assertEquals(2, lines.size());
+ assertEquals("No declared classes", lines.get(0));
+ assertEquals("No declaring classes", lines.get(1));
+ }
+
+ private void fullInnerClassesEnclosingMethodInformation(TestResult result) throws Throwable {
+ List<String> lines = StringUtils.splitLines(runOnArt(result.app, Main.class));
+ assertEquals(2, lines.size());
+ assertEquals("Declared class: " + result.inner.getFinalName(), lines.get(0));
+ assertEquals("Declaring class: " + result.outer.getFinalName(), lines.get(1));
+ }
+
+ @Test
+ public void testKeepAll() throws Throwable {
+ TestResult result =
+ runTest(
+ ImmutableList.of(
+ "-keepattributes InnerClasses,EnclosingMethod",
+ "-keep class **Outer*",
+ keepMainProguardConfiguration(Main.class)));
+ assertThat(result.outer, not(isRenamed()));
+ assertThat(result.inner, not(isRenamed()));
+ assertThat(result.inner, isMemberClass());
+ fullInnerClassesEnclosingMethodInformation(result);
+ }
+
+ @Test
+ public void testKeepAllWithoutAttributes() throws Throwable {
+ TestResult result =
+ runTest(
+ ImmutableList.of("-keep class **Outer*", keepMainProguardConfiguration(Main.class)));
+ assertThat(result.outer, not(isRenamed()));
+ assertThat(result.inner, not(isRenamed()));
+ assertThat(result.inner, not(isMemberClass()));
+ noInnerClassesEnclosingMethodInformation(result);
+ }
+
+ @Test
+ public void testKeepInner() throws Throwable {
+ TestResult result =
+ runTest(
+ ImmutableList.of(
+ "-keepattributes InnerClasses,EnclosingMethod",
+ "-keep class **Outer$Inner",
+ keepMainProguardConfiguration(Main.class)));
+ assertThat(result.outer, isRenamed());
+ assertThat(result.inner, not(isRenamed()));
+ assertThat(result.inner, isMemberClass());
+ fullInnerClassesEnclosingMethodInformation(result);
+ }
+
+ @Test
+ public void testKeepOuter() throws Throwable {
+ TestResult result =
+ runTest(
+ ImmutableList.of(
+ "-keepattributes InnerClasses,EnclosingMethod",
+ "-keep class **Outer",
+ keepMainProguardConfiguration(Main.class)));
+ assertThat(result.outer, not(isRenamed()));
+ assertThat(result.inner, isRenamed());
+ assertThat(result.inner, isMemberClass());
+ fullInnerClassesEnclosingMethodInformation(result);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/innerclassesenclosingmethod/testclasses/Outer.java b/src/test/java/com/android/tools/r8/shaking/innerclassesenclosingmethod/testclasses/Outer.java
new file mode 100644
index 0000000..064583f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/innerclassesenclosingmethod/testclasses/Outer.java
@@ -0,0 +1,10 @@
+// Copyright (c) 2018, 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.innerclassesenclosingmethod.testclasses;
+
+public class Outer {
+
+ public static class Inner {}
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
index 3b718a4..7f310ed 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
@@ -115,6 +115,25 @@
};
}
+ public static Matcher<ClassSubject> isMemberClass() {
+ return new TypeSafeMatcher<ClassSubject>() {
+ @Override
+ public boolean matchesSafely(final ClassSubject clazz) {
+ return clazz.isMemberClass();
+ }
+
+ @Override
+ public void describeTo(final Description description) {
+ description.appendText("is member class");
+ }
+
+ @Override
+ public void describeMismatchSafely(final ClassSubject clazz, Description description) {
+ description.appendText("class ").appendValue(clazz.getOriginalName()).appendText(" is not");
+ }
+ };
+ }
+
public static Matcher<MethodSubject> isAbstract() {
return new TypeSafeMatcher<MethodSubject>() {
@Override
