Enable access modification for desugared library
Fixes: b/288062771
Change-Id: I0e6de43b77146244c6bef1de6d7089b1052d8470
diff --git a/src/main/java/com/android/tools/r8/optimize/accessmodification/AccessModifierOptions.java b/src/main/java/com/android/tools/r8/optimize/accessmodification/AccessModifierOptions.java
index 6a27796..f56bd5c 100644
--- a/src/main/java/com/android/tools/r8/optimize/accessmodification/AccessModifierOptions.java
+++ b/src/main/java/com/android/tools/r8/optimize/accessmodification/AccessModifierOptions.java
@@ -5,11 +5,14 @@
package com.android.tools.r8.optimize.accessmodification;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.SystemPropertyUtils;
public class AccessModifierOptions {
// TODO(b/131130038): Do not allow accessmodification when kept.
- private boolean forceModifyPackagePrivateAndProtectedMethods = true;
+ private boolean forceModifyPackagePrivateAndProtectedMethods =
+ SystemPropertyUtils.parseSystemPropertyOrDefault(
+ "com.android.tools.r8.accessmodification.forcePackagePrivateAndProtected", true);
private final InternalOptions options;
@@ -25,10 +28,7 @@
if (isAccessModificationRulePresent()) {
return true;
}
- // TODO(b/288062771): Enable access modification by default for L8.
- return !options.getLibraryDesugaringOptions().isL8()
- && !options.forceProguardCompatibility
- && options.isOptimizing();
+ return !options.forceProguardCompatibility && options.isOptimizing();
}
private boolean isAccessModificationRulePresent() {
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11StreamAbstractTests.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11StreamAbstractTests.java
index 290a3c5..ef02ac6 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11StreamAbstractTests.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11StreamAbstractTests.java
@@ -18,6 +18,7 @@
import static com.android.tools.r8.utils.FileUtils.JAVA_EXTENSION;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.SingleTestRunResult;
import com.android.tools.r8.TestParameters;
@@ -275,7 +276,7 @@
Assume.assumeFalse(
"getAllFilesWithSuffixInDirectory() seems to find different files on Windows",
ToolHelper.isWindows());
- Assume.assumeTrue(
+ assumeTrue(
"Requires Java base extensions, should add it when not desugaring",
parameters.getApiLevel().getLevel() < AndroidApiLevel.N.getLevel());
@@ -289,19 +290,34 @@
Arrays.stream(JDK_11_STREAM_TEST_COMPILED_FILES)
.filter(file -> !file.toString().contains("lang/invoke"))
.collect(Collectors.toList());
- return testForDesugaredLibrary(
- parameters, libraryDesugaringSpecification, compilationSpecification)
- .addProgramFiles(filesToCompile)
- .applyIf(
- !libraryDesugaringSpecification.hasNioFileDesugaring(parameters),
- b -> b.addProgramFiles(getPathsFiles()))
- .addProgramFiles(getSafeVarArgsFile())
- .addProgramFiles(testNGSupportProgramFiles())
- .addProgramClassFileData(getTestNGMainRunner())
- .addOptionsModification(opt -> opt.testing.trackDesugaredAPIConversions = true)
- .disableL8AnnotationRemoval()
- .compile()
- .withArt6Plus64BitsLib();
+ // Prohibit publicizing LoggingTestCase#setContext. Currently L8 does not support modifying the
+ // InternalOptions for the R8 compilation inside L8, so we use a system property.
+ System.setProperty(
+ "com.android.tools.r8.accessmodification.forcePackagePrivateAndProtected", "0");
+ try {
+ return testForDesugaredLibrary(
+ parameters, libraryDesugaringSpecification, compilationSpecification)
+ .addProgramFiles(filesToCompile)
+ .applyIf(
+ !libraryDesugaringSpecification.hasNioFileDesugaring(parameters),
+ b -> b.addProgramFiles(getPathsFiles()))
+ .addProgramFiles(getSafeVarArgsFile())
+ .addProgramFiles(testNGSupportProgramFiles())
+ .addProgramClassFileData(getTestNGMainRunner())
+ .addL8KeepRules(
+ // Keep LoggingTestCase#setContext so that it is not publicized.
+ // Otherwise, the test runner incorrectly tries to run it as a @Test.
+ "-keepclassmembers class j$.util.stream.LoggingTestCase {",
+ " protected void setContext(java.lang.String, java.lang.Object);",
+ "}")
+ .addOptionsModification(opt -> opt.testing.trackDesugaredAPIConversions = true)
+ .disableL8AnnotationRemoval()
+ .compile()
+ .withArt6Plus64BitsLib();
+ } finally {
+ System.clearProperty(
+ "com.android.tools.r8.accessmodification.forcePackagePrivateAndProtected");
+ }
}
private void runSuccessfulTests(
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11TimeAbstractTests.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11TimeAbstractTests.java
index fe478900..dc0a0ce 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11TimeAbstractTests.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/jdktests/Jdk11TimeAbstractTests.java
@@ -37,6 +37,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.function.Consumer;
import java.util.stream.Collectors;
import org.junit.BeforeClass;
import org.junit.runner.RunWith;
@@ -266,6 +267,17 @@
.applyIf(
!libraryDesugaringSpecification.hasNioFileDesugaring(parameters),
b -> b.addProgramFiles(getPathsFiles()))
+ .apply(
+ b ->
+ // JDK 11 time tests inspect the visibility of private fields and constructors
+ // using reflection.
+ forEachImmutableClass(
+ className ->
+ b.addL8KeepRules(
+ "-keepclassmembers class " + className + " {",
+ " private final <fields>;",
+ " private <init>(...);",
+ "}")))
.compile()
.withArt6Plus64BitsLib();
for (String success : toRun) {
@@ -282,4 +294,21 @@
}
}
}
+
+ private static void forEachImmutableClass(Consumer<String> consumer) {
+ consumer.accept("j$.time.Duration");
+ consumer.accept("j$.time.Instant");
+ consumer.accept("j$.time.LocalDate");
+ consumer.accept("j$.time.LocalDateTime");
+ consumer.accept("j$.time.LocalTime");
+ consumer.accept("j$.time.MonthDay");
+ consumer.accept("j$.time.OffsetDateTime");
+ consumer.accept("j$.time.OffsetTime");
+ consumer.accept("j$.time.Period");
+ consumer.accept("j$.time.Year");
+ consumer.accept("j$.time.YearMonth");
+ consumer.accept("j$.time.ZonedDateTime");
+ consumer.accept("j$.time.ZoneOffset");
+ consumer.accept("j$.time.temporal.ValueRange");
+ }
}
diff --git a/src/test/testbase/java/com/android/tools/r8/desugar/desugaredlibrary/test/DesugaredLibraryTestBuilder.java b/src/test/testbase/java/com/android/tools/r8/desugar/desugaredlibrary/test/DesugaredLibraryTestBuilder.java
index 1c2b231..2128db5 100644
--- a/src/test/testbase/java/com/android/tools/r8/desugar/desugaredlibrary/test/DesugaredLibraryTestBuilder.java
+++ b/src/test/testbase/java/com/android/tools/r8/desugar/desugaredlibrary/test/DesugaredLibraryTestBuilder.java
@@ -33,6 +33,7 @@
import com.android.tools.r8.utils.ConsumerUtils;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.ThrowingConsumer;
import com.android.tools.r8.utils.codeinspector.VerticallyMergedClassesInspector;
import com.google.common.base.Charsets;
@@ -55,7 +56,7 @@
private final CompilationSpecification compilationSpecification;
private final TestCompilerBuilder<?, ?, ?, ? extends SingleTestRunResult<?>, ?> builder;
private List<ArtProfileForRewriting> l8ArtProfilesForRewriting = new ArrayList<>();
- private String l8ExtraKeepRules = "";
+ private List<String> l8ExtraKeepRules = new ArrayList<>();
private Consumer<InternalOptions> l8OptionModifier = ConsumerUtils.emptyConsumer();
private boolean l8FinalPrefixVerification = true;
private boolean overrideDefaultLibrary = false;
@@ -281,9 +282,9 @@
return this;
}
- public DesugaredLibraryTestBuilder<T> addL8KeepRules(String keepRules) {
+ public DesugaredLibraryTestBuilder<T> addL8KeepRules(String... keepRules) {
if (compilationSpecification.isL8Shrink()) {
- l8ExtraKeepRules += keepRules + "\n";
+ Collections.addAll(l8ExtraKeepRules, keepRules);
}
return this;
}
@@ -465,7 +466,7 @@
l8Builder
.applyIf(
compilationSpecification.isL8Shrink() && !l8ExtraKeepRules.isEmpty(),
- b -> b.addKeepRules(l8ExtraKeepRules))
+ b -> b.addKeepRules(StringUtils.lines(l8ExtraKeepRules)))
.addOptionsModifier(l8OptionModifier);
for (ArtProfileForRewriting artProfileForRewriting : l8ArtProfilesForRewriting) {
l8Builder.addArtProfileForRewriting(
diff --git a/src/test/testbase/java/com/android/tools/r8/desugar/desugaredlibrary/test/DesugaredLibraryTestCompileResult.java b/src/test/testbase/java/com/android/tools/r8/desugar/desugaredlibrary/test/DesugaredLibraryTestCompileResult.java
index 2fb1cc6..4ca66ed 100644
--- a/src/test/testbase/java/com/android/tools/r8/desugar/desugaredlibrary/test/DesugaredLibraryTestCompileResult.java
+++ b/src/test/testbase/java/com/android/tools/r8/desugar/desugaredlibrary/test/DesugaredLibraryTestCompileResult.java
@@ -180,6 +180,11 @@
return l8Compile.writeToZip();
}
+ public DesugaredLibraryTestCompileResult<T> writeL8ToZip(Path path) throws IOException {
+ l8Compile.writeToZip(path);
+ return this;
+ }
+
public DesugaredLibraryTestCompileResult<T> addRunClasspathFiles(Path... classpathFiles) {
runnableCompiledResult.addRunClasspathFiles(classpathFiles);
return this;
