Reapply "Fix handling of R class in optimized shrinking"
This reverts commit 83fd1cf6cf56a91918024bf33b28577887c90df9.
Bug: b/336983087
Bug: b/325905703
Bug: b/287398085
Change-Id: I28314b247c937490bced128f160dd3a82ec31398
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfFrameVerifier.java b/src/main/java/com/android/tools/r8/cf/code/CfFrameVerifier.java
index d1b3d3e..86ea3c0 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfFrameVerifier.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfFrameVerifier.java
@@ -248,29 +248,32 @@
// safe assuming an incoming type state T. The type state T is derived from ExcStackFrame
// by replacing the operand stack with a stack whose sole element is the handler's
// exception class.
- for (CfLabel target : tryCatchRange.getTargets()) {
+ List<CfLabel> targets = tryCatchRange.getTargets();
+ for (int i = 0; i < targets.size(); i++) {
+ CfLabel target = targets.get(i);
+ DexType guard = tryCatchRange.guards.get(i);
CfFrame destinationFrame = labelToFrameMap.get(target);
if (destinationFrame == null) {
return CfCodeStackMapValidatingException.invalidTryCatchRange(
method, tryCatchRange, "No frame for target catch range target", appView);
}
- // From the spec: the handler's exception class is assignable to the class Throwable.
- for (DexType guard : tryCatchRange.guards) {
- if (!config.getAssignability().isAssignable(guard, factory.throwableType)) {
- return CfCodeStackMapValidatingException.invalidTryCatchRange(
- method,
- tryCatchRange,
- "Could not assign " + guard.getTypeName() + " to java.lang.Throwable",
- appView);
- }
- Deque<PreciseFrameType> sourceStack =
- ImmutableDeque.of(FrameType.initializedNonNullReference(guard));
- AssignabilityResult assignabilityResult =
- config.getAssignability().isStackAssignable(sourceStack, destinationFrame.getStack());
- if (assignabilityResult.isFailed()) {
- return CfCodeStackMapValidatingException.invalidTryCatchRange(
- method, tryCatchRange, assignabilityResult.asFailed().getMessage(), appView);
- }
+ Deque<PreciseFrameType> sourceStack =
+ ImmutableDeque.of(FrameType.initializedNonNullReference(guard));
+ AssignabilityResult assignabilityResult =
+ config.getAssignability().isStackAssignable(sourceStack, destinationFrame.getStack());
+ if (assignabilityResult.isFailed()) {
+ return CfCodeStackMapValidatingException.invalidTryCatchRange(
+ method, tryCatchRange, assignabilityResult.asFailed().getMessage(), appView);
+ }
+ }
+ // From the spec: the handler's exception class is assignable to the class Throwable.
+ for (DexType guard : tryCatchRange.guards) {
+ if (!config.getAssignability().isAssignable(guard, factory.throwableType)) {
+ return CfCodeStackMapValidatingException.invalidTryCatchRange(
+ method,
+ tryCatchRange,
+ "Could not assign " + guard.getTypeName() + " to java.lang.Throwable",
+ appView);
}
}
return null;
diff --git a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracing.java b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracing.java
index 78abb50..51e854f 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracing.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracing.java
@@ -21,13 +21,11 @@
public static EnqueuerDeferredTracing create(
AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer, Mode mode) {
- if (mode.isInitialTreeShaking()) {
+ InternalOptions options = appView.options();
+ if (!options.isShrinking() || !options.enableEnqueuerDeferredTracing) {
return empty();
}
- InternalOptions options = appView.options();
- if (!options.isOptimizing()
- || !options.isShrinking()
- || !options.enableEnqueuerDeferredTracing) {
+ if (!options.isOptimizing() && !options.isOptimizedResourceShrinking()) {
return empty();
}
return new EnqueuerDeferredTracingImpl(appView, enqueuer, mode);
diff --git a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
index 1eacba7..5291978 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
@@ -128,6 +128,11 @@
// field's holder. Therefore, we unconditionally trace the class initializer in this case.
// The corresponding IR rewriter will rewrite the field access into an init-class instruction.
if (accessKind.isStatic()) {
+ if (enqueuer.getMode().isInitialTreeShaking() && field.getHolder() != context.getHolder()) {
+ // TODO(b/338000616): No support for InitClass until we have LIR in the initial round of
+ // tree shaking.
+ return false;
+ }
KeepReason reason =
enqueuer.getGraphReporter().reportClassReferencedFrom(field.getHolder(), context);
enqueuer.getWorklist().enqueueTraceTypeReferenceAction(field.getHolder(), reason);
@@ -148,7 +153,7 @@
}
assert enqueuer.getKeepInfo(field).isBottom();
- assert !enqueuer.getKeepInfo(field).isPinned(options);
+ assert !enqueuer.getKeepInfo(field).isPinned(options) || options.isOptimizedResourceShrinking();
FieldAccessInfo info = enqueuer.getFieldAccessInfoCollection().get(field.getReference());
if (info.hasReflectiveAccess()
@@ -162,6 +167,12 @@
|| !minimumKeepInfo.isShrinkingAllowed())) {
return false;
}
+ if (!options.isOptimizing()) {
+ assert options.isOptimizedResourceShrinking();
+ if (!enqueuer.isRClass(field.getHolder())) {
+ return false;
+ }
+ }
if (info.isWritten()) {
// If the assigned value may have an override of Object#finalize() then give up.
diff --git a/src/test/java/com/android/tools/r8/androidresources/NoOptResourceShrinkingTest.java b/src/test/java/com/android/tools/r8/androidresources/NoOptResourceShrinkingTest.java
index e428d08..2248d93 100644
--- a/src/test/java/com/android/tools/r8/androidresources/NoOptResourceShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/androidresources/NoOptResourceShrinkingTest.java
@@ -55,13 +55,8 @@
resourceTableInspector -> {
resourceTableInspector.assertContainsResourceWithName("string", "bar");
resourceTableInspector.assertContainsResourceWithName("string", "foo");
- if (optimized) {
- // TODO(b/336983087): This should be removed.
- resourceTableInspector.assertContainsResourceWithName("string", "unused_string");
- } else {
- resourceTableInspector.assertDoesNotContainResourceWithName(
- "string", "unused_string");
- }
+ resourceTableInspector.assertDoesNotContainResourceWithName(
+ "string", "unused_string");
})
.run(parameters.getRuntime(), FooBar.class)
.assertSuccess();
diff --git a/src/test/java/com/android/tools/r8/androidresources/optimizedshrinking/TestOptimizedShrinking.java b/src/test/java/com/android/tools/r8/androidresources/optimizedshrinking/TestOptimizedShrinking.java
index 74c7102..584426b 100644
--- a/src/test/java/com/android/tools/r8/androidresources/optimizedshrinking/TestOptimizedShrinking.java
+++ b/src/test/java/com/android/tools/r8/androidresources/optimizedshrinking/TestOptimizedShrinking.java
@@ -107,7 +107,7 @@
// In optimized mode we track these correctly, so we should not unconditionally keep
// all attributes.
- if (optimized && !debug) {
+ if (optimized) {
resourceTableInspector.assertDoesNotContainResourceWithName(
"attr", "attr_unused_styleable" + i);
} else {
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EmptyEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/EmptyEnumUnboxingTest.java
index c2ab4e2..2cbfd3f 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/EmptyEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EmptyEnumUnboxingTest.java
@@ -38,8 +38,16 @@
.addInnerClasses(EmptyEnumUnboxingTest.class)
.addKeepMainRule(Main.class)
.addKeepRules(enumKeepRules.getKeepRules())
- // TODO(b/166532373): Unbox enum with no cases.
- .addEnumUnboxingInspector(inspector -> inspector.assertNotUnboxed(MyEnum.class))
+ .addEnumUnboxingInspector(
+ inspector -> {
+ if (enumKeepRules.isStudio()) {
+ // TODO(b/166532373): Unbox enum with no cases.
+ inspector.assertNotUnboxed(MyEnum.class);
+ } else {
+ assert enumKeepRules.isNone();
+ inspector.assertUnboxed(MyEnum.class);
+ }
+ })
.enableNeverClassInliningAnnotations()
.enableInliningAnnotations()
.addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
diff --git a/src/test/java/com/android/tools/r8/internal/GMSCoreLatestTest.java b/src/test/java/com/android/tools/r8/internal/GMSCoreLatestTest.java
index ef9401a..bfb7a05 100644
--- a/src/test/java/com/android/tools/r8/internal/GMSCoreLatestTest.java
+++ b/src/test/java/com/android/tools/r8/internal/GMSCoreLatestTest.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ThrowableConsumer;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.references.Reference;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.AssertionUtils;
import com.google.common.collect.Sets;
@@ -24,6 +25,7 @@
import java.util.Collections;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -68,8 +70,10 @@
options -> {
options.testing.processingContextsConsumer =
id -> assertNull(idsRoundOne.put(id, id));
+ options
+ .getOpenClosedInterfacesOptions()
+ .suppressSingleOpenInterface(Reference.classFromClass(Lock.class));
}));
-
compileResult.runDex2Oat(parameters.getRuntime()).assertNoVerificationErrors();
Map<String, String> idsRoundTwo = new ConcurrentHashMap<>();
@@ -83,6 +87,9 @@
AssertionUtils.assertNotNull(idsRoundOne.get(id));
assertNull(idsRoundTwo.put(id, id));
};
+ options
+ .getOpenClosedInterfacesOptions()
+ .suppressSingleOpenInterface(Reference.classFromClass(Lock.class));
}));
// Verify that the result of the two compilations was the same.
diff --git a/src/test/java/com/android/tools/r8/internal/GMSCoreV10Test.java b/src/test/java/com/android/tools/r8/internal/GMSCoreV10Test.java
index 4c61934..484a349 100644
--- a/src/test/java/com/android/tools/r8/internal/GMSCoreV10Test.java
+++ b/src/test/java/com/android/tools/r8/internal/GMSCoreV10Test.java
@@ -15,9 +15,11 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ThrowableConsumer;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.references.Reference;
import com.android.tools.r8.utils.AndroidApiLevel;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.concurrent.locks.Lock;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -59,6 +61,9 @@
builder.addOptionsModification(
options -> {
options.testing.forceJumboStringProcessing = true;
+ options
+ .getOpenClosedInterfacesOptions()
+ .suppressSingleOpenInterface(Reference.classFromClass(Lock.class));
}))
.runDex2Oat(parameters.getRuntime())
.assertNoVerificationErrors();
