Replace receiver check null analysis and place when inlining
Bug: 206427323
Bug: 215339687
Bug: 214377135
Change-Id: I525453812223767a4105c42791f795facdcfac76
diff --git a/src/main/java/com/android/tools/r8/ir/code/Position.java b/src/main/java/com/android/tools/r8/ir/code/Position.java
index 9fd0c74..5de6390 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Position.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Position.java
@@ -180,6 +180,16 @@
.build();
}
+ public Position replaceOutermostCallerPosition(Position newOutermostCallerPosition) {
+ if (!hasCallerPosition()) {
+ return newOutermostCallerPosition;
+ }
+ return builderWithCopy()
+ .setCallerPosition(
+ getCallerPosition().replaceOutermostCallerPosition(newOutermostCallerPosition))
+ .build();
+ }
+
@Override
public final boolean equals(Object other) {
return Equatable.equalsImpl(this, other);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/MethodOptimizationFeedback.java b/src/main/java/com/android/tools/r8/ir/conversion/MethodOptimizationFeedback.java
index 8b229f9..0da54ff 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/MethodOptimizationFeedback.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/MethodOptimizationFeedback.java
@@ -54,8 +54,6 @@
void markAsPropagated(DexEncodedMethod method);
- void markCheckNullReceiverBeforeAnySideEffect(DexEncodedMethod method, boolean mark);
-
void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark);
void setBridgeInfo(DexEncodedMethod method, BridgeInfo bridgeInfo);
@@ -87,8 +85,6 @@
void unsetBridgeInfo(DexEncodedMethod method);
- void unsetCheckNullReceiverBeforeAnySideEffect(ProgramMethod method);
-
void unsetClassInitializerMayBePostponed(ProgramMethod method);
void unsetClassInlinerMethodConstraint(ProgramMethod method);
@@ -131,7 +127,6 @@
if (method.getOptimizationInfo().isMutableOptimizationInfo()) {
unsetAbstractReturnValue(method);
unsetBridgeInfo(method.getDefinition());
- unsetCheckNullReceiverBeforeAnySideEffect(method);
unsetClassInitializerMayBePostponed(method);
unsetClassInlinerMethodConstraint(method);
unsetDynamicReturnType(method);
@@ -158,8 +153,5 @@
methodNeverReturnsNormally(method);
setUnusedArguments(
method, BitSetUtils.createFilled(true, method.getDefinition().getNumberOfArguments()));
- if (method.getDefinition().isInstance()) {
- markCheckNullReceiverBeforeAnySideEffect(method.getDefinition(), true);
- }
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 4e6bea5..a39688a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -374,15 +374,9 @@
// receiver. Therefore, if the receiver may be null and the candidate inlinee does not
// throw if the receiver is null before any other side effect, then we must synthesize a
// null check.
- if (!singleTarget
- .getDefinition()
- .getOptimizationInfo()
- .checksNullReceiverBeforeAnySideEffect()) {
- if (!inlinerOptions.enableInliningOfInvokesWithNullableReceivers) {
- whyAreYouNotInliningReporter.reportReceiverMaybeNull();
- return null;
- }
- action.setShouldSynthesizeNullCheckForReceiver();
+ if (!inlinerOptions.enableInliningOfInvokesWithNullableReceivers) {
+ whyAreYouNotInliningReporter.reportReceiverMaybeNull();
+ return null;
}
}
return action;
@@ -484,9 +478,7 @@
ProgramMethod singleTarget,
InliningIRProvider inliningIRProvider,
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
- boolean removeInnerFramesIfThrowingNpe = false;
- IRCode inlinee =
- inliningIRProvider.getInliningIR(invoke, singleTarget, removeInnerFramesIfThrowingNpe);
+ IRCode inlinee = inliningIRProvider.getInliningIR(invoke, singleTarget);
// In the Java VM Specification section "4.10.2.4. Instance Initialization Methods and
// Newly Created Objects" it says:
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 785c461..834e78d 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
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.optimize;
import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull;
+import static com.android.tools.r8.ir.optimize.SimpleDominatingEffectAnalysis.canInlineWithoutSynthesizingNullCheckForReceiver;
import static com.android.tools.r8.utils.MapUtils.ignoreKey;
import static com.google.common.base.Predicates.not;
@@ -51,6 +52,7 @@
import com.android.tools.r8.ir.conversion.LensCodeRewriter;
import com.android.tools.r8.ir.conversion.MethodProcessor;
import com.android.tools.r8.ir.conversion.PostMethodProcessor;
+import com.android.tools.r8.ir.optimize.SimpleDominatingEffectAnalysis.SimpleEffectAnalysisResult;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackIgnore;
import com.android.tools.r8.ir.optimize.inliner.DefaultInliningReasonStrategy;
@@ -515,7 +517,6 @@
final Reason reason;
private boolean shouldSynthesizeInitClass;
- private boolean shouldSynthesizeNullCheckForReceiver;
private DexProgramClass downcastClass;
@@ -539,15 +540,9 @@
}
void setShouldSynthesizeInitClass() {
- assert !shouldSynthesizeNullCheckForReceiver;
shouldSynthesizeInitClass = true;
}
- void setShouldSynthesizeNullCheckForReceiver() {
- assert !shouldSynthesizeInitClass;
- shouldSynthesizeNullCheckForReceiver = true;
- }
-
InlineeWithReason buildInliningIR(
AppView<? extends AppInfoWithClassHierarchy> appView,
InvokeMethod invoke,
@@ -558,12 +553,7 @@
InternalOptions options = appView.options();
// Build the IR for a yet not processed method, and perform minimal IR processing.
- boolean removeInnerFramesIfThrowingNpe =
- invoke.isInvokeMethodWithReceiver()
- && invoke.asInvokeMethodWithReceiver().getReceiver().isMaybeNull()
- && !shouldSynthesizeNullCheckForReceiver;
- IRCode code =
- inliningIRProvider.getInliningIR(invoke, target, removeInnerFramesIfThrowingNpe);
+ IRCode code = inliningIRProvider.getInliningIR(invoke, target);
// Insert a init class instruction if this is needed to preserve class initialization
// semantics.
@@ -582,11 +572,24 @@
target.getDefinition().isSynchronized() && options.isGeneratingClassFiles();
boolean isSynthesizingNullCheckForReceiverUsingMonitorEnter =
shouldSynthesizeMonitorEnterExit && !target.getDefinition().isStatic();
- if (shouldSynthesizeNullCheckForReceiver
+ if (invoke.isInvokeMethodWithReceiver()
+ && invoke.asInvokeMethodWithReceiver().getReceiver().isMaybeNull()
&& !isSynthesizingNullCheckForReceiverUsingMonitorEnter) {
- synthesizeNullCheckForReceiver(appView, code, invoke);
+ SimpleEffectAnalysisResult checksReceiverBeingNull =
+ canInlineWithoutSynthesizingNullCheckForReceiver(appView, code);
+ if (!checksReceiverBeingNull.hasResult()
+ || !checksReceiverBeingNull.isPartial()
+ || (checksReceiverBeingNull.isPartial()
+ && checksReceiverBeingNull.topMostNotSatisfiedBlockSize() > 1)) {
+ synthesizeNullCheckForReceiver(appView, code, invoke, code.entryBlock());
+ } else {
+ checksReceiverBeingNull.forEachSatisfyingInstruction(
+ this::setRemoveInnerFramePositionForReceiverUse);
+ // Also add a null check on failing paths
+ checksReceiverBeingNull.forEachTopMostNotSatisfiedBlock(
+ block -> synthesizeNullCheckForReceiver(appView, code, invoke, block));
+ }
}
-
// Insert monitor-enter and monitor-exit instructions if the method is synchronized.
if (shouldSynthesizeMonitorEnterExit) {
TypeElement throwableType =
@@ -727,18 +730,15 @@
}
private void synthesizeNullCheckForReceiver(
- AppView<?> appView, IRCode code, InvokeMethod invoke) {
+ AppView<?> appView, IRCode code, InvokeMethod invoke, BasicBlock block) {
List<Value> arguments = code.collectArguments();
if (!arguments.isEmpty()) {
Value receiver = arguments.get(0);
assert receiver.isThis();
-
- BasicBlock entryBlock = code.entryBlock();
-
// Insert a new block between the last argument instruction and the first actual
// instruction of the method.
BasicBlock throwBlock =
- entryBlock.listIterator(code, arguments.size()).split(code, 0, null);
+ block.listIterator(code, block.isEntry() ? arguments.size() : 0).split(code, 0, null);
assert !throwBlock.hasCatchHandlers();
InstructionListIterator iterator = throwBlock.listIterator(code);
@@ -754,6 +754,21 @@
assert false : "Unable to synthesize a null check for the receiver";
}
}
+
+ private void setRemoveInnerFramePositionForReceiverUse(Instruction instruction) {
+ Position position = instruction.getPosition();
+ if (position == null) {
+ assert false : "Expected position for inlinee call to receiver";
+ return;
+ }
+ Position removeInnerFrame =
+ position
+ .getOutermostCaller()
+ .builderWithCopy()
+ .setRemoveInnerFramesIfThrowingNpe(true)
+ .build();
+ instruction.forceOverwritePosition(position.replaceOutermostCallerPosition(removeInnerFrame));
+ }
}
public static class RetryAction extends InlineResult {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/SimpleDominatingEffectAnalysis.java b/src/main/java/com/android/tools/r8/ir/optimize/SimpleDominatingEffectAnalysis.java
new file mode 100644
index 0000000..8942395
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/optimize/SimpleDominatingEffectAnalysis.java
@@ -0,0 +1,324 @@
+// Copyright (c) 2022, 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.ir.optimize;
+
+import static com.android.tools.r8.ir.optimize.SimpleDominatingEffectAnalysis.InstructionEffect.NO_EFFECT;
+import static com.android.tools.r8.ir.optimize.SimpleDominatingEffectAnalysis.InstructionEffect.OTHER_EFFECT;
+import static com.android.tools.r8.utils.TraversalContinuation.BREAK;
+import static com.android.tools.r8.utils.TraversalContinuation.CONTINUE;
+
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.utils.DepthFirstSearchWorkListBase.DFSNodeWithState;
+import com.android.tools.r8.utils.DepthFirstSearchWorkListBase.StatefulDepthFirstSearchWorkList;
+import com.android.tools.r8.utils.IntBox;
+import com.android.tools.r8.utils.TraversalContinuation;
+import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Consumer;
+import java.util.function.Function;
+
+public class SimpleDominatingEffectAnalysis {
+
+ public enum InstructionEffect {
+ NO_EFFECT,
+ DESIRED_EFFECT,
+ OTHER_EFFECT;
+
+ public boolean isDesired() {
+ return this == DESIRED_EFFECT;
+ }
+
+ public boolean isOther() {
+ return this == OTHER_EFFECT;
+ }
+
+ public boolean isNoEffect() {
+ return this == NO_EFFECT;
+ }
+
+ public static InstructionEffect fromBoolean(boolean value) {
+ return value ? DESIRED_EFFECT : OTHER_EFFECT;
+ }
+
+ public ResultState toResultState() {
+ switch (this) {
+ case NO_EFFECT:
+ return ResultState.NOT_COMPUTED;
+ case DESIRED_EFFECT:
+ return ResultState.SATISFIED;
+ default:
+ assert isOther();
+ return ResultState.NOT_SATISFIED;
+ }
+ }
+ }
+
+ /**
+ * ResultState encodes a lattice on the following form: PARTIAL / \ SATISFIED NOT_SATISFIED \ /
+ * NOT_COMPUTED
+ *
+ * <p>PARTIAL results occur when have control flow where one or more branches are SATISFIED and
+ * one or more branches are NOT_SATISFIED.
+ */
+ private enum ResultState {
+ PARTIAL,
+ SATISFIED,
+ NOT_SATISFIED,
+ NOT_COMPUTED;
+
+ public boolean isPartial() {
+ return this == PARTIAL;
+ }
+
+ public boolean isSatisfied() {
+ return this == SATISFIED;
+ }
+
+ public boolean isNotSatisfied() {
+ return this == NOT_SATISFIED;
+ }
+
+ public boolean isNotComputed() {
+ return this == NOT_COMPUTED;
+ }
+
+ public ResultState join(ResultState other) {
+ if (isPartial() || other.isNotComputed()) {
+ return this;
+ }
+ if (isNotComputed() || other.isPartial()) {
+ return other;
+ }
+ if (this == other) {
+ return this;
+ }
+ return PARTIAL;
+ }
+ }
+
+ private static class ResultStateWithPartialBlocks {
+
+ private final ResultState state;
+ private final List<BasicBlock> failingBlocks;
+
+ private ResultStateWithPartialBlocks(ResultState state, List<BasicBlock> failingBlocks) {
+ this.state = state;
+ this.failingBlocks = failingBlocks;
+ }
+
+ public ResultStateWithPartialBlocks joinChildren(
+ List<DFSNodeWithState<BasicBlock, ResultStateWithPartialBlocks>> childNodes) {
+ assert state.isNotComputed();
+ ResultState newState =
+ childNodes.isEmpty() ? ResultState.NOT_SATISFIED : ResultState.NOT_COMPUTED;
+ for (DFSNodeWithState<BasicBlock, ResultStateWithPartialBlocks> childNode : childNodes) {
+ ResultStateWithPartialBlocks childState = childNode.getState();
+ assert !childState.state.isNotComputed();
+ newState = newState.join(childState.state);
+ }
+ assert !newState.isNotComputed();
+ List<BasicBlock> newFailingBlocks = new ArrayList<>();
+ if (newState.isPartial()) {
+ // Compute the initial basic blocks where that leads to OTHER_EFFECT.
+ for (DFSNodeWithState<BasicBlock, ResultStateWithPartialBlocks> childNode : childNodes) {
+ if (childNode.getState().state.isNotSatisfied()) {
+ newFailingBlocks.add(childNode.getNode());
+ } else if (childNode.getState().state.isPartial()) {
+ newFailingBlocks.addAll(childNode.getState().failingBlocks);
+ }
+ }
+ }
+ return new ResultStateWithPartialBlocks(newState, newFailingBlocks);
+ }
+ }
+
+ // The instruction analysis drives the SimpleDominatingEffectAnalysis by assigning an effect
+ // to a basic block.
+ public interface InstructionAnalysis {
+
+ // Analyse the instruction and assign and effect. If the desired effect is seen, it should
+ // dominate other users and DESIRED_EFFECT should be returned. If a violating effect is seen
+ // return OTHER_EFFECT. When an effect is seen there is no need to visit successor instructions
+ // in the block or successor blocks.
+ // If the instruction do not violate the effect, use NO_EFFECT. The analysis will, if the entire
+ // block is visited without any effect, visit successor blocks until an effect is found, the
+ // depth is violated or we see a return for which the result of this path will be notSatisfied.
+ InstructionEffect analyze(Instruction instruction);
+
+ // Return the successors of the block. The default is to only look at normal successors.
+ default List<BasicBlock> getSuccessors(BasicBlock block) {
+ return block.getNormalSuccessors();
+ }
+
+ // The max bound on instructions to consider before giving up.
+ default int maxNumberOfInstructions() {
+ return 100;
+ }
+ }
+
+ public static class SimpleEffectAnalysisResult {
+
+ private final List<Instruction> satisfyingInstructions;
+ private final List<BasicBlock> topmostNotSatisfiedBlocks;
+
+ private SimpleEffectAnalysisResult(
+ List<Instruction> satisfyingInstructions, List<BasicBlock> topmostNotSatisfiedBlocks) {
+
+ this.satisfyingInstructions = satisfyingInstructions;
+ this.topmostNotSatisfiedBlocks = topmostNotSatisfiedBlocks;
+ }
+
+ public boolean hasResult() {
+ return true;
+ }
+
+ public void forEachSatisfyingInstruction(Consumer<Instruction> instructionConsumer) {
+ satisfyingInstructions.forEach(instructionConsumer);
+ }
+
+ public void forEachTopMostNotSatisfiedBlock(Consumer<BasicBlock> blockConsumer) {
+ topmostNotSatisfiedBlocks.forEach(blockConsumer);
+ }
+
+ public int topMostNotSatisfiedBlockSize() {
+ return topmostNotSatisfiedBlocks.size();
+ }
+
+ public static SimpleEffectAnalysisResultBuilder builder() {
+ return new SimpleEffectAnalysisResultBuilder();
+ }
+
+ public boolean isPartial() {
+ return !satisfyingInstructions.isEmpty() && !topmostNotSatisfiedBlocks.isEmpty();
+ }
+ }
+
+ private static class SimpleEffectAnalysisResultBuilder {
+
+ List<Instruction> satisfyingInstructions = new ArrayList<>();
+ List<BasicBlock> failingBlocksForPartialResults = ImmutableList.of();
+ private boolean isFailed;
+
+ public void fail() {
+ isFailed = true;
+ }
+
+ public void addSatisfyingInstruction(Instruction instruction) {
+ satisfyingInstructions.add(instruction);
+ }
+
+ public void setFailingBlocksForPartialResults(List<BasicBlock> basicBlocks) {
+ this.failingBlocksForPartialResults = basicBlocks;
+ }
+
+ public SimpleEffectAnalysisResult build() {
+ return isFailed
+ ? NO_RESULT
+ : new SimpleEffectAnalysisResult(satisfyingInstructions, failingBlocksForPartialResults);
+ }
+ }
+
+ private static final SimpleEffectAnalysisResult NO_RESULT =
+ new SimpleEffectAnalysisResult(ImmutableList.of(), ImmutableList.of()) {
+ @Override
+ public boolean hasResult() {
+ return false;
+ }
+ };
+
+ public static SimpleEffectAnalysisResult run(IRCode code, InstructionAnalysis analysis) {
+ SimpleEffectAnalysisResultBuilder builder = SimpleEffectAnalysisResult.builder();
+ IntBox visitedInstructions = new IntBox();
+ new StatefulDepthFirstSearchWorkList<BasicBlock, ResultStateWithPartialBlocks>() {
+
+ @Override
+ protected TraversalContinuation process(
+ DFSNodeWithState<BasicBlock, ResultStateWithPartialBlocks> node,
+ Function<BasicBlock, DFSNodeWithState<BasicBlock, ResultStateWithPartialBlocks>>
+ childNodeConsumer) {
+ InstructionEffect effect = NO_EFFECT;
+ for (Instruction instruction : node.getNode().getInstructions()) {
+ if (visitedInstructions.getAndIncrement() > analysis.maxNumberOfInstructions()) {
+ builder.fail();
+ return BREAK;
+ }
+ effect = analysis.analyze(instruction);
+ if (!effect.isNoEffect()) {
+ if (effect.isDesired()) {
+ builder.addSatisfyingInstruction(instruction);
+ }
+ break;
+ }
+ }
+ if (effect.isNoEffect()) {
+ List<BasicBlock> successors = analysis.getSuccessors(node.getNode());
+ for (BasicBlock successor : successors) {
+ DFSNodeWithState<BasicBlock, ResultStateWithPartialBlocks> childNode =
+ childNodeConsumer.apply(successor);
+ if (childNode.hasState()) {
+ // If we see a block where the children have not been processed we cannot guarantee
+ // all paths having the effect since - ex. we could have a non-terminating loop.
+ builder.fail();
+ return BREAK;
+ }
+ }
+ }
+ node.setState(new ResultStateWithPartialBlocks(effect.toResultState(), ImmutableList.of()));
+ return CONTINUE;
+ }
+
+ @Override
+ protected TraversalContinuation joiner(
+ DFSNodeWithState<BasicBlock, ResultStateWithPartialBlocks> node,
+ List<DFSNodeWithState<BasicBlock, ResultStateWithPartialBlocks>> childNodes) {
+ ResultStateWithPartialBlocks resultState = node.getState();
+ if (resultState.state.isNotComputed()) {
+ resultState = resultState.joinChildren(childNodes);
+ } else {
+ assert resultState.state.isSatisfied() || resultState.state.isNotSatisfied();
+ assert childNodes.isEmpty();
+ }
+ node.setState(resultState);
+ if (node.getNode().isEntry()) {
+ builder.setFailingBlocksForPartialResults(resultState.failingBlocks);
+ }
+ return CONTINUE;
+ }
+ }.run(code.entryBlock());
+
+ return builder.build();
+ }
+
+ public static SimpleEffectAnalysisResult canInlineWithoutSynthesizingNullCheckForReceiver(
+ AppView<?> appView, IRCode code) {
+ assert code.context().getDefinition().isVirtualMethod();
+ Value receiver = code.getThis();
+ if (!receiver.isUsed()) {
+ return NO_RESULT;
+ }
+ ProgramMethod context = code.context();
+ return run(
+ code,
+ instruction -> {
+ if ((instruction.isInvokeMethodWithReceiver()
+ && instruction.asInvokeMethodWithReceiver().getReceiver() == receiver)
+ || (instruction.isInstanceFieldInstruction()
+ && instruction.asInstanceFieldInstruction().object() == receiver)
+ || (instruction.isMonitorEnter() && instruction.asMonitor().object() == receiver)) {
+ // Conservatively bailout if there are catch handlers.
+ return InstructionEffect.fromBoolean(!instruction.getBlock().hasCatchHandlers());
+ }
+ return instruction.instructionMayHaveSideEffects(appView, context)
+ ? OTHER_EFFECT
+ : NO_EFFECT;
+ });
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
index 06e2b18..93b5d8f 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerCostAnalysis.java
@@ -75,7 +75,7 @@
// Not a direct inlinee.
continue;
}
- IRCode inliningIR = inliningIRProvider.getAndCacheInliningIR(invoke, inlinee, false);
+ IRCode inliningIR = inliningIRProvider.getAndCacheInliningIR(invoke, inlinee);
int increment =
inlinee.getDefinition().getCode().estimatedSizeForInlining()
- estimateSizeOfNonMaterializingInstructions(invoke, inliningIR);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
index 09b1cc3..95483ce 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/DefaultMethodOptimizationInfo.java
@@ -31,7 +31,6 @@
static int UNKNOWN_RETURNED_ARGUMENT = -1;
static boolean UNKNOWN_NEVER_RETURNS_NORMALLY = false;
static AbstractValue UNKNOWN_ABSTRACT_RETURN_VALUE = UnknownValue.getInstance();
- static boolean UNKNOWN_CHECKS_NULL_RECEIVER_BEFORE_ANY_SIDE_EFFECT = false;
static boolean UNKNOWN_TRIGGERS_CLASS_INIT_BEFORE_ANY_SIDE_EFFECT = false;
static boolean UNKNOWN_INITIALIZER_ENABLING_JAVA_ASSERTIONS = false;
static boolean UNKNOWN_MAY_HAVE_SIDE_EFFECTS = true;
@@ -167,11 +166,6 @@
}
@Override
- public boolean checksNullReceiverBeforeAnySideEffect() {
- return UNKNOWN_CHECKS_NULL_RECEIVER_BEFORE_ANY_SIDE_EFFECT;
- }
-
- @Override
public boolean triggersClassInitBeforeAnySideEffect() {
return UNKNOWN_TRIGGERS_CLASS_INIT_BEFORE_ANY_SIDE_EFFECT;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
index a4667c9..17cffe5 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfo.java
@@ -88,8 +88,6 @@
public abstract boolean forceInline();
- public abstract boolean checksNullReceiverBeforeAnySideEffect();
-
public abstract boolean triggersClassInitBeforeAnySideEffect();
public abstract boolean mayHaveSideEffects();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
index a0c4eb2..acf0da6 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -542,11 +542,6 @@
// Identifies if the method preserves class initialization after inlining.
feedback.markTriggerClassInitBeforeAnySideEffect(
method, triggersClassInitializationBeforeSideEffect(code));
- } else {
- // Identifies if the method preserves null check of the receiver after inlining.
- Value receiver = code.getThis();
- feedback.markCheckNullReceiverBeforeAnySideEffect(
- method, receiver.isUsed() && checksNullBeforeSideEffect(code, receiver));
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
index 64bdb91..69f0497 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableMethodOptimizationInfo.java
@@ -89,7 +89,7 @@
private static final int UNUSED_FLAG_1 = 0x20;
private static final int NEVER_RETURNS_NORMALLY_FLAG = 0x40;
private static final int UNUSED_FLAG_2 = 0x80;
- private static final int CHECKS_NULL_RECEIVER_BEFORE_ANY_SIDE_EFFECT_FLAG = 0x100;
+ private static final int UNUSED_FLAG_3 = 0x100;
private static final int TRIGGERS_CLASS_INIT_BEFORE_ANY_SIDE_EFFECT_FLAG = 0x200;
private static final int INITIALIZER_ENABLING_JAVA_ASSERTIONS_FLAG = 0x400;
private static final int REACHABILITY_SENSITIVE_FLAG = 0x800;
@@ -116,9 +116,7 @@
defaultFlags |=
BooleanUtils.intValue(defaultOptInfo.neverReturnsNormally()) * NEVER_RETURNS_NORMALLY_FLAG;
defaultFlags |= 0 * UNUSED_FLAG_2;
- defaultFlags |=
- BooleanUtils.intValue(defaultOptInfo.checksNullReceiverBeforeAnySideEffect())
- * CHECKS_NULL_RECEIVER_BEFORE_ANY_SIDE_EFFECT_FLAG;
+ defaultFlags |= 0 * UNUSED_FLAG_3;
defaultFlags |=
BooleanUtils.intValue(defaultOptInfo.triggersClassInitBeforeAnySideEffect())
* TRIGGERS_CLASS_INIT_BEFORE_ANY_SIDE_EFFECT_FLAG;
@@ -488,11 +486,6 @@
}
@Override
- public boolean checksNullReceiverBeforeAnySideEffect() {
- return isFlagSet(CHECKS_NULL_RECEIVER_BEFORE_ANY_SIDE_EFFECT_FLAG);
- }
-
- @Override
public boolean triggersClassInitBeforeAnySideEffect() {
return isFlagSet(TRIGGERS_CLASS_INIT_BEFORE_ANY_SIDE_EFFECT_FLAG);
}
@@ -671,14 +664,6 @@
}
}
- void markCheckNullReceiverBeforeAnySideEffect(boolean mark) {
- setFlag(CHECKS_NULL_RECEIVER_BEFORE_ANY_SIDE_EFFECT_FLAG, mark);
- }
-
- void unsetCheckNullReceiverBeforeAnySideEffect() {
- clearFlag(CHECKS_NULL_RECEIVER_BEFORE_ANY_SIDE_EFFECT_FLAG);
- }
-
void markTriggerClassInitBeforeAnySideEffect(boolean mark) {
setFlag(TRIGGERS_CLASS_INIT_BEFORE_ANY_SIDE_EFFECT_FLAG, mark);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java
index 5e9386c..4b6241a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackDelayed.java
@@ -229,12 +229,6 @@
}
@Override
- public synchronized void markCheckNullReceiverBeforeAnySideEffect(
- DexEncodedMethod method, boolean mark) {
- getMethodOptimizationInfoForUpdating(method).markCheckNullReceiverBeforeAnySideEffect(mark);
- }
-
- @Override
public synchronized void markTriggerClassInitBeforeAnySideEffect(
DexEncodedMethod method, boolean mark) {
getMethodOptimizationInfoForUpdating(method).markTriggerClassInitBeforeAnySideEffect(mark);
@@ -311,11 +305,6 @@
}
@Override
- public synchronized void unsetCheckNullReceiverBeforeAnySideEffect(ProgramMethod method) {
- getMethodOptimizationInfoForUpdating(method).unsetCheckNullReceiverBeforeAnySideEffect();
- }
-
- @Override
public synchronized void unsetClassInitializerMayBePostponed(ProgramMethod method) {
getMethodOptimizationInfoForUpdating(method).unsetClassInitializerMayBePostponed();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java
index fd6d27e..ac66916 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackIgnore.java
@@ -94,9 +94,6 @@
public void markProcessed(DexEncodedMethod method, ConstraintWithTarget state) {}
@Override
- public void markCheckNullReceiverBeforeAnySideEffect(DexEncodedMethod method, boolean mark) {}
-
- @Override
public void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark) {}
@Override
@@ -143,9 +140,6 @@
public void unsetBridgeInfo(DexEncodedMethod method) {}
@Override
- public void unsetCheckNullReceiverBeforeAnySideEffect(ProgramMethod method) {}
-
- @Override
public void unsetClassInitializerMayBePostponed(ProgramMethod method) {}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
index a763db6..5be58f0 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/OptimizationFeedbackSimple.java
@@ -137,11 +137,6 @@
}
@Override
- public void markCheckNullReceiverBeforeAnySideEffect(DexEncodedMethod method, boolean mark) {
- method.getMutableOptimizationInfo().markCheckNullReceiverBeforeAnySideEffect(mark);
- }
-
- @Override
public void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark) {
// Ignored.
}
@@ -234,12 +229,6 @@
}
@Override
- public void unsetCheckNullReceiverBeforeAnySideEffect(ProgramMethod method) {
- withMutableMethodOptimizationInfo(
- method, MutableMethodOptimizationInfo::unsetCheckNullReceiverBeforeAnySideEffect);
- }
-
- @Override
public void unsetClassInitializerMayBePostponed(ProgramMethod method) {
withMutableMethodOptimizationInfo(
method, MutableMethodOptimizationInfo::unsetClassInitializerMayBePostponed);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/inliner/InliningIRProvider.java b/src/main/java/com/android/tools/r8/ir/optimize/inliner/InliningIRProvider.java
index 677ccdd..958b02d 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/inliner/InliningIRProvider.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/inliner/InliningIRProvider.java
@@ -43,14 +43,12 @@
public static InliningIRProvider getThrowingInstance() {
return new InliningIRProvider() {
@Override
- public IRCode getInliningIR(
- InvokeMethod invoke, ProgramMethod method, boolean removeInnerFramesIfNpe) {
+ public IRCode getInliningIR(InvokeMethod invoke, ProgramMethod method) {
throw new Unreachable();
}
@Override
- public IRCode getAndCacheInliningIR(
- InvokeMethod invoke, ProgramMethod method, boolean removeInnerFrameIfThrowingNpe) {
+ public IRCode getAndCacheInliningIR(InvokeMethod invoke, ProgramMethod method) {
throw new Unreachable();
}
@@ -76,24 +74,23 @@
};
}
- public IRCode getInliningIR(
- InvokeMethod invoke, ProgramMethod method, boolean removeInnerFramesIfNpe) {
+ public IRCode getInliningIR(InvokeMethod invoke, ProgramMethod method) {
IRCode cached = cache.remove(invoke);
if (cached != null) {
return cached;
}
- Position position = Position.getPositionForInlining(appView, invoke, context);
- if (removeInnerFramesIfNpe) {
- position = position.builderWithCopy().setRemoveInnerFramesIfThrowingNpe(true).build();
- }
Origin origin = method.getOrigin();
return method.buildInliningIR(
- context, appView, valueNumberGenerator, position, origin, methodProcessor);
+ context,
+ appView,
+ valueNumberGenerator,
+ Position.getPositionForInlining(appView, invoke, context),
+ origin,
+ methodProcessor);
}
- public IRCode getAndCacheInliningIR(
- InvokeMethod invoke, ProgramMethod method, boolean removeInnerFrameIfThrowingNpe) {
- IRCode inliningIR = getInliningIR(invoke, method, removeInnerFrameIfThrowingNpe);
+ public IRCode getAndCacheInliningIR(InvokeMethod invoke, ProgramMethod method) {
+ IRCode inliningIR = getInliningIR(invoke, method);
cacheInliningIR(invoke, inliningIR);
return inliningIR;
}
diff --git a/src/main/java/com/android/tools/r8/utils/DepthFirstSearchWorkListBase.java b/src/main/java/com/android/tools/r8/utils/DepthFirstSearchWorkListBase.java
index 5900602..c82f332 100644
--- a/src/main/java/com/android/tools/r8/utils/DepthFirstSearchWorkListBase.java
+++ b/src/main/java/com/android/tools/r8/utils/DepthFirstSearchWorkListBase.java
@@ -31,6 +31,8 @@
S getState();
void setState(S backtrackState);
+
+ boolean hasState();
}
enum ProcessingState {
@@ -93,6 +95,11 @@
public void setState(S state) {
this.state = state;
}
+
+ @Override
+ public boolean hasState() {
+ return state != null;
+ }
}
private final ArrayDeque<T> workList = new ArrayDeque<>();
diff --git a/src/main/java/com/android/tools/r8/utils/WorkList.java b/src/main/java/com/android/tools/r8/utils/WorkList.java
index 70faab7..79c2ed7 100644
--- a/src/main/java/com/android/tools/r8/utils/WorkList.java
+++ b/src/main/java/com/android/tools/r8/utils/WorkList.java
@@ -92,6 +92,18 @@
return false;
}
+ public boolean addFirstIfNotSeen(T item) {
+ if (seen.add(item)) {
+ workingList.addFirst(item);
+ return true;
+ }
+ return false;
+ }
+
+ public void addFirstIgnoringSeenSet(T item) {
+ workingList.addFirst(item);
+ }
+
public boolean hasNext() {
return !workingList.isEmpty();
}
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineBranchTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineBranchTest.java
new file mode 100644
index 0000000..92b6019
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineBranchTest.java
@@ -0,0 +1,157 @@
+// Copyright (c) 2022, 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.naming.retrace;
+
+import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
+import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForLineNumbers;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.naming.retrace.StackTrace.StackTraceLine;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class RetraceInlineBranchTest extends TestBase {
+
+ @Parameter() public TestParameters parameters;
+
+ public StackTrace expectedStackTrace;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Before
+ public void setup() throws Exception {
+ // Get the expected stack trace by running on the JVM.
+ expectedStackTrace =
+ testForRuntime(parameters)
+ .addInnerClasses(getClass())
+ .run(parameters.getRuntime(), Main.class)
+ .getStackTrace();
+ }
+
+ @Test
+ public void testR8() throws Throwable {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .setMinApi(parameters.getApiLevel())
+ .addKeepMainRule(Main.class)
+ .addKeepAttributeLineNumberTable()
+ .enableNeverClassInliningAnnotations()
+ .enableInliningAnnotations()
+ .run(parameters.getRuntime(), Main.class)
+ .inspectStackTrace(
+ (stackTrace, inspector) -> {
+ if (canUseJavaUtilObjectsRequireNonNull(parameters)) {
+ StackTrace requireNonNullFrame =
+ StackTrace.builder().add(stackTrace.get(0)).build();
+ assertThat(
+ requireNonNullFrame,
+ isSameExceptForLineNumbers(
+ StackTrace.builder()
+ .add(
+ StackTraceLine.builder()
+ .setClassName(Objects.class.getTypeName())
+ .setMethodName("requireNonNull")
+ .setFileName("Objects.java")
+ .build())
+ .build()));
+
+ StackTrace stackTraceWithoutRequireNonNull =
+ StackTrace.builder().add(stackTrace).remove(0).build();
+ assertThat(stackTraceWithoutRequireNonNull, isSame(expectedStackTrace));
+ } else {
+ // To check that we inserted the check in the branch correctly we use a rudimentary
+ // line check by comparing that the getClass method comes later than the first if.
+ MethodSubject methodSubject =
+ inspector.clazz(Main.class).uniqueMethodWithName("call");
+ assertThat(methodSubject, isPresent());
+ List<InstructionSubject> getClassCalls =
+ methodSubject
+ .streamInstructions()
+ .filter(
+ instructionSubject ->
+ instructionSubject.isInvoke()
+ && instructionSubject
+ .getMethod()
+ .qualifiedName()
+ .contains("getClass"))
+ .collect(Collectors.toList());
+ assertEquals(1, getClassCalls.size());
+ Optional<InstructionSubject> firstIf =
+ methodSubject.streamInstructions().filter(InstructionSubject::isIf).findFirst();
+ assertTrue(firstIf.isPresent());
+ assertTrue(
+ methodSubject.getLineNumberForInstruction(getClassCalls.get(0))
+ > methodSubject.getLineNumberForInstruction(firstIf.get()));
+ assertThat(stackTrace, isSame(expectedStackTrace));
+ }
+ });
+ }
+
+ @NeverClassInline
+ static class Foo {
+
+ private int value;
+
+ @NeverInline
+ int getValue() {
+ return value;
+ }
+
+ @NeverInline
+ void setValue(int value) {
+ this.value = value;
+ }
+
+ void inlinable(int arg) {
+ String newValue;
+ if (arg > 0) {
+ newValue = getValue() + "";
+ } else {
+ if (arg < 0) {
+ setValue(arg);
+ newValue = "-1";
+ } else {
+ // When inlining this block this is the only path that needs a null-check.
+ newValue = "Hello World";
+ }
+ }
+ System.out.println(newValue);
+ }
+ }
+
+ static class Main {
+
+ @NeverInline
+ private static void call(Foo foo, String[] args) {
+ foo.inlinable(args.length);
+ }
+
+ public static void main(String[] args) {
+ call(args.length == 0 ? null : new Foo(), args);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineConditionTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineConditionTest.java
index 15466f7..36921ce 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineConditionTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineConditionTest.java
@@ -4,13 +4,16 @@
package com.android.tools.r8.naming.retrace;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
-import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.SingleTestRunResult;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestBuilder;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import org.hamcrest.CoreMatchers;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -22,25 +25,39 @@
@Parameter() public TestParameters parameters;
+ public StackTrace expectedStackTrace;
+
@Parameters(name = "{0}")
public static TestParametersCollection data() {
return getTestParameters().withAllRuntimesAndApiLevels().build();
}
+ @Before
+ public void setup() throws Exception {
+ // Get the expected stack trace by running on the JVM.
+ TestBuilder<? extends SingleTestRunResult<?>, ?> testBuilder =
+ testForRuntime(parameters).addInnerClasses(getClass());
+ SingleTestRunResult<?> runResult = testBuilder.run(parameters.getRuntime(), Main.class);
+ if (parameters.isCfRuntime()) {
+ expectedStackTrace = runResult.map(StackTrace::extractFromJvm);
+ } else {
+ expectedStackTrace =
+ StackTrace.extractFromArt(runResult.getStdErr(), parameters.asDexRuntime().getVm());
+ }
+ }
+
@Test
public void testR8() throws Throwable {
- R8TestCompileResult compileResult =
- testForR8(parameters.getBackend())
- .addInnerClasses(getClass())
- .setMinApi(parameters.getApiLevel())
- .addKeepMainRule(Main.class)
- .compile()
- .inspectProguardMap(
- map -> {
- // TODO(b/215339687): We should not have a rewriteFrame in the mapping file since
- // an explicit null check should be inserted.
- assertThat(map, CoreMatchers.containsString("com.android.tools.r8.rewriteFrame"));
- });
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .setMinApi(parameters.getApiLevel())
+ .addKeepMainRule(Main.class)
+ .compile()
+ .inspectProguardMap(
+ map -> {
+ assertThat(
+ map, not(CoreMatchers.containsString("com.android.tools.r8.rewriteFrame")));
+ });
}
static class Foo {
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckFollowingImplicitReceiverNullCheckTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckFollowingImplicitReceiverNullCheckTest.java
index b5cd101..15f867f 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckFollowingImplicitReceiverNullCheckTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckFollowingImplicitReceiverNullCheckTest.java
@@ -31,12 +31,10 @@
}
public StackTrace expectedStackTrace;
- public StackTrace unexpectedStackTrace;
@Before
public void setup() throws Exception {
expectedStackTrace = getStackTrace();
- unexpectedStackTrace = getStackTrace("foo");
}
private StackTrace getStackTrace(String... args) throws Exception {
@@ -62,9 +60,8 @@
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Caller.class)
.assertFailureWithErrorThatThrows(NullPointerException.class)
- // TODO(b/214377135): Should retrace to expectedStackTrace.
.inspectStackTrace(
- (stackTrace, codeInspector) -> assertThat(stackTrace, isSame(unexpectedStackTrace)));
+ (stackTrace, codeInspector) -> assertThat(stackTrace, isSame(expectedStackTrace)));
}
static class Foo {
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckInlinedTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckInlinedTest.java
index 6143522..6b7b2d0 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckInlinedTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckInlinedTest.java
@@ -4,19 +4,21 @@
package com.android.tools.r8.naming.retrace;
import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
+import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForLineNumbers;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
-import static com.android.tools.r8.utils.codeinspector.Matchers.notIf;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoMethodStaticizing;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.naming.retrace.StackTrace.StackTraceLine;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import java.util.List;
+import java.util.Objects;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -71,8 +73,28 @@
MethodSubject staticized = callerClass.uniqueMethodWithName("outerCaller");
assertThat(staticized, isPresentAndRenamed());
// TODO(b/214377135): The stack traces should be the same (when 206427323) is
- // resolved.
- assertThat(stackTrace, notIf(isSame(expectedStackTrace), throwReceiverNpe));
+ // resolved.
+ if (throwReceiverNpe && canUseJavaUtilObjectsRequireNonNull(parameters)) {
+ StackTrace requireNonNullFrame =
+ StackTrace.builder().add(stackTrace.get(0)).build();
+ assertThat(
+ requireNonNullFrame,
+ isSameExceptForLineNumbers(
+ StackTrace.builder()
+ .add(
+ StackTraceLine.builder()
+ .setClassName(Objects.class.getTypeName())
+ .setMethodName("requireNonNull")
+ .setFileName("Objects.java")
+ .build())
+ .build()));
+
+ StackTrace stackTraceWithoutRequireNonNull =
+ StackTrace.builder().add(stackTrace).remove(0).build();
+ assertThat(stackTraceWithoutRequireNonNull, isSame(expectedStackTrace));
+ } else {
+ assertThat(stackTrace, isSame(expectedStackTrace));
+ }
});
}
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckTest.java
index 13e3e7f..70c2be4 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckTest.java
@@ -8,11 +8,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
-import com.android.tools.r8.R8FullTestBuilder;
-import com.android.tools.r8.R8TestRunResult;
-import com.android.tools.r8.SingleTestRunResult;
import com.android.tools.r8.TestBase;
-import com.android.tools.r8.TestBuilder;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.naming.retrace.StackTrace.StackTraceLine;
import com.android.tools.r8.utils.BooleanUtils;
@@ -43,46 +39,36 @@
public StackTrace expectedStackTrace;
+ private String[] getArgs() {
+ return throwReceiverNpe ? new String[] {"foo"} : new String[0];
+ }
+
@Before
public void setup() throws Exception {
- // Get the expected stack trace by running on the JVM.
- TestBuilder<? extends SingleTestRunResult<?>, ?> testBuilder =
- testForRuntime(parameters).addProgramClasses(Caller.class, Foo.class);
- SingleTestRunResult<?> runResult;
- if (throwReceiverNpe) {
- runResult = testBuilder.run(parameters.getRuntime(), Caller.class, "foo");
- } else {
- runResult = testBuilder.run(parameters.getRuntime(), Caller.class);
- }
- if (parameters.isCfRuntime()) {
- expectedStackTrace = runResult.map(StackTrace::extractFromJvm);
- } else {
- expectedStackTrace =
- StackTrace.extractFromArt(runResult.getStdErr(), parameters.asDexRuntime().getVm());
- }
+ // Get the expected stack trace by running on the JVM/ART.
+ expectedStackTrace =
+ testForRuntime(parameters)
+ .addProgramClasses(Caller.class, Foo.class)
+ .run(parameters.getRuntime(), Caller.class, getArgs())
+ .getStackTrace();
}
@Test
public void testR8() throws Exception {
- R8FullTestBuilder r8FullTestBuilder =
- testForR8(parameters.getBackend())
- .addInnerClasses(getClass())
- .addKeepMainRule(Caller.class)
- .addKeepAttributeLineNumberTable()
- .addKeepAttributeSourceFile()
- .setMinApi(parameters.getApiLevel())
- .enableInliningAnnotations()
- .enableExperimentalMapFileVersion();
- R8TestRunResult runResult;
- if (throwReceiverNpe) {
- runResult = r8FullTestBuilder.run(parameters.getRuntime(), Caller.class, "foo");
- } else {
- runResult = r8FullTestBuilder.run(parameters.getRuntime(), Caller.class);
- }
- runResult
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Caller.class)
+ .addKeepAttributeLineNumberTable()
+ .addKeepAttributeSourceFile()
+ .setMinApi(parameters.getApiLevel())
+ .enableInliningAnnotations()
+ .enableExperimentalMapFileVersion()
+ .run(parameters.getRuntime(), Caller.class, getArgs())
.assertFailureWithErrorThatThrows(NullPointerException.class)
.inspectStackTrace(
(stackTrace, codeInspector) -> {
+ // TODO(b/214377135): The stack traces should be the same (when 206427323) is
+ // resolved.
if (throwReceiverNpe && canUseJavaUtilObjectsRequireNonNull(parameters)) {
StackTrace requireNonNullFrame =
StackTrace.builder().add(stackTrace.get(0)).build();
diff --git a/src/test/java/com/android/tools/r8/shaking/proxy/ProxiesTest.java b/src/test/java/com/android/tools/r8/shaking/proxy/ProxiesTest.java
index e7bb456..047a109 100644
--- a/src/test/java/com/android/tools/r8/shaking/proxy/ProxiesTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/proxy/ProxiesTest.java
@@ -65,7 +65,6 @@
.addOptionsModification(
o -> {
o.enableDevirtualization = false;
- o.inlinerOptions().enableInliningOfInvokesWithNullableReceivers = false;
})
.enableAlwaysInliningAnnotations()
.noMinification()
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
index 6803e16..594b17a 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
@@ -89,9 +89,9 @@
return Streams.stream(iterateTryCatches());
}
- public void getLineNumberForInstruction(InstructionSubject subject) {
+ public int getLineNumberForInstruction(InstructionSubject subject) {
assert hasLineNumberTable();
- getLineNumberTable().getLineForInstruction(subject);
+ return getLineNumberTable().getLineForInstruction(subject);
}
@Override
