This also changes the transformation to inline the body of a directly
invoked lambda rather than producing a call to an anonymous local
function. The latter is unsupported in inline functions and problematic
from an ABI perspective, since it results in functions whose name
depends on the entire source code up to this point.
All tests in this commit _pass_ to document existing behavior, but
this will change in a separate commit as we reflect desired behavior
in the test expectations.
This commit introduces support for calling and referencing local functions and
objects in evaluate expression on the IR backend.
The primary incision is a lowering inserted after Local Declaration Lowering,
that uses the intermediate data structures recorded by LDL to rewrite calls to
local functions to the appropriate function in the binary, instead of predicting
the compilation strategy. The required changes to the rest of the pipeline
facilitate piping the required data around.
The key to this transformation is that _captures by the local function_ must be
introduced as _captures by the fragment function_, such that the evaluator
infrastructure can find the appropriate values at run-time. This is necessary
due to the strategy of compiling local functions to static functions instead of
closures.
Additional test coverage of stepping behavior support the corresponding changes
in the Evaluator, part of the Kotlin Debugger plug-in.
Locals introduced in the body of a do-while loop are not
necessarily live at the do-while condition. For example,
if there is a continue in the body before the declaration:
do {
if (shouldContinue(x))
continue
val y = 32 // not always defined in the condition
doSomething(y)
} while (x < 2)
For locals referenced in the condition such code is rejected
by the frontend because a local referenced in the condition
must be always defined when you get there.
However, locals that are not used in the condition were always
put in the local variable table. This leads to invalid locals
information which can trip of debuggers and other build tools
such as the D8 dexer.
This patch only puts in locals information for locals actually
referenced in the local variable table for the condition.
^Fixes KT-51754
Do not generate linenumber for the start of the finally block, because
that is usually where the only word 'finally' is located. Instead,
generate linenumber for the first expression inside the finally block.
Not generating this linenumber fixes an issue in code coverage tools
which would consider such finally uncovered. Although this might be
technically considered as designed, it makes more sense to NOT detect it
as uncovered because semantics of the finally block shouldn't really
differ whether it's executed normally or because an exception happened.
It's also beneficial for the tool support to behave like javac, which
doesn't generate the linenumber either.
#KT-50973 Fixed
Previously, it was obtained from expected type of a variable being assigned,
but it's better to use the type of resulting expression
Initially this part was brought in 4ab0897d7d,
but as we see in commit message and tests it was all about unit-coercion
We are going to deprecate `WITH_RUNTIME` directive. The main reason
behind this change is that `WITH_STDLIB` directive better describes
its meaning, specifically it will add kotlin stdlib to test's classpath.
- Mangle names for extension receivers in lambdas
- Correctly mark anonymous variables and variables for arguments
for destructuring declaration.
There is one failure remaining which is cause by lambda
type inference differences that leads to FIR having an explicit
return from the lambda whereas old frontend leads to an implicit
return. This difference is visible in debug stepping that the
local variables tests do because the implicit return has the line
number of the closing brace of the lambda. This change adds an
IrText test to make the difference clear.
Putting them in the local variable table means that the debugger
needs to have special handling for parameters with specific names.
That forces us to generate mangled names for these.
Instead of also implementing the name mangling for FIR, this
change gets rid of the parameters from the LVT instead.
Their call sites are all in the same file, so we can check whether the
declarations used in the inline function are accessible from all the
places where it will be inlined.
#KT-48736 Fixed
This ensures that the debugger always has a bytecode offset for
the line number of a break/continue so that you step there and
so that you can set breakpoints there.
The `nop` instruction is optimized out if it has no line number
information.
^KT-46450 Fixed
This avoids an extra call to 'analyze', which is rather costly.
Update debugger testData: Constant condition
elimination now performs DCE more consistently.
CoroutineTransformermethodVisitor attempts to extend the ranges
of local variables in various situations. Probably in an attempt
to give a better debugging experience. However, all of these
range extensions lead to invalid local variable tables where
something is in the local variable table where nothing is in the
corresponding slot.
The code that extends variables to the next suspension point
instead of ending them when they are no longer live has issues
with loops. When resuming and reentering the loop, the locals
table will mention a local that we did not spill and which
is therefore not restored when resuming.
The code that extends local variable table entries if there
are no suspension points between two entries doesn't work
for code such as:
```
var s: String
if (suspendHere() == "OK") {
s = "OK"
} else {
s = "FAIL"
}
```
If the local variable ranges are collapsed into one, one of
the branches will have the local defined in the local variable
table before the slot is initialized.
For code such as:
```
try {
var y = "y"
for (i in 0 until 1) {
return y
}
} finally {
println("finally")
}
```
The local variables `y` and `i` ended up covering the finally block as
well in the debugger.
This change splits the range of the locals so that they do
not cover the finally block.
This change does not change the inliner to do similar transformations,
so the range of locals is still wrong win finally blocks when inlined
into an inline function. Added a failing test to that effect.
The coroutine transformation would leave locals in the local
variable table across the code that reloads local variables from
the continuation on reentry. However, when reentering the function
the local has no value until after the reloads from the
continuation.
This change splits the locals in the local variable table to
avoid such uninitialized locals. A local alive across a
suspension point has its range split in two. One that goes
from the original start to the state label for the restart
after the suspension. The other goes from after the local
has been reloaded from the continuation until the previous
end of the local.
This solves no immediate deficiency, but is a to-do that arose as part
of an ongoing effort to port the old, slightly too restrictive
checkLocalVariableTable tests to the more "functional" debugger
infrastructure.
This just shaves a little bit of overhead off the test expectations
and couples them less tightly to the specifics of the test
infrastructure.
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
Dmitriy Novozhilov