Merge "Handle classes implementing merged types in Proguard -if rules"
diff --git a/build.gradle b/build.gradle
index f5a3337..cfaacdb 100644
--- a/build.gradle
+++ b/build.gradle
@@ -1310,17 +1310,23 @@
if (project.property('tool') == 'r8') {
exclude "com/android/tools/r8/art/*/d8/**"
exclude "com/android/tools/r8/jctf/d8/**"
- } else {
- assert(project.property('tool') == 'd8')
+ exclude "com/android/tools/r8/jctf/r8cf/**"
+ } else if (project.property('tool') == 'd8') {
exclude "com/android/tools/r8/art/*/r8/**"
exclude "com/android/tools/r8/jctf/r8/**"
+ exclude "com/android/tools/r8/jctf/r8cf/**"
+ } else {
+ assert(project.property('tool') == 'r8cf')
+ exclude "com/android/tools/r8/art/*/d8/**"
+ exclude "com/android/tools/r8/art/*/r8/**"
+ exclude "com/android/tools/r8/jctf/d8/**"
+ exclude "com/android/tools/r8/jctf/r8/**"
}
}
if (!project.hasProperty('all_tests')) {
exclude "com/android/tools/r8/art/dx/**"
exclude "com/android/tools/r8/art/jack/**"
}
- // TODO(tamaskenez) enable jctf on all_tests when consolidated
if (!project.hasProperty('jctf') && !project.hasProperty('only_jctf')) {
exclude "com/android/tools/r8/jctf/**"
}
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
diff --git a/tools/disasm.py b/tools/disasm.py
index 0d2599a..a9a5a0a 100755
--- a/tools/disasm.py
+++ b/tools/disasm.py
@@ -7,4 +7,4 @@
import toolhelper
if __name__ == '__main__':
- sys.exit(toolhelper.run('disasm', sys.argv[1:]))
+ sys.exit(toolhelper.run('disasm', sys.argv[1:]), debug=False)
diff --git a/tools/test.py b/tools/test.py
index 72e7e01..cd9b70f 100755
--- a/tools/test.py
+++ b/tools/test.py
@@ -52,14 +52,14 @@
help='Print a line before a tests starts and after it ends to stdout.',
default=False, action='store_true')
result.add_option('--tool',
- help='Tool to run ART tests with: "r8" (default) or "d8". Ignored if'
- ' "--all_tests" enabled.',
- default=None, choices=["r8", "d8"])
+ help='Tool to run ART tests with: "r8" (default) or "d8" or "r8cf"'
+ ' (r8 w/CF-backend). Ignored if "--all_tests" enabled.',
+ default=None, choices=["r8", "d8", "r8cf"])
result.add_option('--jctf',
- help='Run JCTF tests with: "r8" (default) or "d8".',
+ help='Run JCTF tests with: "r8" (default) or "d8" or "r8cf".',
default=False, action='store_true')
result.add_option('--only-jctf', '--only_jctf',
- help='Run only JCTF tests with: "r8" (default) or "d8".',
+ help='Run only JCTF tests with: "r8" (default) or "d8" or "r8cf".',
default=False, action='store_true')
result.add_option('--jctf-compile-only', '--jctf_compile_only',
help="Don't run, only compile JCTF tests.",
