Status Update
Comments
tn...@google.com <tn...@google.com>
tn...@google.com <tn...@google.com> #2
Thank you for the report and reproduction. I can reproduce on a Android TV emulator.
an...@google.com <an...@google.com> #3
Status of current findings.
This is related to the SplitReturnRewriter
added in
Not running this code pass on java.lang.Object androidx.collection.LruCache.remove(java.lang.Object)
works around the issue. I used the following patch:
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
index 157d26c66a..63e0f76064 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
@@ -43,6 +43,10 @@ public class SplitReturnRewriter extends CodeRewriterPass<AppInfo> {
@Override
protected CodeRewriterResult rewriteCode(IRCode code) {
+ if (code.context().toSourceString().equals(
+ "java.lang.Object androidx.collection.LruCache.remove(java.lang.Object)")) {
+ return CodeRewriterResult.NO_CHANGE;
+ }
int color = colorExceptionHandlers(code);
AffectedValues affectedValues = new AffectedValues();
boolean changed = false;
Comparing the DEX with and without this patch shows one instruction diff in void androidx.leanback.widget.GridLayoutManager$2.addItem(java.lang.Object, int, int, int, int)
where java.lang.Object androidx.collection.LruCache.remove(java.lang.Object)
is inlined into.
Tracing the IR for void androidx.leanback.widget.GridLayoutManager$2.addItem(java.lang.Object, int, int, int, int)
for the two versions shows that they start to deviate in second pass of IR after inserting assume instructions (SSA)
with
block 29, pred-counts: 1, succ-count: 2, filled: true, sealed: false
predecessors: 28
successors: 31 30 (no try/catch successors)
no phis
#143:loadView;1762:addItem: -1: InstanceGet v67 <- v136; field: androidx.collection.LruCache androidx.leanback.widget.ViewsStateBundle.mChildStates
: -1: Assume v140 <- v67; lower bound: @Nullable androidx.collection.LruCache {}
: -1: SafeCheckCast v131 <- v65; java.lang.String
: -1: Invoke-Virtual v132 <- v140, v131; method: java.lang.Object androidx.collection.LruCache.remove(java.lang.String)
: -1: CheckCast v69 <- v132; android.util.SparseArray
#144:loadView;1762:addItem: -1: If v69 EQZ block 31 (fallthrough 30)
vs.
block 29, pred-counts: 1, succ-count: 2, filled: true, sealed: false
predecessors: 28
successors: 31 30 (no try/catch successors)
no phis
#143:loadView;1762:addItem: -1: InstanceGet v67 <- v136; field: androidx.collection.LruCache androidx.leanback.widget.ViewsStateBundle.mChildStates
: -1: Assume v140 <- v67; lower bound: @Nullable androidx.collection.LruCache {}
: -1: SafeCheckCast v131 <- v65; java.lang.String
: -1: Invoke-Virtual v132 <- v140, v131; method: java.lang.Object androidx.collection.LruCache.remove(java.lang.String)
: -1: Assume v141 <- v132; not null
: -1: CheckCast v69 <- v141; android.util.SparseArray
#144:loadView;1762:addItem: -1: If v69 EQZ block 31 (fallthrough 30)
This is from computeDynamicReturnType
concluding a dynamic return type of DynamicTypeWithUpperBound(@NotNull java.lang.Object {})
.
Description
To repro:
Build.VERSION.SDK_INT
is above a particular value.@RequiresApi
to capture this requirement.Expected:
Lint warns me if I call this method without checking
Build.VERSION.SDK_INT
(like how the@RequiresApi
annotation works in non-test code).Actual:
Lint tells me to use
@SdkSuppress
instead (see screenshot). This doesn't seem equivalent to me, since@SdkSuppress
should only be used on public@Test
methods to allow the test runner to decide whether to run them or not. This is a private method, which won't be directly invoked by the test runner.