If an inline parameter has a default value, its type is nullable.
There's already code to handle this in `IrInlineCodegen`, but it
really should be in `isInlineParameter` instead, otherwise e.g.
SyntheticAccessorLowering fails.
When we inline an anonymous object which captures something such as
crossinline values or reified parameters, we copy and transform its
metadata in `AnonymousObjectTransformer.transformMetadata`. Basically we
read the metadata of the original class, add a minor protobuf extension
and write it to the new class.
This also includes copying the string table. We read the string table
into `JvmNameResolver` (a representation of string table used in
deserialization), then construct a `JvmStringTable` (a representation
used in _serialization_) and then write it back.
There's a few optimizations in the string table representation in JVM
metadata which allow to store less strings and thus take less space. See
`StringTableTypes.Record` in `jvm_metadata.proto` for more information.
One of the optimizations `Record.range` allows to avoid storing the same
record many times in a sequence. For example, if we have N different
strings in the string table but none of them require any operation (such
as substring, char replacement, etc.), then we only store the record
with all default values (no operation, no predefined string, etc.) and
set its `range` to N. Upon reading such optimized record list in
`JvmNameResolver`, we "expand" it back to normal, so that we could index
it quickly and figure out what operation needs to be performed on each
string from the string table.
The problem was that when we expanded this list, we didn't set the range
of the expanded record entry to 1. So each record in
`JvmNameResolver.records` still has its original range. It doesn't cause
any problems most of the time because the range in this expanded list is
almost unused. However, when copying/transforming metadata for anonymous
objects, we mistakenly passed this expanded list with incorrect ranges
to `JvmStringTable`. So the metadata in the copied anonymous object
ended up being incorrect: each record now was present the number of
times equal to its range. Copying such metadata once again led to
another multiplication of the record list size. Multiple copies resulted
in exponential increase in memory consumption and quickly led to OOM.
For the fix, we now take the original, unexpanded list of records when
creating `JvmStringTable` out of `JvmNameResolver` for transformation of
anonymous object metadata.
Note that another possible fix would be to make range for each record in
`JvmNameResolver.records` equal to 1. This is undesirable though, since
then we'd need to copy each `JvmProtoBuf.StringTableTypes.Record`
instance, of which there could be many, and use some memory for no
apparent gain (since ranges in that expanded list are now not used at
all).
#KT-38197 Fixed
In general, `InliningContext.findAnonymousTransformationInfo` was not
reliable because it mapped each type to *some* info for that type,
preferring ones with `shouldRegenerate == true` if those exist. Thus, it
returned incorrect results if one type was regenerated multiple times,
e.g. in a nested inlining context or because of a `finally` (which
duplicates anonymous objects). The solution is to avoid a global map and
attach the current transformation info directly to the current inlining
context.
If `mapLineNumber` was called in non-monotonic order, e.g. N then N+2
then N+1, the first two calls created a range that spans [N; N+2] but
the third call did not reuse it.
The fields containing crossinline lambdas should be package-private to
avoid generating synthetic accessors, which break object regeneration.
Note that the inline methods cannot actually be called, as call sites
will attempt to read the captured lambda from a field through a *copy*
of the local containing the object, so these reads will not be inlined,
causing an exception at runtime:
inline fun f(crossinline g: () -> Unit) = object : I {
inline fun h() = g()
// effectively `val tmp = this; return tmp.$g()`:
override fun run() = h()
}
f {}.run() // NoSuchFieldError: $g
This particular example can be fixed by reusing locals for receiver
parameters in IrInlineCodegen, but explicitly assigning `this` to
another variable and calling an inline method on it will break it again.
(This is only applicable to the JVM_IR backend, as the non-IR one fails
to generate `f` at all for some other reason.)
Otherwise, the assumption that coroutine codegen makes about every
inlined function already having the markers breaks and it is no longer
true that calls to inline lambdas do not require them.
This is one example of a function replaced by a lowering after
AddContinuationLowering mentioned in the last commit: the lowering that
makes default stubs static is further down. Although this is not a
lambda, calling getOrCreateSuspendFunctionViewIfNeeded on it in the
inliner is fatal all the same.
Private $$forInline versions of public suspend functions should not
be mangled. (Note that there are no $$forInline versions of private
suspend functions, as those are effectively inline-only.)
* unify various checks for whether a suspend function needs a
continuation;
* mark the continuation parameter with an origin (this also allows
correctly computing the offset of the local in codegen -- see the
new test);
* when wrapping `suspend fun main`, use a suspend reference instead of
a synthetic non-suspend lambda (required to correctly implement the
previous point, as previously the continuation parameter was passed
to main() itself correctly only because of very lenient checks in
AddContinuationLowering).
parameters of inline function. Otherwise, it leads to AnalyzerException,
when inlined lambda contains try-catch block. The reason is simple:
in try block, we leave some variables on stack, while on catch block the
stack is empty. Spilling the variables before try block does the trick.
#KT-34708 Fixed
The only case where this code is reachable at all is objects in lambdas
that use reified type parameters of the outer inline function. Since the
type parameters are declared outside the inlining root, regenerating the
object is actually pointless, and remapping its captures even more so
(not to mention that the code under the condition uses the captures of
the lambda, not of the method it's currently transforming).
Also,
1. remove some redundant copies;
2. fix remapping of non-local returns in lambdas if the body is moved
after LocalDeclarationsLowering (the lambda is no longer inside the
body, but must still be visited)
In case of inline it should be same descriptor (except of fake override), In general case getter could be synthetic accessor and in such case it's not inline
For example, a lambda `{ param -> captured }` of type `E.(T) -> U` will
be transformed by LocalDeclarationsLowering into a private static method
fun f$lambda-0($this: E, $captured: U, param: T) = $captured
The reason for such an ordering is that a lambda looks the same as a
local function, and local function can have default arguments, and those
arguments can reference captured variables; thus, captured variables
must come before actual declared arguments.
However, this is not the order that the inliner wants. Moreover, since
it was written to handle lambdas represented as `invoke` methods of
anonymous objects, it does not expect the actual callable method to have
any parameters corresponding to captured variables at all. This results
in it attempting to generate a temporary node with descriptor
(LE;LU;LT;LU;)LU;
while still using locals 1 and 2 as `param` and `$captured` respectively.
In the example above, this is not critical, as they both have reference
type and the lambda will eventually be pasted into a different node
anyway; however, if it happens that one of them is a primitive, or both
are primitives of different types, the bytecode will use incorrect
instructions, causing verification errors. The correct descriptor is
(LE;LT;LU;)LU;
As for SAM wrappers, the bytecode sequence
new A
dup
new B
dup
invokespecial B.<init>
invokespecial A.<init>
breaks the inliner, so instead we do
new B
dup
invokespecial B.<init>
store x
new A
dup
load x
invokespecial A.<init>
To avoid bytecode sequences like
new _1Kt$sam$i$java_lang_Runnable$0
dup
new _1Kt$f$1
dup
invokespecial _1Kt$f$1.<init>()V
invokespecial _1Kt$sam$i$java_lang_Runnable$0.<init>(...)V
as the different order of `new` and `<init>` confuses the inliner.
Ilmir Usmanov