This change improves the debugging experience around local functions
on the IR backend. The changes include moving old
checkLocalVariablesTable (cLVT) tests to the new stepping/local variable
infrastructure in order to refine the tests and further define the
behavior of the two JVM backends, and their differences.
The primary ported test case is cLVT/localFun.kt that documents the
discrepancy in implementation strategy for local functions on the two
backends. The old backend implements local functions as lambdas
assigned to a local variable while the IR backend lifts them out as
static funtions on the surrounding class. The discrepancies and their
consequences are documented in bytecodeListing, idea-stepping,
localVariableTable and debugStepping tests.
The only _code change_ is disabling the captured variable name
mangling for captured variables on the IR backend. Captured variables
are passed as arguments to the static function, so in the debugger,
they really just are local variables. For them to show properly in the
debugger and be detectable by evaluate expression, they simply need no
mangling.
Finally, this change cleans 3 redundant cLVT tests, copyFunction.kt
and destructuringInlineLambda.kt and destructuringInFor.kt, that are
all covered in the new suite. The stepping behavior needs to be made
precise around for loops, but that is an entirely seperate issue.
Reveals discrepancy in LVT presence on lambda implementations on the
old and new backend.
The generated code in the constructors of Suspend Lambda objects is
identical, but the IR backend generates an LVT with the constructor
parameters.
The user has to be very insistent to see this ("for step into" +
disabling "Show only kotlin variables"), but it is an observable
difference.
The existing backend restores LVs and parameters from the suspend lambda
fields used for spilling between suspension points, hence they are
visible in the debugger as local variables, plain and simple.
This PR introduces the same pattern to the IR backend, to bring the
debugging experience in line with the existing backend.
Both backends are still at the mercy of the liveness analysis
performed in the coroutine transformer where a liveness analysis
minimizes live ranges of entries in the LVT. E.g. an unused parameter
will be dropped entirely.
Adjusted existing test expectations accounting for the differences in
LV behavior.
The debug experiece of destructuring patterns in lambdas is different
across the two backends due to the IR backend moving local variables
to fields.
However, since the destructuring variable is never actually visible in
the debugger (no linenumbers in the live range of the variable), and
the variable is never used for anything other than hiding it from the
debugger, we propose that it is not actually necessary to include it
in the LVT (and in fact, could be left out of the LVT on the old
backend).
This commit unifies the expectation format between the stepping and
LVT tests in the new debugging test harness.
Furthermore, it introduces a compression of runs of locations without
linenumbers. These default to showing as bytecode offsets from
previous line number, which overspecifies the tests: the bytecodes
chosen should not be constrained by a debugging step test.
This commit enables the execution of suspend box tests in a separate
test. It's a QoL improvement in the existing bb tests but, motivating
these changes, enables the new debugger stepping tests to step
coroutine code.
This is the first step in a push to improve the test coverage of the
IR debugging experience. This commit improves on the sketch of local
variable table tests built on the new debugger stepping
infrastructure. As improvements on the existing
checkLocalVariableTableTest they:
- don't overspecify codegen strategy: no hard requirements on slots.
- test the observed lifespans of locals by stepping through code.
Ultimately this should enable us to bring over all existing tests for
improved coverage of both old and new JVM backends.
WIP list:
- Gracefully handle absent LVT: treat as empty Add type of local and
- type of value stored there to expectations Print values in local
- slots if primitive or java.lang.String Enable specifying
- expectations per backend Gracefully handle absent box methods
- Gracefully handle null values in slots of reference type
- Port a first LVT test
The current backend does that for some bridges. We do it consistently
for all bridges.
The line number used differs. For the JVM_IR backend, we use the
line number of the class to which the bridge is added. For the
JVM backend, that does not appear to be the case for bridges
in lambdas. I prefer the line number for a lambda invoke bridge
to be the line for the lambda instead os some surrounding class.
The JVM BE inlines calls to the underlying function in a $default stub
verbatim, e.g., without renaming LVT entries or regenerating anonymous
objects. This commit introduces the same behavior in the JVM IR BE.
Fixes KT-36769.
Reimplement the same hacky approach used in the old backend (see
cc2fe6b0c6).
Previously, the debugger incorrectly stepped into Collections.kt on
"step over" inline function calls from stdlib like 'any'.
Since `if` and `when` expressions are represented the same way in IR,
the behavior is fixed for both of them. It's not the case in the old JVM
backend, where stepping over `when` conditions still suffers from the
same problem, which the newly added test checks.
Loads of temporary variables that contain constants are replaced
with a copy of the constant. This avoids locals loads and stores.
However, the copy of the constant needs to have the offset of
the load and not of the original constant.
Fixes KT-41963.
JVM_IR has consistent stepping behavior for secondary constructors
and init blocks. This change fixes the expectations for tests that
expect the less consistent JVM backend behavior.
Temporary variable loads in when expressions had the offsets
of the variable declaration. That leads to hacks during codegen
for line number generation.
Instead of those hacks, give the variable loads the offsets of
the context in which they occur. That avoids the codegen hacks
and fixes stepping behavior for more when expressions.
In addition, made the stepping information for constructor calls
consistent across JVM_IR and JVM. For JVM_IR that stepping behavior
is consistent for enum constructor calls in <clinit> for JVM it
is not.
In particular, the current line numbers could lead to stepping
into the catch handler even when the code in the try did not
throw an exception.
This was caused by the code materializing the final value having
the catch line number. This patch delays the materialization
until the line number of the usage has been emitted.
These line number tests only tested that a set of line numbers where
present in the java bytecode. Not that they would be hit in the
right order by the debugger. Moving them to stepping tests fixes that.
This exposes a couple of issues (in particular around try-catch-finally)
that should be fixed.
A number of tests are marked as failing now. Will investigate and
work on fixes next.
Specifically, this commit improves the stepping behavior of the IR
backend around functions with defaults.
- Improved line numbers in the default handler itself for better
stepping when inlined.
- Improved source information on default arguments
- Improved test coverage of stepping behavior in old and IR backends.
Improves the stepping behaviour around inline methods with default
arguments. In particular, we now accurately step through the
evaluation of default arguments, but do _not_ spuriously show the exit
from the $default handler.
Fix line number generation for assignments where the right-hand
side of the assignment is not on the same line.
Fix line number generation for intrinsics functions where the
function is not on the same line as the last argument.
Be careful to not break stepping behavior with the iinc
optimizations.
Kristoffer Andersen