Soft-pin items retained by reflective identification
Change-Id: I5fe708e8689ba20be5547d9342edc6313521008f
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 1f58402..6be7520 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -411,6 +411,11 @@
}
public static ProgramMethod asProgramMethodOrNull(
+ DexEncodedMethod method, DexProgramClass holder) {
+ return method != null ? method.asProgramMethod(holder) : null;
+ }
+
+ public static ProgramMethod asProgramMethodOrNull(
DexEncodedMethod method, DexDefinitionSupplier definitions) {
return method != null ? method.asProgramMethod(definitions) : null;
}
diff --git a/src/main/java/com/android/tools/r8/graph/MethodCollection.java b/src/main/java/com/android/tools/r8/graph/MethodCollection.java
index 3dc2b58..4a403c4 100644
--- a/src/main/java/com/android/tools/r8/graph/MethodCollection.java
+++ b/src/main/java/com/android/tools/r8/graph/MethodCollection.java
@@ -167,6 +167,10 @@
return backing.getMethod(methodProto, methodName);
}
+ public final DexEncodedMethod getMethod(DexMethodSignature method) {
+ return getMethod(method.getProto(), method.getName());
+ }
+
public DexEncodedMethod getMethod(Predicate<DexEncodedMethod> predicate) {
DexEncodedMethod result = backing.getDirectMethod(predicate);
return result != null ? result : backing.getVirtualMethod(predicate);
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 46732db..7a0c559 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking;
+import static com.android.tools.r8.graph.DexEncodedMethod.asProgramMethodOrNull;
import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
import static com.android.tools.r8.graph.FieldAccessInfoImpl.MISSING_FIELD_ACCESS_INFO;
import static com.android.tools.r8.ir.desugar.LambdaDescriptor.isLambdaMetafactoryMethod;
@@ -48,6 +49,7 @@
import com.android.tools.r8.graph.DexMember;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexMethodHandle;
+import com.android.tools.r8.graph.DexMethodSignature;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexReference;
@@ -142,6 +144,7 @@
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.Visibility;
import com.android.tools.r8.utils.WorkList;
+import com.android.tools.r8.utils.collections.DexMethodSignatureSet;
import com.android.tools.r8.utils.collections.ProgramFieldSet;
import com.android.tools.r8.utils.collections.ProgramMethodMap;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
@@ -764,12 +767,14 @@
return getProgramClassOrNull(member.getHolderType(), context);
}
+ private DexClass getClassOrNullFromReflectiveAccess(DexType type, ProgramDefinition context) {
+ // To avoid that we report reflectively accessed types as missing.
+ return definitionFor(type, context, this::recordNonProgramClass, this::ignoreMissingClass);
+ }
+
private DexProgramClass getProgramClassOrNullFromReflectiveAccess(
DexType type, ProgramDefinition context) {
- // To avoid that we report reflectively accessed types as missing.
- DexClass clazz =
- definitionFor(type, context, this::recordNonProgramClass, this::ignoreMissingClass);
- return clazz != null && clazz.isProgramClass() ? clazz.asProgramClass() : null;
+ return asProgramClassOrNull(getClassOrNullFromReflectiveAccess(type, context));
}
private void warnIfLibraryTypeInheritsFromProgramType(DexLibraryClass clazz) {
@@ -858,6 +863,8 @@
}
if (clazz.isExternalizable(appView)) {
workList.enqueueMarkMethodLiveAction(defaultInitializer, defaultInitializer, witness);
+ applyMinimumKeepInfoWhenLiveOrTargeted(
+ defaultInitializer, KeepMethodInfo.newEmptyJoiner().disallowOptimization());
}
}
}
@@ -899,6 +906,9 @@
}
if (clazz.hasDefaultInitializer()) {
workList.enqueueMarkMethodLiveAction(clazz.getProgramDefaultInitializer(), clazz, reason);
+ applyMinimumKeepInfoWhenLiveOrTargeted(
+ clazz.getProgramDefaultInitializer(),
+ KeepMethodInfo.newEmptyJoiner().disallowOptimization());
}
}
@@ -3232,9 +3242,7 @@
applyMinimumKeepInfoWhenLive(clazz, preconditionEvent, minimumKeepInfo),
(field, minimumKeepInfo) ->
applyMinimumKeepInfoWhenLive(field, preconditionEvent, minimumKeepInfo),
- (method, minimumKeepInfo) ->
- applyMinimumKeepInfoWhenLiveOrTargeted(
- method, preconditionEvent, minimumKeepInfo));
+ this::applyMinimumKeepInfoWhenLiveOrTargeted);
}
enqueueAllIfNotShrinking();
trace(executorService, timing);
@@ -3373,8 +3381,14 @@
private void applyMinimumKeepInfoWhenLiveOrTargeted(
ProgramMethod method,
- EnqueuerEvent preconditionEvent,
KeepMethodInfo.Joiner minimumKeepInfo) {
+ applyMinimumKeepInfoWhenLiveOrTargeted(method, minimumKeepInfo, EnqueuerEvent.unconditional());
+ }
+
+ private void applyMinimumKeepInfoWhenLiveOrTargeted(
+ ProgramMethod method,
+ KeepMethodInfo.Joiner minimumKeepInfo,
+ EnqueuerEvent preconditionEvent) {
if (liveMethods.contains(method) || targetedMethods.contains(method)) {
keepInfo.joinMethod(method, info -> info.merge(minimumKeepInfo));
} else {
@@ -3405,7 +3419,7 @@
ProgramMethod method,
KeepMethodInfo.Joiner minimumKeepInfo) {
if (isPreconditionForMinimumKeepInfoSatisfied(preconditionEvent)) {
- applyMinimumKeepInfoWhenLiveOrTargeted(method, preconditionEvent, minimumKeepInfo);
+ applyMinimumKeepInfoWhenLiveOrTargeted(method, minimumKeepInfo, preconditionEvent);
} else {
dependentMinimumKeepInfo
.getOrCreateMinimumKeepInfoFor(preconditionEvent)
@@ -3429,7 +3443,7 @@
applyMinimumKeepInfoWhenLive(field, preconditionEvent, minimumKeepInfoForField),
(method, minimumKeepInfoForMethod) ->
applyMinimumKeepInfoWhenLiveOrTargeted(
- method, preconditionEvent, minimumKeepInfoForMethod));
+ method, minimumKeepInfoForMethod, preconditionEvent));
}
}
@@ -3480,10 +3494,6 @@
assert old == null || old == clazz;
}
- public void addLiveMethods(Iterable<ProgramMethod> methods) {
- methods.forEach(this::addLiveMethod);
- }
-
public void addLiveMethod(ProgramMethod method) {
DexMethod signature = method.getDefinition().getReference();
ProgramMethod old = liveMethods.put(signature, method);
@@ -3500,18 +3510,6 @@
newInterfaces.add(newInterface);
}
- void addLiveMethodWithKeepAction(
- ProgramMethod method, Consumer<KeepMethodInfo.Joiner> keepAction) {
- addLiveMethod(method);
- liveMethodsWithKeepActions.add(new Pair<>(method, keepAction));
- }
-
- public ProgramMethodSet getLiveMethods() {
- ProgramMethodSet set = ProgramMethodSet.create();
- liveMethods.values().forEach(set::add);
- return set;
- }
-
public void addMinimumKeepInfo(ProgramMethod method, Consumer<KeepMethodInfo.Joiner> consumer) {
consumer.accept(
minimumKeepInfo.computeIfAbsent(method, ignoreKey(KeepMethodInfo::newEmptyJoiner)));
@@ -3543,8 +3541,7 @@
minimumKeepInfo.forEach(
(method, minimumKeepInfoForMethod) ->
- enqueuer.applyMinimumKeepInfoWhenLiveOrTargeted(
- method, UnconditionalKeepInfoEvent.get(), minimumKeepInfoForMethod));
+ enqueuer.applyMinimumKeepInfoWhenLiveOrTargeted(method, minimumKeepInfoForMethod));
}
}
@@ -4531,18 +4528,19 @@
if (clazz == null) {
return;
}
- DexEncodedMethod targetedMethodDefinition = clazz.lookupMethod(targetedMethodReference);
- if (targetedMethodDefinition == null) {
+ ProgramMethod targetedMethod = clazz.lookupProgramMethod(targetedMethodReference);
+ if (targetedMethod == null) {
return;
}
- ProgramMethod targetedMethod = new ProgramMethod(clazz, targetedMethodDefinition);
KeepReason reason = KeepReason.reflectiveUseIn(method);
- if (targetedMethodDefinition.isStatic() || targetedMethodDefinition.isInstanceInitializer()) {
+ if (targetedMethod.getDefinition().belongsToDirectPool()) {
markMethodAsTargeted(targetedMethod, reason);
markDirectStaticOrConstructorMethodAsLive(targetedMethod, reason);
} else {
markVirtualMethodAsLive(targetedMethod, reason);
}
+ applyMinimumKeepInfoWhenLiveOrTargeted(
+ targetedMethod, KeepMethodInfo.newEmptyJoiner().disallowOptimization());
}
}
@@ -4572,6 +4570,8 @@
markClassAsInstantiatedWithReason(clazz, reason);
markMethodAsTargeted(defaultInitializer, reason);
markDirectStaticOrConstructorMethodAsLive(defaultInitializer, reason);
+ applyMinimumKeepInfoWhenLiveOrTargeted(
+ defaultInitializer, KeepMethodInfo.newEmptyJoiner().disallowOptimization());
}
}
@@ -4673,6 +4673,8 @@
markClassAsInstantiatedWithReason(clazz, reason);
markMethodAsTargeted(initializer, reason);
markDirectStaticOrConstructorMethodAsLive(initializer, reason);
+ applyMinimumKeepInfoWhenLiveOrTargeted(
+ initializer, KeepMethodInfo.newEmptyJoiner().disallowOptimization());
}
}
@@ -4795,6 +4797,7 @@
DexType serviceType, ProgramMethod context, KeepReason reason) {
List<DexType> serviceImplementationTypes =
appView.appServices().serviceImplementationsFor(serviceType);
+ DexMethodSignatureSet serviceMethods = getServiceMethods(serviceType, context);
for (DexType serviceImplementationType : serviceImplementationTypes) {
if (!serviceImplementationType.isClassType()) {
// Should never happen.
@@ -4803,12 +4806,46 @@
DexProgramClass serviceImplementationClass =
getProgramClassOrNull(serviceImplementationType, context);
- if (serviceImplementationClass != null && serviceImplementationClass.isProgramClass()) {
- markClassAsInstantiatedWithReason(serviceImplementationClass, reason);
+ if (serviceImplementationClass == null) {
+ continue;
+ }
+
+ markClassAsInstantiatedWithReason(serviceImplementationClass, reason);
+
+ ProgramMethod defaultInitializer = serviceImplementationClass.getProgramDefaultInitializer();
+ if (defaultInitializer != null) {
+ applyMinimumKeepInfoWhenLiveOrTargeted(
+ defaultInitializer, KeepMethodInfo.newEmptyJoiner().disallowOptimization());
+ }
+
+ for (DexMethodSignature serviceMethod : serviceMethods) {
+ ProgramMethod serviceImplementationMethod =
+ asProgramMethodOrNull(
+ serviceImplementationClass.getMethodCollection().getMethod(serviceMethod),
+ serviceImplementationClass);
+ if (serviceImplementationMethod != null) {
+ applyMinimumKeepInfoWhenLiveOrTargeted(
+ serviceImplementationMethod, KeepMethodInfo.newEmptyJoiner().disallowOptimization());
+ }
}
}
}
+ private DexMethodSignatureSet getServiceMethods(DexType serviceType, ProgramMethod context) {
+ DexMethodSignatureSet serviceMethods = DexMethodSignatureSet.create();
+ WorkList<DexType> serviceTypes = WorkList.newIdentityWorkList(serviceType);
+ while (serviceTypes.hasNext()) {
+ DexType current = serviceTypes.next();
+ DexClass clazz = getClassOrNullFromReflectiveAccess(current, context);
+ if (clazz == null) {
+ continue;
+ }
+ clazz.forEachClassMethodMatching(DexEncodedMethod::belongsToVirtualPool, serviceMethods::add);
+ serviceTypes.addIfNotSeen(clazz.getInterfaces());
+ }
+ return serviceMethods;
+ }
+
private static class SetWithReportedReason<T> {
private final Set<T> items = Sets.newIdentityHashSet();
diff --git a/src/main/java/com/android/tools/r8/shaking/EnqueuerEvent.java b/src/main/java/com/android/tools/r8/shaking/EnqueuerEvent.java
index 8a3439d..93d5f60 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerEvent.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerEvent.java
@@ -12,6 +12,10 @@
public abstract class EnqueuerEvent {
+ public static UnconditionalKeepInfoEvent unconditional() {
+ return UnconditionalKeepInfoEvent.get();
+ }
+
public DexDefinition getDefinition(DexDefinitionSupplier definitions) {
return null;
}
diff --git a/src/test/java/com/android/tools/r8/naming/b124357885/B124357885Test.java b/src/test/java/com/android/tools/r8/naming/b124357885/B124357885Test.java
index a3ae2e9..7bf4064 100644
--- a/src/test/java/com/android/tools/r8/naming/b124357885/B124357885Test.java
+++ b/src/test/java/com/android/tools/r8/naming/b124357885/B124357885Test.java
@@ -11,6 +11,8 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
+import com.android.tools.r8.KeepUnusedReturnValue;
+import com.android.tools.r8.NeverInline;
import com.android.tools.r8.R8TestCompileResult;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -53,40 +55,53 @@
public void test() throws Exception {
R8TestCompileResult compileResult =
testForR8Compat(parameters.getBackend())
- .addProgramClasses(Main.class, Service.class, Foo.class, FooImpl.class)
+ .addProgramClasses(Main.class, Foo.class, FooImpl.class)
.addKeepMainRule(Main.class)
.addKeepAttributes(
ProguardKeepAttributes.SIGNATURE,
ProguardKeepAttributes.INNER_CLASSES,
ProguardKeepAttributes.ENCLOSING_METHOD)
+ .applyIf(
+ minification,
+ testBuilder ->
+ testBuilder.addKeepRules(
+ "-keep,allowoptimization,allowshrinking class "
+ + Main.class.getTypeName()
+ + " { *** test(); }"))
+ .enableInliningAnnotations()
+ .enableKeepUnusedReturnValueAnnotations()
.minification(minification)
.setMinApi(parameters.getApiLevel())
.compile()
.inspect(
inspector -> {
- assertThat(inspector.clazz(Main.class), isPresentAndNotRenamed());
+ ClassSubject mainClass = inspector.clazz(Main.class);
+ assertThat(mainClass, isPresentAndNotRenamed());
assertThat(inspector.clazz(Foo.class), not(isPresent()));
assertThat(inspector.clazz(FooImpl.class), isPresentAndRenamed(minification));
- ClassSubject serviceClass = inspector.clazz(Service.class);
- assertThat(serviceClass, isPresentAndRenamed(minification));
- // TODO(124477502): Using uniqueMethodWithName("fooList") does not work.
- assertEquals(1, serviceClass.allMethods().size());
- MethodSubject fooList = serviceClass.allMethods().get(0);
- assertThat(fooList, isPresent());
+ // TODO(124477502): Using uniqueMethodWithName("test") does not work.
+ MethodSubject testMethod =
+ mainClass.uniqueMethodThatMatches(
+ method ->
+ method.isStatic()
+ && !method.getMethod().getName().toString().equals("main"));
+ assertThat(testMethod, isPresent());
checkSignature(
- inspector, fooList.asFoundMethodSubject().getFinalSignatureAttribute());
+ inspector, testMethod.asFoundMethodSubject().getFinalSignatureAttribute());
});
String fooImplFinalName = compileResult.inspector().clazz(FooImpl.class).getFinalName();
compileResult
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutput(StringUtils.lines(fooImplFinalName, fooImplFinalName));
+ .assertSuccessWithOutput(
+ StringUtils.lines(fooImplFinalName, fooImplFinalName, "Hello world!"));
}
public static class Main {
public static void main(String... args) throws Exception {
- Method method = Service.class.getMethod("fooList");
+ String methodName = System.currentTimeMillis() >= 0 ? "test" : null;
+ Method method = Main.class.getDeclaredMethod(methodName);
try {
ParameterizedType type = (ParameterizedType) method.getGenericReturnType();
@@ -99,11 +114,17 @@
// Convince R8 we only use subtypes to get class merging of Foo into FooImpl.
Foo<String> foo = new FooImpl<>();
System.out.println(foo.getClass().getCanonicalName());
- }
- }
- interface Service {
- Foo<String> fooList();
+ // Ensure test() remains in output.
+ test();
+ }
+
+ @NeverInline
+ @KeepUnusedReturnValue
+ static Foo<String> test() {
+ System.out.println("Hello world!");
+ return new FooImpl<>();
+ }
}
interface Foo<T> {}
