Revert "Compute reachability sensitive when parsing program classes"
This reverts commit cebd7fa8add4216e1a11441a134ac7d845c97141.
Reason for revert: test failures
Change-Id: I83ebe28f9f543162ea732d9d2f3d219b8b115529
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index a7e37bc..c43043c 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -809,11 +809,8 @@
public final DexType annotationCovariantReturnTypes =
createStaticallyKnownType(
"Ldalvik/annotation/codegen/CovariantReturnType$CovariantReturnTypes;");
-
- public final String annotationReachabilitySensitiveDesc =
- "Ldalvik/annotation/optimization/ReachabilitySensitive;";
public final DexType annotationReachabilitySensitive =
- createStaticallyKnownType(annotationReachabilitySensitiveDesc);
+ createStaticallyKnownType("Ldalvik/annotation/optimization/ReachabilitySensitive;");
private static final String METAFACTORY_METHOD_NAME = "metafactory";
private static final String METAFACTORY_ALT_METHOD_NAME = "altMetafactory";
diff --git a/src/main/java/com/android/tools/r8/graph/DexProgramClass.java b/src/main/java/com/android/tools/r8/graph/DexProgramClass.java
index 4669fdb..68bc137 100644
--- a/src/main/java/com/android/tools/r8/graph/DexProgramClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexProgramClass.java
@@ -204,14 +204,24 @@
* that is the case, dead reference elimination is disabled and locals are kept alive for their
* entire scope.
*/
- public boolean isReachabilitySensitive() {
- assert !reachabilitySensitive.isUnknown();
+ public boolean getOrComputeReachabilitySensitive(AppView<?> appView) {
+ if (reachabilitySensitive.isUnknown()) {
+ reachabilitySensitive = OptionalBool.of(internalComputeReachabilitySensitive(appView));
+ }
return reachabilitySensitive.isTrue();
}
- public void setReachabilitySensitive(boolean isReachabilitySensitive) {
- assert reachabilitySensitive.isUnknown();
- reachabilitySensitive = OptionalBool.of(isReachabilitySensitive);
+ @SuppressWarnings("ReferenceEquality")
+ private boolean internalComputeReachabilitySensitive(AppView<?> appView) {
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+ for (DexEncodedMember<?, ?> member : members()) {
+ for (DexAnnotation annotation : member.annotations().annotations) {
+ if (annotation.annotation.type == dexItemFactory.annotationReachabilitySensitive) {
+ return true;
+ }
+ }
+ }
+ return false;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index f53c0ed..089194a 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -51,6 +51,7 @@
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.base.Equivalence.Wrapper;
+import com.google.common.collect.Iterables;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collections;
@@ -257,7 +258,7 @@
private final List<DexEncodedMethod> directMethods = new ArrayList<>();
private final List<DexEncodedMethod> virtualMethods = new ArrayList<>();
private final Set<Wrapper<DexMethod>> methodSignatures = new HashSet<>();
- private boolean hasReachabilitySensitiveMember = false;
+ private boolean hasReachabilitySensitiveMethod = false;
private SyntheticMarker syntheticMarker = null;
public CreateDexClassVisitor(
@@ -491,6 +492,7 @@
addAnnotation(DexAnnotation.createAnnotationDefaultAnnotation(
type, defaultAnnotations, application.getFactory()));
}
+ checkReachabilitySensitivity();
checkRecord();
T clazz =
classKind.create(
@@ -552,7 +554,6 @@
if (clazz.isProgramClass()) {
DexProgramClass programClass = clazz.asProgramClass();
programClass.setInitialClassFileVersion(version);
- programClass.setReachabilitySensitive(hasReachabilitySensitiveMember);
if (deprecated) {
programClass.setDeprecated();
}
@@ -598,6 +599,33 @@
}
}
+ // If anything is marked reachability sensitive, all methods need to be parsed including
+ // locals information. This propagates the reachability sensitivity bit so that if any field
+ // or method is annotated, all methods get parsed with locals information.
+ private void checkReachabilitySensitivity() {
+ if (hasReachabilitySensitiveMethod || hasReachabilitySensitiveField()) {
+ for (DexEncodedMethod method : Iterables.concat(directMethods, virtualMethods)) {
+ Code code = method.getCode();
+ if (code != null && code.isCfCode()) {
+ code.asLazyCfCode().markReachabilitySensitive();
+ }
+ }
+ }
+ }
+
+ @SuppressWarnings("ReferenceEquality")
+ private boolean hasReachabilitySensitiveField() {
+ DexType reachabilitySensitive = application.getFactory().annotationReachabilitySensitive;
+ for (DexEncodedField field : Iterables.concat(instanceFields, staticFields)) {
+ for (DexAnnotation annotation : field.annotations().annotations) {
+ if (annotation.annotation.type == reachabilitySensitive) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private void addDefaultAnnotation(String name, DexValue value) {
if (defaultAnnotations == null) {
defaultAnnotations = new ArrayList<>();
@@ -690,11 +718,6 @@
parsingContext,
parent.application::addKeepDeclaration);
}
-
- // Check for reachability sensitive annotation.
- parent.hasReachabilitySensitiveMember |=
- parent.application.getFactory().annotationReachabilitySensitiveDesc.equals(desc);
-
return createAnnotationVisitor(
desc, visible, getAnnotations(), parent.application, DexAnnotation::new);
}
@@ -858,11 +881,6 @@
parsingContext,
parent.application::addKeepDeclaration);
}
-
- // Check for reachability sensitive annotation.
- parent.hasReachabilitySensitiveMember |=
- parent.application.getFactory().annotationReachabilitySensitiveDesc.equals(desc);
-
return createAnnotationVisitor(
desc, visible, getAnnotations(), parent.application, DexAnnotation::new);
}
@@ -1012,6 +1030,7 @@
.build();
Wrapper<DexMethod> signature = MethodSignatureEquivalence.get().wrap(method);
if (parent.methodSignatures.add(signature)) {
+ parent.hasReachabilitySensitiveMethod |= isReachabilitySensitive();
if (flags.isStatic() || flags.isConstructor() || flags.isPrivate()) {
parent.directMethods.add(dexMethod);
} else {
@@ -1030,6 +1049,18 @@
}
}
+ @SuppressWarnings("ReferenceEquality")
+ private boolean isReachabilitySensitive() {
+ DexType reachabilitySensitive =
+ parent.application.getFactory().annotationReachabilitySensitive;
+ for (DexAnnotation annotation : getAnnotations()) {
+ if (annotation.annotation.type == reachabilitySensitive) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private List<DexAnnotation> getAnnotations() {
if (annotations == null) {
annotations = new ArrayList<>();
diff --git a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
index 7a903b2..c41f8df 100644
--- a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
@@ -119,6 +119,12 @@
private JarApplicationReader application;
private CfCode code;
private ReparseContext context;
+ private boolean reachabilitySensitive = false;
+
+ public void markReachabilitySensitive() {
+ assert code == null;
+ reachabilitySensitive = true;
+ }
@Override
public boolean isCfCode() {
@@ -152,13 +158,11 @@
private void internalParseCode() {
ReparseContext context = this.context;
JarApplicationReader application = this.application;
- assert context != null;
assert application != null;
- boolean reachabilitySensitive = context.owner.asProgramClass().isReachabilitySensitive();
- DebugParsingOptions parsingOptions = getParsingOptions(application, reachabilitySensitive);
+ assert context != null;
// The ClassCodeVisitor is in charge of setting this.context to null.
try {
- parseCode(context, false, parsingOptions);
+ parseCode(context, false);
} catch (JsrEncountered e) {
for (Code code : context.codeList) {
code.asLazyCfCode().code = null;
@@ -166,7 +170,7 @@
code.asLazyCfCode().application = application;
}
try {
- parseCode(context, true, parsingOptions);
+ parseCode(context, true);
} catch (JsrEncountered e1) {
throw new Unreachable(e1);
}
@@ -200,8 +204,9 @@
}
}
- private void parseCode(
- ReparseContext context, boolean useJsrInliner, DebugParsingOptions parsingOptions) {
+ public void parseCode(ReparseContext context, boolean useJsrInliner) {
+ DebugParsingOptions parsingOptions = getParsingOptions(application, reachabilitySensitive);
+
ClassCodeVisitor classVisitor =
new ClassCodeVisitor(
context.owner,
@@ -296,8 +301,7 @@
@Override
public String toString() {
- // Don't force parsing in toString as it causes unexpected behavior when debugging.
- return code != null ? code.toString() : "<lazy-code>";
+ return asCfCode().toString();
}
@Override
diff --git a/src/main/java/com/android/tools/r8/graph/ProgramMethod.java b/src/main/java/com/android/tools/r8/graph/ProgramMethod.java
index 54dc05d..03758b2 100644
--- a/src/main/java/com/android/tools/r8/graph/ProgramMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/ProgramMethod.java
@@ -180,8 +180,8 @@
return getDefinition().getKotlinInfo();
}
- public boolean isReachabilitySensitive() {
- return getHolder().isReachabilitySensitive();
+ public boolean getOrComputeReachabilitySensitive(AppView<?> appView) {
+ return getHolder().getOrComputeReachabilitySensitive(appView);
}
public void setCode(Code newCode, AppView<?> appView) {
@@ -202,7 +202,7 @@
if (appView.testing().noLocalsTableOnInput) {
return false;
}
- return appView.options().debug || isReachabilitySensitive();
+ return appView.options().debug || getOrComputeReachabilitySensitive(appView);
}
public ProgramMethod rewrittenWithLens(
diff --git a/src/main/java/com/android/tools/r8/graph/fixup/TreeFixerBase.java b/src/main/java/com/android/tools/r8/graph/fixup/TreeFixerBase.java
index 5f6d96f..ec65897 100644
--- a/src/main/java/com/android/tools/r8/graph/fixup/TreeFixerBase.java
+++ b/src/main/java/com/android/tools/r8/graph/fixup/TreeFixerBase.java
@@ -113,6 +113,7 @@
return newProgramClasses;
}
+ @SuppressWarnings("ReferenceEquality")
// Should remain private as the correctness of the fixup requires the lazy 'newProgramClasses'.
private DexProgramClass fixupClass(DexProgramClass clazz) {
DexProgramClass newClass =
@@ -140,7 +141,6 @@
newClass.setInstanceFields(fixupFields(clazz.instanceFields()));
newClass.setStaticFields(fixupFields(clazz.staticFields()));
// Transfer properties that are not passed to the constructor.
- newClass.setReachabilitySensitive(clazz.isReachabilitySensitive());
if (clazz.hasClassFileVersion()) {
newClass.setInitialClassFileVersion(clazz.getInitialClassFileVersion());
}
@@ -151,7 +151,7 @@
newClass.setKotlinInfo(clazz.getKotlinInfo());
}
// If the class type changed, record the move in the lens.
- if (newClass.getType().isNotIdenticalTo(clazz.getType())) {
+ if (newClass.getType() != clazz.getType()) {
return recordClassChange(clazz, newClass);
}
return newClass;
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index 5077746..7f7653b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -632,7 +632,7 @@
public boolean isConsistentSSABeforeTypesAreCorrectAllowingRedundantBlocks(AppView<?> appView) {
assert isConsistentGraph(appView, true);
- assert consistentBlockInstructions(true);
+ assert consistentBlockInstructions(appView, true);
assert consistentDefUseChains();
assert validThrowingInstructions();
assert noCriticalEdges();
@@ -689,7 +689,7 @@
assert consistentBlockNumbering();
assert consistentPredecessorSuccessors();
assert consistentCatchHandlers();
- assert consistentBlockInstructions(ssa);
+ assert consistentBlockInstructions(appView, ssa);
assert consistentMetadata();
assert verifyAllThrowingInstructionsHavePositions();
return true;
@@ -889,11 +889,13 @@
return true;
}
- private boolean consistentBlockInstructions(boolean ssa) {
+ private boolean consistentBlockInstructions(AppView<?> appView, boolean ssa) {
boolean argumentsAllowed = true;
for (BasicBlock block : blocks) {
assert block.consistentBlockInstructions(
- argumentsAllowed, options.debug || context().isReachabilitySensitive(), ssa);
+ argumentsAllowed,
+ options.debug || context().getOrComputeReachabilitySensitive(appView),
+ ssa);
argumentsAllowed = false;
}
return true;
diff --git a/src/main/java/com/android/tools/r8/ir/code/MoveException.java b/src/main/java/com/android/tools/r8/ir/code/MoveException.java
index 4d73577..15082ac 100644
--- a/src/main/java/com/android/tools/r8/ir/code/MoveException.java
+++ b/src/main/java/com/android/tools/r8/ir/code/MoveException.java
@@ -87,7 +87,7 @@
public DeadInstructionResult canBeDeadCode(AppView<?> appView, IRCode code) {
InternalOptions options = appView.options();
if (options.debug
- || code.context().isReachabilitySensitive()
+ || code.context().getOrComputeReachabilitySensitive(appView)
|| !code.getConversionOptions().isGeneratingDex()) {
return DeadInstructionResult.notDead();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 8d03811..b95529b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -502,7 +502,7 @@
}
public boolean isDebugMode() {
- return appView.options().debug || getProgramMethod().isReachabilitySensitive();
+ return appView.options().debug || getProgramMethod().getOrComputeReachabilitySensitive(appView);
}
public Int2ReferenceSortedMap<BlockInfo> getCFG() {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index e3e340b..733a207 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -522,7 +522,7 @@
previous = printMethod(code, "IR after disable assertions (SSA)", previous);
}
- boolean isDebugMode = options.debug || context.isReachabilitySensitive();
+ boolean isDebugMode = options.debug || context.getOrComputeReachabilitySensitive(appView);
assert !method.isProcessed() || !isDebugMode
: "Method already processed: "
+ context.toSourceString()
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/CodeRewriterPass.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/CodeRewriterPass.java
index 44cf6e7..c56ddbf 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/CodeRewriterPass.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/CodeRewriterPass.java
@@ -76,7 +76,7 @@
}
protected boolean isDebugMode(ProgramMethod context) {
- return options.debug || context.isReachabilitySensitive();
+ return options.debug || context.getOrComputeReachabilitySensitive(appView);
}
protected abstract String getRewriterId();
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/itf/EmulatedInterfaceApplicationRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/itf/EmulatedInterfaceApplicationRewriter.java
index b97b26e..8de8311 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/itf/EmulatedInterfaceApplicationRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/itf/EmulatedInterfaceApplicationRewriter.java
@@ -90,7 +90,6 @@
MethodCollectionFactory.fromMethods(newDirectMethods, newVirtualMethods),
false,
emulatedInterface.getChecksumSupplier());
- newEmulatedInterface.setReachabilitySensitive(false);
newEmulatedInterface.addExtraInterfaces(
getRewrittenInterfacesOfEmulatedInterface(emulatedInterface), appView.dexItemFactory());
return newEmulatedInterface;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ClassInitializerDefaultsOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/ClassInitializerDefaultsOptimization.java
index b6804e7..c6d8ac4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ClassInitializerDefaultsOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ClassInitializerDefaultsOptimization.java
@@ -151,7 +151,7 @@
}
ProgramMethod context = code.context();
- if (context.isReachabilitySensitive()) {
+ if (context.getOrComputeReachabilitySensitive(appView)) {
return ClassInitializerDefaultsResult.empty();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 4b9c547..3d78982 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -160,7 +160,7 @@
}
ProgramMethod method = code.context();
DexEncodedMethod definition = method.getDefinition();
- if (definition.isClassInitializer() || method.isReachabilitySensitive()) {
+ if (definition.isClassInitializer() || method.getOrComputeReachabilitySensitive(appView)) {
return ConstraintWithTarget.NEVER;
}
KeepMethodInfo keepInfo = appView.getKeepInfo(method);
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
index c099750..2acd209 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
@@ -233,7 +233,7 @@
// and we do not actually want locals information in the output.
if (options().debug) {
computeDebugInfo(code, blocks, liveIntervals, this, liveAtEntrySets);
- } else if (code.context().isReachabilitySensitive()) {
+ } else if (code.context().getOrComputeReachabilitySensitive(appView)) {
InstructionListIterator it = code.instructionListIterator();
while (it.hasNext()) {
Instruction instruction = it.next();
@@ -1661,7 +1661,7 @@
// Set all free positions for possible registers to max integer.
RegisterPositions freePositions = new RegisterPositionsImpl(registerConstraint + 1);
- if ((options().debug || code.context().isReachabilitySensitive())
+ if ((options().debug || code.context().getOrComputeReachabilitySensitive(appView))
&& !code.method().accessFlags.isStatic()) {
// If we are generating debug information or if the method is reachability sensitive,
// we pin the this value register. The debugger expects to always be able to find it in
@@ -2712,7 +2712,7 @@
}
}
}
- if (appView.options().debug || code.context().isReachabilitySensitive()) {
+ if (appView.options().debug || code.context().getOrComputeReachabilitySensitive(appView)) {
// In debug mode, or if the method is reachability sensitive, extend the live range
// to cover the full scope of a local variable (encoded as debug values).
int number = instruction.getNumber();
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 af43714..8eac490 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerDeferredTracingImpl.java
@@ -97,7 +97,7 @@
}
// If the access is from a reachability sensitive method, then bail out.
- if (context.getHolder().isReachabilitySensitive()) {
+ if (context.getHolder().getOrComputeReachabilitySensitive(appView)) {
return enqueueDeferredEnqueuerActions(field);
}
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticClassBuilder.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticClassBuilder.java
index 44324b7..068a1a7 100644
--- a/src/main/java/com/android/tools/r8/synthesis/SyntheticClassBuilder.java
+++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticClassBuilder.java
@@ -220,9 +220,6 @@
}
clazz.setDirectMethods(directMethods.toArray(DexEncodedMethod.EMPTY_ARRAY));
clazz.setVirtualMethods(virtualMethods.toArray(DexEncodedMethod.EMPTY_ARRAY));
- if (clazz.isProgramClass()) {
- clazz.asProgramClass().setReachabilitySensitive(false);
- }
return clazz;
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 1a92b00..7a109a2 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -917,12 +917,12 @@
public boolean debug = false;
- public boolean shouldCompileMethodInDebugMode(ProgramMethod method) {
- return debug || method.isReachabilitySensitive();
+ public boolean shouldCompileMethodInDebugMode(AppView<?> appView, ProgramMethod method) {
+ return debug || method.getOrComputeReachabilitySensitive(appView);
}
public boolean shouldCompileMethodInReleaseMode(AppView<?> appView, ProgramMethod method) {
- return !shouldCompileMethodInDebugMode(method);
+ return !shouldCompileMethodInDebugMode(appView, method);
}
private final AccessModifierOptions accessModifierOptions = new AccessModifierOptions(this);
