Revert "[KeepAnno] Use native interpretation in R8"
This reverts back the API command to use rule extraction in the reader
instead of direct native interpretation. The keepanno tests do not use
the API call in this case such that they continue to test the "direct"
native interpretation.
This CL should be reverted to re-enable the native interpretation once
it is deemed safe for use in platform.
Bug: b/349276075
Change-Id: I02aec9c88417481685c123f27235b6d23a9eda6c
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 2fc05e8..3bd0ac5 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -17,6 +17,9 @@
import com.android.tools.r8.inspector.internal.InspectorImpl;
import com.android.tools.r8.ir.desugar.desugaredlibrary.DesugaredLibrarySpecification;
import com.android.tools.r8.keepanno.annotations.KeepForApi;
+import com.android.tools.r8.keepanno.asm.KeepEdgeReader;
+import com.android.tools.r8.keepanno.ast.KeepDeclaration;
+import com.android.tools.r8.keepanno.keeprules.KeepRuleExtractor;
import com.android.tools.r8.naming.MapConsumer;
import com.android.tools.r8.naming.ProguardMapStringConsumer;
import com.android.tools.r8.naming.SourceFileRewriter;
@@ -62,6 +65,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.Deque;
import java.util.List;
import java.util.Optional;
@@ -133,7 +137,8 @@
FeatureSplitConfiguration.builder();
private String synthesizedClassPrefix = "";
private boolean enableMissingLibraryApiModeling = false;
- private boolean enableExperimentalKeepAnnotations = false;
+ private boolean enableExperimentalKeepAnnotations =
+ System.getProperty("com.android.tools.r8.enableKeepAnnotations") != null;
public boolean enableStartupLayoutOptimization = true;
private SemanticVersion fakeCompilerVersion = null;
private AndroidResourceProvider androidResourceProvider = null;
@@ -699,6 +704,10 @@
// Add embedded keep rules.
amendWithRulesAndProvidersForInjarsAndMetaInf(reporter, parser);
+
+ // Extract out rules for keep annotations and amend the configuration.
+ // TODO(b/248408342): Remove this and parse annotations as part of R8 root-set & enqueuer.
+ extractKeepAnnotationRules(parser);
ProguardConfiguration configuration = configurationBuilder.build();
getAppBuilder().addFilteredLibraryArchives(configuration.getLibraryjars());
@@ -747,7 +756,6 @@
getSourceFileProvider(),
enableMissingLibraryApiModeling,
enableStartupLayoutOptimization,
- enableExperimentalKeepAnnotations,
getAndroidPlatformBuild(),
getArtProfilesForRewriting(),
getStartupProfileProviders(),
@@ -828,6 +836,35 @@
}
}
+ private void extractKeepAnnotationRules(ProguardConfigurationParser parser) {
+ if (!enableExperimentalKeepAnnotations) {
+ return;
+ }
+ try {
+ for (ProgramResourceProvider provider : getAppBuilder().getProgramResourceProviders()) {
+ for (ProgramResource resource : provider.getProgramResources()) {
+ if (resource.getKind() == Kind.CF) {
+ List<KeepDeclaration> declarations =
+ KeepEdgeReader.readKeepEdges(resource.getBytes());
+ if (!declarations.isEmpty()) {
+ KeepRuleExtractor extractor =
+ new KeepRuleExtractor(
+ rule -> {
+ ProguardConfigurationSourceStrings source =
+ new ProguardConfigurationSourceStrings(
+ Collections.singletonList(rule), null, resource.getOrigin());
+ parser.parse(source);
+ });
+ declarations.forEach(extractor::extract);
+ }
+ }
+ }
+ }
+ } catch (ResourceException e) {
+ throw getAppBuilder().getReporter().fatalError(new ExceptionDiagnostic(e));
+ }
+ }
+
// Internal for-testing method to add post-processors of the proguard configuration.
void addProguardConfigurationConsumerForTesting(Consumer<ProguardConfiguration.Builder> c) {
Consumer<ProguardConfiguration.Builder> oldConsumer = proguardConfigurationConsumerForTesting;
@@ -919,7 +956,6 @@
private final String synthesizedClassPrefix;
private final boolean enableMissingLibraryApiModeling;
private final boolean enableStartupLayoutOptimization;
- private final boolean enableExperimentalKeepAnnotations;
private final AndroidResourceProvider androidResourceProvider;
private final AndroidResourceConsumer androidResourceConsumer;
private final ResourceShrinkerConfiguration resourceShrinkerConfiguration;
@@ -1013,7 +1049,6 @@
SourceFileProvider sourceFileProvider,
boolean enableMissingLibraryApiModeling,
boolean enableStartupLayoutOptimization,
- boolean enableExperimentalKeepAnnotations,
boolean isAndroidPlatformBuild,
List<ArtProfileForRewriting> artProfilesForRewriting,
List<StartupProfileProvider> startupProfileProviders,
@@ -1068,7 +1103,6 @@
this.synthesizedClassPrefix = synthesizedClassPrefix;
this.enableMissingLibraryApiModeling = enableMissingLibraryApiModeling;
this.enableStartupLayoutOptimization = enableStartupLayoutOptimization;
- this.enableExperimentalKeepAnnotations = enableExperimentalKeepAnnotations;
this.androidResourceProvider = androidResourceProvider;
this.androidResourceConsumer = androidResourceConsumer;
this.resourceShrinkerConfiguration = resourceShrinkerConfiguration;
@@ -1097,7 +1131,6 @@
synthesizedClassPrefix = null;
enableMissingLibraryApiModeling = false;
enableStartupLayoutOptimization = true;
- enableExperimentalKeepAnnotations = false;
androidResourceProvider = null;
androidResourceConsumer = null;
resourceShrinkerConfiguration = null;
@@ -1158,10 +1191,6 @@
assert !internal.enableTreeShakingOfLibraryMethodOverrides;
- if (enableExperimentalKeepAnnotations) {
- internal.testing.enableEmbeddedKeepAnnotations = true;
- }
-
if (!internal.isShrinking()) {
// If R8 is not shrinking, there is no point in running various optimizations since the
// optimized classes will still remain in the program (the application size could increase).
diff --git a/src/test/java/com/android/tools/r8/keepanno/KeepAnnoTestBuilder.java b/src/test/java/com/android/tools/r8/keepanno/KeepAnnoTestBuilder.java
index b4e7eab..516cccd 100644
--- a/src/test/java/com/android/tools/r8/keepanno/KeepAnnoTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/keepanno/KeepAnnoTestBuilder.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.keepanno;
-import static org.junit.Assert.assertEquals;
import com.android.tools.r8.ExternalR8TestBuilder;
import com.android.tools.r8.ProguardTestBuilder;
@@ -190,11 +189,10 @@
TestBase.testForR8(temp, parameters().getBackend())
.enableExperimentalKeepAnnotations()
.setMinApi(parameters());
- builder.getBuilder().setEnableExperimentalKeepAnnotations(isDirect());
- // If the internal reader value does not match we will need to update the extract tests to
- // also strip the annotations.
- builder.addOptionsModification(
- o -> assertEquals(isDirect(), o.testing.enableEmbeddedKeepAnnotations));
+
+ // TODO(b/323816623): Replace the testing flag by the API call.
+ builder.getBuilder().setEnableExperimentalKeepAnnotations(false);
+ builder.addOptionsModification(o -> o.testing.enableEmbeddedKeepAnnotations = isDirect());
}
private boolean isExtractRules() {
diff --git a/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java b/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
index 9d35920..4d7338d 100644
--- a/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
@@ -782,7 +782,7 @@
}
public T enableExperimentalKeepAnnotations() {
- builder.setEnableExperimentalKeepAnnotations(true);
+ addOptionsModification(o -> o.testing.enableEmbeddedKeepAnnotations = true);
try {
builder.addClasspathFiles(KeepAnnoTestUtils.getKeepAnnoLib(getState().getTempFolder()));
return self();
