Allow using non-public static fields for triggering clinit
Bug: 151281085
Change-Id: If0dac5965cb8c4aeb7cff6bda5f431477476b92a
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
index a6c8186..8a21e7b 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -138,10 +138,18 @@
return accessFlags.isStatic();
}
+ public boolean isPackagePrivate() {
+ return accessFlags.isPackagePrivate();
+ }
+
public boolean isPrivate() {
return accessFlags.isPrivate();
}
+ public boolean isProtected() {
+ return accessFlags.isProtected();
+ }
+
public boolean isPublic() {
return accessFlags.isPublic();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/AssumeDynamicTypeRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/AssumeDynamicTypeRemover.java
index 0ebe028..6bb9b03 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/AssumeDynamicTypeRemover.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/AssumeDynamicTypeRemover.java
@@ -55,9 +55,6 @@
public void markUsersForRemoval(Value value) {
for (Instruction user : value.aliasedUsers()) {
if (user.isAssumeDynamicType()) {
- assert value.numberOfAllUsers() == 1
- : "Expected value flowing into Assume<DynamicTypeAssumption> instruction to have a "
- + "unique user.";
markForRemoval(user.asAssumeDynamicType());
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index ed4c3e3..7df2fbc 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -44,6 +44,7 @@
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.PredicateSet;
import com.android.tools.r8.utils.SetUtils;
+import com.android.tools.r8.utils.Visibility;
import com.android.tools.r8.utils.WorkList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
@@ -164,8 +165,11 @@
public final Set<DexType> neverMerge;
/** Set of const-class references. */
public final Set<DexType> constClassReferences;
- /** Set of init-class references. */
- public final Set<DexType> initClassReferences;
+ /**
+ * A map from seen init-class references to the minimum required visibility of the corresponding
+ * static field.
+ */
+ public final Map<DexType, Visibility> initClassReferences;
/**
* All methods and fields whose value *must* never be propagated due to a configuration directive.
* (testing only).
@@ -231,7 +235,7 @@
EnumValueInfoMapCollection enumValueInfoMaps,
Set<DexType> instantiatedLambdas,
Set<DexType> constClassReferences,
- Set<DexType> initClassReferences) {
+ Map<DexType, Visibility> initClassReferences) {
super(application);
this.missingTypes = missingTypes;
this.liveTypes = liveTypes;
@@ -316,7 +320,7 @@
EnumValueInfoMapCollection enumValueInfoMaps,
Set<DexType> instantiatedLambdas,
Set<DexType> constClassReferences,
- Set<DexType> initClassReferences) {
+ Map<DexType, Visibility> initClassReferences) {
super(appInfoWithSubtyping);
this.missingTypes = missingTypes;
this.liveTypes = liveTypes;
@@ -1077,7 +1081,7 @@
enumValueInfoMaps.rewrittenWithLens(lens),
rewriteItems(instantiatedLambdas, lens::lookupType),
rewriteItems(constClassReferences, lens::lookupType),
- rewriteItems(initClassReferences, lens::lookupType));
+ rewriteReferenceKeys(initClassReferences, lens::lookupType));
}
/**
diff --git a/src/main/java/com/android/tools/r8/shaking/ClassInitFieldSynthesizer.java b/src/main/java/com/android/tools/r8/shaking/ClassInitFieldSynthesizer.java
index a6231aa..60bb9d7 100644
--- a/src/main/java/com/android/tools/r8/shaking/ClassInitFieldSynthesizer.java
+++ b/src/main/java/com/android/tools/r8/shaking/ClassInitFieldSynthesizer.java
@@ -7,6 +7,7 @@
import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
import com.android.tools.r8.dex.Constants;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexEncodedField;
@@ -16,6 +17,7 @@
import com.android.tools.r8.graph.FieldAccessFlags;
import com.android.tools.r8.graph.InitClassLens;
import com.android.tools.r8.utils.ThreadUtils;
+import com.android.tools.r8.utils.Visibility;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
@@ -36,7 +38,7 @@
appView.setInitClassLens(lensBuilder.build());
}
- private void synthesizeClassInitField(DexType type) {
+ private void synthesizeClassInitField(DexType type, Visibility minimumRequiredVisibility) {
DexProgramClass clazz = asProgramClassOrNull(appView.definitionFor(type));
if (clazz == null) {
assert false;
@@ -46,12 +48,12 @@
DexEncodedField encodedClinitField = null;
for (DexEncodedField staticField : clazz.staticFields()) {
// We need to field to be accessible from the contexts in which it is accessed.
- if (!staticField.isPublic()) {
+ if (!isMinimumRequiredVisibility(staticField, minimumRequiredVisibility)) {
continue;
}
// When compiling for dex, we can't use wide fields since we've only allocated a single
// register for the out-value of each ClassInit instruction
- if (appView.options().isGeneratingDex() && staticField.field.type.isWideType()) {
+ if (staticField.field.type.isWideType()) {
continue;
}
encodedClinitField = staticField;
@@ -74,4 +76,21 @@
}
lensBuilder.map(type, encodedClinitField.field);
}
+
+ private boolean isMinimumRequiredVisibility(
+ DexEncodedField field, Visibility minimumRequiredVisibility) {
+ if (field.isPublic()) {
+ return true;
+ }
+ switch (minimumRequiredVisibility) {
+ case PROTECTED:
+ return field.isProtected();
+ case PACKAGE_PRIVATE:
+ return field.isPackagePrivate() || field.isProtected();
+ case PUBLIC:
+ return false;
+ default:
+ throw new Unreachable();
+ }
+ }
}
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 e6040a0..2bc230c 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -97,6 +97,7 @@
import com.android.tools.r8.utils.SetUtils;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.Timing;
+import com.android.tools.r8.utils.Visibility;
import com.android.tools.r8.utils.WorkList;
import com.google.common.base.Equivalence.Wrapper;
import com.google.common.collect.ImmutableSet;
@@ -299,8 +300,11 @@
*/
private final Set<DexType> constClassReferences = Sets.newIdentityHashSet();
- /** A set of seen init-class references. */
- private final Set<DexType> initClassReferences = Sets.newIdentityHashSet();
+ /**
+ * A map from seen init-class references to the minimum required visibility of the corresponding
+ * static field.
+ */
+ private final Map<DexType, Visibility> initClassReferences = new IdentityHashMap<>();
/**
* A map from annotation classes to annotations that need to be processed should the classes ever
@@ -736,19 +740,60 @@
boolean traceInitClass(DexType type, ProgramMethod currentMethod) {
assert type.isClassType();
- if (!initClassReferences.add(type)) {
+ Visibility oldMinimumRequiredVisibility = initClassReferences.get(type);
+ if (oldMinimumRequiredVisibility == null) {
+ DexProgramClass clazz = getProgramClassOrNull(type);
+ if (clazz == null) {
+ assert false;
+ return false;
+ }
+
+ initClassReferences.put(
+ type, computeMinimumRequiredVisibilityForInitClassField(type, currentMethod.getHolder()));
+
+ markTypeAsLive(type, classReferencedFromReporter(currentMethod.getMethod()));
+ markDirectAndIndirectClassInitializersAsLive(clazz);
+ return true;
+ }
+
+ if (oldMinimumRequiredVisibility.isPublic()) {
return false;
}
- DexProgramClass clazz = getProgramClassOrNull(type);
- if (clazz == null) {
- assert false;
+ Visibility minimumRequiredVisibilityForCurrentMethod =
+ computeMinimumRequiredVisibilityForInitClassField(type, currentMethod.getHolder());
+
+ // There should never be a need to have an InitClass instruction for the enclosing class.
+ assert !minimumRequiredVisibilityForCurrentMethod.isPrivate();
+
+ if (minimumRequiredVisibilityForCurrentMethod.isPublic()) {
+ initClassReferences.put(type, minimumRequiredVisibilityForCurrentMethod);
+ return true;
+ }
+
+ if (oldMinimumRequiredVisibility.isProtected()) {
return false;
}
- markTypeAsLive(type, classReferencedFromReporter(currentMethod.getMethod()));
- markDirectAndIndirectClassInitializersAsLive(clazz);
- return true;
+ if (minimumRequiredVisibilityForCurrentMethod.isProtected()) {
+ initClassReferences.put(type, minimumRequiredVisibilityForCurrentMethod);
+ return true;
+ }
+
+ assert oldMinimumRequiredVisibility.isPackagePrivate();
+ assert minimumRequiredVisibilityForCurrentMethod.isPackagePrivate();
+ return false;
+ }
+
+ private Visibility computeMinimumRequiredVisibilityForInitClassField(
+ DexType clazz, DexProgramClass context) {
+ if (clazz.isSamePackage(context.type)) {
+ return Visibility.PACKAGE_PRIVATE;
+ }
+ if (appInfo.isStrictSubtypeOf(context.type, clazz)) {
+ return Visibility.PROTECTED;
+ }
+ return Visibility.PUBLIC;
}
void traceMethodHandle(
diff --git a/src/main/java/com/android/tools/r8/utils/ThreadUtils.java b/src/main/java/com/android/tools/r8/utils/ThreadUtils.java
index 9bb8877..84aa0d0 100644
--- a/src/main/java/com/android/tools/r8/utils/ThreadUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ThreadUtils.java
@@ -7,6 +7,7 @@
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -39,6 +40,18 @@
executorService);
}
+ public static <T, U, E extends Exception> void processItems(
+ Map<T, U> items, ThrowingBiConsumer<T, U, E> consumer, ExecutorService executorService)
+ throws ExecutionException {
+ processItemsWithResults(
+ items.entrySet(),
+ arg -> {
+ consumer.accept(arg.getKey(), arg.getValue());
+ return null;
+ },
+ executorService);
+ }
+
public static void awaitFutures(Iterable<? extends Future<?>> futures)
throws ExecutionException {
Iterator<? extends Future<?>> futureIterator = futures.iterator();
diff --git a/src/main/java/com/android/tools/r8/utils/Visibility.java b/src/main/java/com/android/tools/r8/utils/Visibility.java
new file mode 100644
index 0000000..32ec7ba
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/Visibility.java
@@ -0,0 +1,50 @@
+// Copyright (c) 2020, 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.android.tools.r8.errors.Unreachable;
+
+public enum Visibility {
+ PUBLIC,
+ PROTECTED,
+ PRIVATE,
+ PACKAGE_PRIVATE;
+
+ public boolean isPackagePrivate() {
+ return this == PACKAGE_PRIVATE;
+ }
+
+ public boolean isPrivate() {
+ return this == PRIVATE;
+ }
+
+ public boolean isProtected() {
+ return this == PROTECTED;
+ }
+
+ public boolean isPublic() {
+ return this == PUBLIC;
+ }
+
+ @Override
+ public String toString() {
+ switch (this) {
+ case PUBLIC:
+ return "public";
+
+ case PROTECTED:
+ return "protected";
+
+ case PRIVATE:
+ return "private";
+
+ case PACKAGE_PRIVATE:
+ return "package-private";
+
+ default:
+ throw new Unreachable("Unexpected visibility");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/internal/YouTubeV1419TreeShakeJarVerificationTest.java b/src/test/java/com/android/tools/r8/internal/YouTubeV1419TreeShakeJarVerificationTest.java
index dd47f7f..97b4db0 100644
--- a/src/test/java/com/android/tools/r8/internal/YouTubeV1419TreeShakeJarVerificationTest.java
+++ b/src/test/java/com/android/tools/r8/internal/YouTubeV1419TreeShakeJarVerificationTest.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.analysis.ProtoApplicationStats;
import org.junit.Test;
@@ -30,7 +31,7 @@
@Parameters(name = "{0}")
public static TestParametersCollection data() {
- return getTestParameters().withDexRuntimes().build();
+ return getTestParameters().withNoneRuntime().build();
}
public YouTubeV1419TreeShakeJarVerificationTest(TestParameters parameters) {
@@ -44,11 +45,17 @@
assumeTrue(isLocalDevelopment());
assumeTrue(shouldRunSlowTests());
+ LibrarySanitizer librarySanitizer =
+ new LibrarySanitizer(temp).addProguardConfigurationFiles(getKeepRuleFiles()).sanitize();
+
R8TestCompileResult compileResult =
- testForR8(parameters.getBackend())
- .addKeepRuleFiles(getKeepRuleFiles())
+ testForR8(Backend.DEX)
+ .addKeepRuleFiles(librarySanitizer.getSanitizedProguardConfiguration())
+ .addLibraryFiles(librarySanitizer.getSanitizedLibrary())
+ .addMainDexRuleFiles(getMainDexRuleFiles())
.allowDiagnosticMessages()
.allowUnusedProguardConfigurationRules()
+ .setMinApi(AndroidApiLevel.H_MR2)
.compile()
.assertAllInfoMessagesMatch(
containsString("Proguard configuration rule does not match anything"))
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCannotBePostponedTest.java b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCannotBePostponedTest.java
index b3a0265..7bc227a 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCannotBePostponedTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/SingletonClassInitializerPatternCannotBePostponedTest.java
@@ -48,8 +48,8 @@
ClassSubject classSubject = inspector.clazz(A.class);
assertThat(classSubject, isPresent());
- // A static field A.$r8$clinit has been synthesized to allow inlining of A.inlineable().
- assertThat(classSubject.uniqueFieldWithName("$r8$clinit"), isPresent());
+ // The field A.INSTANCE has been accessed to allow inlining of A.inlineable().
+ assertThat(classSubject.uniqueFieldWithName("INSTANCE"), isPresent());
assertThat(classSubject.uniqueMethodWithName("inlineable"), not(isPresent()));
}
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 a360336..98f6b1e 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
@@ -12,6 +12,7 @@
import com.android.tools.r8.retrace.RetraceMethodResult;
import com.android.tools.r8.retrace.RetraceMethodResult.Element;
import com.android.tools.r8.utils.Box;
+import com.android.tools.r8.utils.Visibility;
import com.google.common.collect.ImmutableList;
import java.util.List;
import java.util.stream.Collectors;
@@ -21,33 +22,6 @@
public class Matchers {
- private enum Visibility {
- PUBLIC,
- PROTECTED,
- PRIVATE,
- PACKAGE_PRIVATE;
-
- @Override
- public String toString() {
- switch (this) {
- case PUBLIC:
- return "public";
-
- case PROTECTED:
- return "protected";
-
- case PRIVATE:
- return "private";
-
- case PACKAGE_PRIVATE:
- return "package-private";
-
- default:
- throw new Unreachable("Unexpected visibility");
- }
- }
- }
-
private static String type(Subject subject) {
String type = "<unknown subject type>";
if (subject instanceof ClassSubject) {
