Rewrite removed types in signature annotations to type Object
Bug: 126592786
Change-Id: Ic0a79e0506d52fda4833c469f2fd9a31063ef892
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 4755dca..95e3156 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -63,6 +63,7 @@
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.CfgPrinter;
+import com.android.tools.r8.utils.CollectionUtils;
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions;
@@ -530,6 +531,9 @@
new SourceFileRewriter(appView.appInfo(), options).run();
timing.end();
+ // Collect the already pruned types before creating a new app info without liveness.
+ Set<DexType> prunedTypes = appView.withLiveness().appInfo().getPrunedTypes();
+
if (!options.mainDexKeepRules.isEmpty()) {
appView.setAppInfo(new AppInfoWithSubtyping(application));
// No need to build a new main dex root set
@@ -593,7 +597,9 @@
appViewWithLiveness.setAppInfo(
appViewWithLiveness
.appInfo()
- .prunedCopyFrom(application, pruner.getRemovedClasses()));
+ .prunedCopyFrom(
+ application,
+ CollectionUtils.mergeSets(prunedTypes, pruner.getRemovedClasses())));
// Print reasons on the application after pruning, so that we reflect the actual result.
if (whyAreYouKeepingConsumer != null) {
diff --git a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
index e55fc6e..fd8508b 100644
--- a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
+++ b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
@@ -161,6 +161,9 @@
public DexType parsedTypeName(String name) {
DexType type = appInfo.dexItemFactory.createType(getDescriptorFromClassBinaryName(name));
type = appView.graphLense().lookupType(type);
+ if (appInfo.wasPruned(type)) {
+ type = appInfo.dexItemFactory.objectType;
+ }
DexString renamedDescriptor = renaming.getOrDefault(type, type.descriptor);
renamedSignature.append(getClassBinaryNameFromDescriptor(renamedDescriptor.toString()));
return type;
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 07023b5..c0cebeb 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -60,6 +60,7 @@
import com.android.tools.r8.shaking.RootSetBuilder.ConsequentRootSet;
import com.android.tools.r8.shaking.RootSetBuilder.IfRuleEvaluator;
import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
+import com.android.tools.r8.utils.CollectionUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.Timing;
@@ -2255,7 +2256,7 @@
this.prunedTypes =
removedClasses == null
? previous.prunedTypes
- : mergeSets(previous.prunedTypes, removedClasses);
+ : CollectionUtils.mergeSets(previous.prunedTypes, removedClasses);
this.switchMaps = previous.switchMaps;
this.ordinalsMaps = previous.ordinalsMaps;
assert Sets.intersection(instanceFieldReads.keySet(), staticFieldReads.keySet()).isEmpty();
@@ -2310,7 +2311,8 @@
// after second tree shaking.
this.callSites = previous.callSites;
this.brokenSuperInvokes = lense.rewriteMethodsConservatively(previous.brokenSuperInvokes);
- this.prunedTypes = rewriteItems(previous.prunedTypes, lense::lookupType);
+ // Don't rewrite pruned types - the removed types are identified by their original name.
+ this.prunedTypes = previous.prunedTypes;
this.mayHaveSideEffects =
rewriteReferenceKeys(previous.mayHaveSideEffects, lense::lookupReference);
this.noSideEffects = rewriteReferenceKeys(previous.noSideEffects, lense::lookupReference);
@@ -2561,13 +2563,6 @@
return instantiatedLambdas.contains(type);
}
- private static <T> Set<T> mergeSets(Collection<T> first, Collection<T> second) {
- ImmutableSet.Builder<T> builder = ImmutableSet.builder();
- builder.addAll(first);
- builder.addAll(second);
- return builder.build();
- }
-
@Override
public boolean hasLiveness() {
return true;
@@ -2608,6 +2603,10 @@
return prunedTypes.contains(type);
}
+ public Set<DexType> getPrunedTypes() {
+ return prunedTypes;
+ }
+
public DexEncodedMethod lookup(Type type, DexMethod target, DexType invocationContext) {
DexType holder = target.getHolder();
if (!holder.isClassType()) {
diff --git a/src/main/java/com/android/tools/r8/utils/CollectionUtils.java b/src/main/java/com/android/tools/r8/utils/CollectionUtils.java
new file mode 100644
index 0000000..6584f77
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/CollectionUtils.java
@@ -0,0 +1,18 @@
+// Copyright (c) 2019, 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.utils;
+
+import com.google.common.collect.ImmutableSet;
+import java.util.Collection;
+import java.util.Set;
+
+public class CollectionUtils {
+ public static <T> Set<T> mergeSets(Collection<T> first, Collection<T> second) {
+ ImmutableSet.Builder<T> builder = ImmutableSet.builder();
+ builder.addAll(first);
+ builder.addAll(second);
+ return builder.build();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/b126592786/B126592786.java b/src/test/java/com/android/tools/r8/naming/b126592786/B126592786.java
new file mode 100644
index 0000000..0971e8c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/b126592786/B126592786.java
@@ -0,0 +1,111 @@
+// Copyright (c) 2019, 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.naming.b126592786;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.FieldSubject;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class B126592786 extends TestBase {
+
+ private final Backend backend;
+ private final boolean minify;
+
+ @Parameterized.Parameters(name = "Backend: {0} minify: {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(Backend.values(), BooleanUtils.values());
+ }
+
+ public B126592786(Backend backend, boolean minify) {
+ this.backend = backend;
+ this.minify = minify;
+ }
+
+ public void runTest(boolean genericTypeLive) throws Exception {
+ Class<?> mainClass = genericTypeLive ? MainGenericTypeLive.class : MainGenericTypeNotLive.class;
+ testForR8(backend)
+ .minification(minify)
+ .addProgramClasses(A.class, GenericType.class, mainClass)
+ .addKeepMainRule(mainClass)
+ .addKeepRules(
+ "-keepclassmembers @" + Marker.class.getTypeName() + " class * {",
+ " <fields>;",
+ "}",
+ "-keepattributes InnerClasses,EnclosingMethod,Signature ")
+ .compile()
+ .inspect(inspector -> {
+ String genericTypeDescriptor = "Ljava/lang/Object;";
+ if (genericTypeLive) {
+ ClassSubject genericType = inspector.clazz(GenericType.class);
+ assertThat(genericType, isRenamed(minify));
+ genericTypeDescriptor = genericType.getFinalDescriptor();
+ }
+ String expectedSignature = "Ljava/util/List<" + genericTypeDescriptor + ">;";
+ FieldSubject list = inspector.clazz(A.class).uniqueFieldWithName("list");
+ assertThat(list, isPresent());
+ assertThat(list.getSignatureAnnotation(), isPresent());
+ assertEquals(expectedSignature, list.getSignatureAnnotationValue());
+ })
+ .run(mainClass)
+ .assertSuccess();
+ }
+
+ @Test
+ public void testGenericClassNotLive() throws Exception {
+ runTest(false);
+ }
+
+ @Test
+ public void testGenericClassLive() throws Exception {
+ runTest(true);
+ }
+}
+
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.RUNTIME)
+@interface Marker {
+}
+
+@Marker
+class A {
+
+ List<GenericType> list;
+}
+
+@Marker
+class GenericType {
+
+}
+
+class MainGenericTypeNotLive {
+
+ public static void main(String[] args) {
+ System.out.println(A.class);
+ }
+}
+
+class MainGenericTypeLive {
+
+ public static void main(String[] args) {
+ System.out.println(A.class);
+ System.out.println(GenericType.class);
+ }
+}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
index 3f8cd81..20da5d0 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
@@ -90,4 +90,9 @@
public String getFinalSignatureAttribute() {
return null;
}
+
+ @Override
+ public AnnotationSubject annotation(String name) {
+ return new AbsentAnnotationSubject();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
index df23da9..0d63e38 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.utils.codeinspector;
+import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexValue;
import com.android.tools.r8.naming.MemberNaming;
@@ -128,6 +129,14 @@
}
@Override
+ public AnnotationSubject annotation(String name) {
+ DexAnnotation annotation = codeInspector.findAnnotation(name, dexField.annotations);
+ return annotation == null
+ ? new AbsentAnnotationSubject()
+ : new FoundAnnotationSubject(annotation);
+ }
+
+ @Override
public String toString() {
return dexField.toSourceString();
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
index dd7cd85..4d7c0fc 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
@@ -288,5 +288,4 @@
? new AbsentAnnotationSubject()
: new FoundAnnotationSubject(annotation);
}
-
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
index af1fe14..a5cca23 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
@@ -4,6 +4,9 @@
package com.android.tools.r8.utils.codeinspector;
+import com.android.tools.r8.graph.DexAnnotationElement;
+import com.android.tools.r8.graph.DexValue;
+import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.naming.MemberNaming.Signature;
public abstract class MemberSubject extends Subject {
@@ -48,6 +51,34 @@
return finalSignature == null ? null : finalSignature.name;
}
+ public abstract AnnotationSubject annotation(String name);
+
+ public AnnotationSubject getSignatureAnnotation() {
+ return annotation("dalvik.annotation.Signature");
+ }
+
+ public String getSignatureAnnotationValue() {
+ AnnotationSubject annotation = getSignatureAnnotation();
+ if (!annotation.isPresent()) {
+ return null;
+ }
+
+ assert annotation.getAnnotation().elements.length == 1;
+ DexAnnotationElement element = annotation.getAnnotation().elements[0];
+ assert element.name.toString().equals("value");
+ assert element.value instanceof DexValue.DexValueArray;
+ DexValue.DexValueArray array = (DexValue.DexValueArray) element.value;
+ StringBuilder builder = new StringBuilder();
+ for (DexValue value : array.getValues()) {
+ if (value instanceof DexValueString) {
+ builder.append(((DexValueString) value).value);
+ } else {
+ builder.append(value.toString());
+ }
+ }
+ return builder.toString();
+ }
+
public FieldSubject asFieldSubject() {
return null;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
index 1575372..8fb4001 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
@@ -72,6 +72,4 @@
public boolean isMethodSubject() {
return true;
}
-
- public abstract AnnotationSubject annotation(String name);
}
