Only remove visibility bridges
Change-Id: Iab8b753c1fd340b2c84c935fb36a825794abe87e
diff --git a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
index e4892f2..6c3e715 100644
--- a/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
+++ b/src/main/java/com/android/tools/r8/optimize/VisibilityBridgeRemover.java
@@ -35,7 +35,7 @@
DexMethod target = targetExtractor.getTarget();
InvokeKind kind = targetExtractor.getKind();
if (target != null &&
- target.proto.parameters.values.length == method.method.proto.parameters.values.length) {
+ target.proto == method.method.proto) {
assert !method.accessFlags.isPrivate() && !method.accessFlags.isConstructor();
if (kind == InvokeKind.SUPER) {
// This is a visibility forward, so check for the direct target.
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 075c569..28d4872 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -87,17 +87,32 @@
/**
* Compile an application with R8.
*/
+ protected AndroidApp compileWithR8(List<Class> classes)
+ throws CompilationException, ProguardRuleParserException, ExecutionException, IOException {
+ R8Command command = ToolHelper.prepareR8CommandBuilder(readClasses(classes)).build();
+ return ToolHelper.runR8(command);
+ }
+
+ /**
+ * Compile an application with R8.
+ */
protected AndroidApp compileWithR8(AndroidApp app, Consumer<InternalOptions> optionsConsumer)
throws CompilationException, ProguardRuleParserException, ExecutionException, IOException {
- R8Command command =
- ToolHelper.prepareR8CommandBuilder(app)
- .build();
+ R8Command command = ToolHelper.prepareR8CommandBuilder(app).build();
return ToolHelper.runR8(command, optionsConsumer);
}
/**
* Compile an application with R8 using the supplied proguard configuration.
*/
+ protected AndroidApp compileWithR8(List<Class> classes, String proguardConfig)
+ throws CompilationException, ProguardRuleParserException, ExecutionException, IOException {
+ return compileWithR8(readClasses(classes), writeTextToTempFile(proguardConfig));
+ }
+
+ /**
+ * Compile an application with R8 using the supplied proguard configuration.
+ */
protected AndroidApp compileWithR8(List<Class> classes, Path proguardConfig)
throws CompilationException, ProguardRuleParserException, ExecutionException, IOException {
return compileWithR8(readClasses(classes), proguardConfig);
@@ -129,6 +144,28 @@
}
/**
+ * Generate a Proguard configuration for keeping the "public static void main(String[])" method
+ * of the specified class.
+ */
+ public String keepMainProguardConfiguration(Class clazz) {
+ return "-keep public class " + clazz.getCanonicalName() + " {\n"
+ + " public static void main(java.lang.String[]);\n"
+ + "}\n";
+ }
+
+ /**
+ * Generate a Proguard configuration for keeping the "public static void main(String[])" method
+ * of the specified class and specify if -allowaccessmodification and -dontobfuscate are added
+ * as well.
+ */
+ public String keepMainProguardConfiguration(
+ Class clazz, boolean allowaccessmodification, boolean obfuscate) {
+ return keepMainProguardConfiguration(clazz)
+ + (allowaccessmodification ? "-allowaccessmodification\n" : "")
+ + (obfuscate ? "" : "-dontobfuscate\n");
+ }
+
+ /**
* Run application on Art with the specified main class.
*/
protected String runOnArt(AndroidApp app, Class mainClass) throws IOException {
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/DataAdapter.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/DataAdapter.java
new file mode 100644
index 0000000..eae4025
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/DataAdapter.java
@@ -0,0 +1,13 @@
+// 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.bridgeremoval.bridgestokeep;
+
+public interface DataAdapter {
+
+ interface Observer extends ObservableList.Observer {
+ }
+
+ void registerObserver(Observer observer);
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/KeepNonVisibilityBridgeMethodsTest.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/KeepNonVisibilityBridgeMethodsTest.java
new file mode 100644
index 0000000..7e5b06a
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/KeepNonVisibilityBridgeMethodsTest.java
@@ -0,0 +1,54 @@
+// 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.bridgeremoval.bridgestokeep;
+
+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.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.lang.reflect.Method;
+import java.util.List;
+import org.junit.Test;
+
+public class KeepNonVisibilityBridgeMethodsTest extends TestBase {
+
+ private String keepMainAllowAccessModification(Class clazz, boolean obfuscate) {
+ return "-keep public class " + clazz.getCanonicalName() + " {\n"
+ + " public static void main(java.lang.String[]);\n"
+ + "}\n"
+ + "-allowaccessmodification\n"
+ + (obfuscate ? "" : "-dontobfuscate\n");
+ }
+
+ private void run(boolean obfuscate) throws Exception {
+ List<Class> classes = ImmutableList.of(
+ DataAdapter.class,
+ SimpleDataAdapter.class,
+ ObservableList.class,
+ SimpleObservableList.class,
+ Main.class);
+ String proguardConfig = keepMainAllowAccessModification(Main.class, obfuscate);
+ DexInspector inspector = new DexInspector(compileWithR8(classes, proguardConfig));
+
+ // The bridge for registerObserver cannot be removed.
+ Method registerObserver =
+ SimpleDataAdapter.class.getMethod("registerObserver", DataAdapter.Observer.class);
+ MethodSubject subject = inspector.method(registerObserver);
+ assertTrue(subject.isPresent());
+ assertTrue(subject.isBridge());
+ }
+
+ @Test
+ public void testWithObfuscation() throws Exception {
+ run(true);
+ }
+
+ @Test
+ public void testWithoutObfuscation() throws Exception {
+ run(false);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/Main.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/Main.java
new file mode 100644
index 0000000..4d80056
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/Main.java
@@ -0,0 +1,17 @@
+// 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.bridgeremoval.bridgestokeep;
+
+// Reduced test case from code where removal of bridge methods caused failure.
+public class Main {
+
+ public static void registerObserver(DataAdapter dataAdapter) {
+ dataAdapter.registerObserver(null);
+ }
+
+ public static void main(String[] args) {
+ registerObserver(new SimpleDataAdapter());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/ObservableList.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/ObservableList.java
new file mode 100644
index 0000000..fa8fdce
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/ObservableList.java
@@ -0,0 +1,15 @@
+// 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.bridgeremoval.bridgestokeep;
+
+import com.android.tools.r8.bridgeremoval.bridgestokeep.ObservableList.Observer;
+
+public interface ObservableList<O extends Observer> {
+
+ interface Observer {
+ }
+
+ void registerObserver(O observer);
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/SimpleDataAdapter.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/SimpleDataAdapter.java
new file mode 100644
index 0000000..148a61e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/SimpleDataAdapter.java
@@ -0,0 +1,17 @@
+// 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.bridgeremoval.bridgestokeep;
+
+public class SimpleDataAdapter
+ extends SimpleObservableList<DataAdapter.Observer>
+ implements DataAdapter {
+
+ public SimpleDataAdapter() {
+ }
+
+ // This class has no implementation of method registerObserver(DataAdapter.Observer observer)
+ // from interface DataAdapter. There is one in SimpleObservableList<DataAdapter.Observer> so
+ // javac inserts a bridge with signature registerObserver(DataAdapter.Observer observer).
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/SimpleObservableList.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/SimpleObservableList.java
new file mode 100644
index 0000000..5fa667b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestokeep/SimpleObservableList.java
@@ -0,0 +1,15 @@
+// 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.bridgeremoval.bridgestokeep;
+
+import com.android.tools.r8.bridgeremoval.bridgestokeep.ObservableList.Observer;
+
+public class SimpleObservableList<O extends Observer>
+ implements ObservableList<O> {
+
+ @Override
+ public void registerObserver(O observer) {
+ }
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/Main.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/Main.java
new file mode 100644
index 0000000..4c8ac31
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/Main.java
@@ -0,0 +1,13 @@
+// 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.bridgeremoval.bridgestoremove;
+
+public class Main {
+
+ public static void main(String[] args) {
+ (new Outer()).create().method();
+ (new Outer.StaticSubClass()).method();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/Outer.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/Outer.java
new file mode 100644
index 0000000..2e1b1bf
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/Outer.java
@@ -0,0 +1,26 @@
+// 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.bridgeremoval.bridgestoremove;
+
+public class Outer {
+
+ class SuperClass {
+ public void method() { }
+ }
+
+ // As SuperClass is package private SubClass will have a bridge method for "method".
+ public class SubClass extends SuperClass { }
+
+ public SubClass create() {
+ return new SubClass();
+ }
+
+ static class StaticSuperClass {
+ public void method() { }
+ }
+
+ // As SuperClass is package private SubClass will have a bridge method for "method".
+ public static class StaticSubClass extends StaticSuperClass { }
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/RemoveVisibilityBridgeMethodsTest.java b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/RemoveVisibilityBridgeMethodsTest.java
new file mode 100644
index 0000000..c33d5b4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/bridgeremoval/bridgestoremove/RemoveVisibilityBridgeMethodsTest.java
@@ -0,0 +1,41 @@
+// 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.bridgeremoval.bridgestoremove;
+
+import static org.junit.Assert.assertFalse;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.DexInspector;
+import com.google.common.collect.ImmutableList;
+import java.lang.reflect.Method;
+import java.util.List;
+import org.junit.Test;
+
+public class RemoveVisibilityBridgeMethodsTest extends TestBase {
+
+ private void run(boolean obfuscate) throws Exception {
+ List<Class> classes = ImmutableList.of(
+ Outer.class,
+ Main.class);
+ String proguardConfig = keepMainProguardConfiguration(Main.class, true, obfuscate);
+ DexInspector inspector = new DexInspector(compileWithR8(classes, proguardConfig));
+
+ List<Method> removedMethods = ImmutableList.of(
+ Outer.SubClass.class.getMethod("method"),
+ Outer.StaticSubClass.class.getMethod("method"));
+
+ removedMethods.forEach(method -> assertFalse(inspector.method(method).isPresent()));
+ }
+
+ @Test
+ public void testWithObfuscation() throws Exception {
+ run(true);
+ }
+
+ @Test
+ public void testWithoutObfuscation() throws Exception {
+ run(false);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java b/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
index 0e24d9b..a02a186 100644
--- a/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
@@ -64,12 +64,6 @@
return result;
}
- private String keepMain(Class clazz) {
- return "-keep public class " + clazz.getCanonicalName() + " {\n"
- + " public static void main(java.lang.String[]);\n"
- + "}";
- }
-
private class Result {
final DexInspector inspector;
final Set<String> classesAfterProguard;
@@ -135,7 +129,7 @@
allClasses.add(mainClass);
Path proguardConfig = writeTextToTempFile(
- keepMain(mainClass),
+ keepMainProguardConfiguration(mainClass),
"-keepclasseswithmembers class * { ",
" <fields>; ",
" native <methods>; ",
@@ -158,7 +152,7 @@
public void testKeepClassesWithMembers() throws Exception {
for (Class mainClass : mainClasses) {
Path proguardConfig = writeTextToTempFile(
- keepMain(mainClass),
+ keepMainProguardConfiguration(mainClass),
"-keepclasseswithmembers,includedescriptorclasses class * { ",
" <fields>; ",
" native <methods>; ",
@@ -181,7 +175,7 @@
public void testKeepClassMembers() throws Exception {
for (Class mainClass : mainClasses) {
Path proguardConfig = writeTextToTempFile(
- keepMain(mainClass),
+ keepMainProguardConfiguration(mainClass),
"-keepclassmembers,includedescriptorclasses class * { ",
" <fields>; ",
" native <methods>; ",
@@ -204,7 +198,7 @@
public void testKeepClassMemberNames() throws Exception {
for (Class mainClass : mainClasses) {
Path proguardConfig = writeTextToTempFile(
- keepMain(mainClass),
+ keepMainProguardConfiguration(mainClass),
// same as -keepclassmembers,allowshrinking,includedescriptorclasses
"-keepclassmembernames,includedescriptorclasses class * { ",
" <fields>; ",
diff --git a/src/test/java/com/android/tools/r8/utils/DexInspector.java b/src/test/java/com/android/tools/r8/utils/DexInspector.java
index 4e661ba..61c3c14 100644
--- a/src/test/java/com/android/tools/r8/utils/DexInspector.java
+++ b/src/test/java/com/android/tools/r8/utils/DexInspector.java
@@ -66,8 +66,10 @@
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
+import java.lang.reflect.Method;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
@@ -158,6 +160,9 @@
}
}
+ public ClassSubject clazz(Class clazz) {
+ return clazz(clazz.getTypeName());
+ }
public ClassSubject clazz(String name) {
ClassNaming naming = null;
@@ -188,6 +193,14 @@
}, inspection);
}
+ public MethodSubject method(Method method) {
+ ClassSubject clazz = clazz(method.getDeclaringClass());
+ if (!clazz.isPresent()) {
+ return new AbsentMethodSubject();
+ }
+ return clazz.method(method);
+ }
+
private String getObfuscatedTypeName(String originalTypeName) {
String obfuscatedType = null;
if (mapping != null) {
@@ -244,6 +257,14 @@
public abstract void forAllMethods(Consumer<FoundMethodSubject> inspection);
+ public MethodSubject method(Method method) {
+ List<String> parameters = new ArrayList<>();
+ for (Class<?> parameterType : method.getParameterTypes()) {
+ parameters.add(parameterType.getTypeName());
+ }
+ return method(method.getReturnType().getTypeName(), method.getName(), parameters);
+ }
+
public abstract MethodSubject method(String returnType, String name, List<String> parameters);
public MethodSubject method(MethodSignature signature) {
@@ -468,6 +489,11 @@
public boolean isRenamed() {
return naming == null || !getFinalDescriptor().equals(getOriginalDescriptor());
}
+
+ @Override
+ public String toString() {
+ return dexClass.toSourceString();
+ }
}
public abstract class MemberSubject extends Subject {
@@ -489,6 +515,8 @@
public abstract boolean isAbstract();
+ public abstract boolean isBridge();
+
public abstract DexEncodedMethod getMethod();
public Iterator<InstructionSubject> iterateInstructions() {
@@ -541,6 +569,11 @@
}
@Override
+ public boolean isBridge() {
+ return false;
+ }
+
+ @Override
public DexEncodedMethod getMethod() {
return null;
}
@@ -602,6 +635,11 @@
}
@Override
+ public boolean isBridge() {
+ return dexMethod.accessFlags.isBridge();
+ }
+
+ @Override
public DexEncodedMethod getMethod() {
return dexMethod;
}
@@ -629,6 +667,11 @@
Predicate<InstructionSubject> filter) {
return new FilteredInstructionIterator<>(this, filter);
}
+
+ @Override
+ public String toString() {
+ return dexMethod.toSourceString();
+ }
}
public abstract class FieldSubject extends MemberSubject {
