Re-enable LT2Fir function/method body elidation #5540
Conversation
aviroop-123
commented
Oct 30, 2025
aviroop-123
commented
- Adds more tests for header compilation, specifically nested classes/functions.
- Fixes for nested function / method declaration.
- Re-enable LT2Fir function/method body removal & fix for test stub.
…e inline functions. ^KT-78422
demiurg906
requested review from
demiurg906
and removed request for
FenstonSingel and
mMaxy
| result.transformReturnTypeRef(transformer, ResolutionMode.UpdateImplicitTypeRef(returnTypeRef)) | ||
| } | ||
| if (session.languageVersionSettings.getFlag(AnalysisFlags.headerMode) && !function.isInline) { | ||
| if (session.languageVersionSettings.getFlag(AnalysisFlags.headerMode) && !function.isInline && function.isFromInlineFunction != true) { |
There was a problem hiding this comment.
You don't need this attribute, actually.
It's enough to check that:
- header mode is enabled
- and the function is non local
- and the function is inline
If the function is local (either real local function or member of a local class), and its contained in the non inline function, it would be removed together with the body of the outer function.
There was a problem hiding this comment.
I think I understand what you mean. But would this mean we're processing the bodies of the local functions / classes even though it'll get removed at a later stage?
Also, does that mean that we don't need the forceKeepingTheBodyInHeaderMode?
| overriddenFunctions.map { it.status as FirResolvedDeclarationStatus }, | ||
| ) | ||
| // Once the modality is determined, we can remove the body. | ||
| if (session.languageVersionSettings.getFlag(AnalysisFlags.headerMode) |
There was a problem hiding this comment.
The argument from the other comment also applies here.
Also please use the following formatting in case of long if conditions:
if(
cond1 &&
cond2 &&
cond3
) {
...
}| var containerIsExpect: Boolean = false | ||
|
|
||
| var containedWithinInlineFunction: Boolean = false | ||
|
|
There was a problem hiding this comment.
I would rename it to forceKeepingTheBodyInHeaderMode.
| isExpect: Boolean, | ||
| forceLocalContext: Boolean = false, | ||
| containedWithinInlineFunction: Boolean? = null, | ||
| l: () -> T, |
There was a problem hiding this comment.
Never used at call sites
| name: Name, | ||
| isExpect: Boolean, | ||
| containedWithinInlineFunction: Boolean? = null, | ||
| l: () -> T, |
There was a problem hiding this comment.
This function is called only from withChildClassName, so the parameter could be removed from there too.
|
|
||
| inline fun <R> withForcedLocalContext(block: () -> R): R { | ||
| inline fun <R> withForcedLocalContext(containedWithinInlineFunction: Boolean? = null, block: () -> R): R { | ||
| val oldContainedWithinInlineFunction = context.containedWithinInlineFunction |
There was a problem hiding this comment.
According to my previous comment should be also renamed. Also the default could be changed forceKeepingTheBodyInHeaderMode: Boolean = false.
So the assignment would look like the following:
val oldContainedWithinInlineFunction = context.containedWithinInlineFunction
context.containedWithinInlineFunction = oldContainedWithinInlineFunction || containedWithinInlineFunction| } | ||
|
|
||
| fun convertBlockExpressionWithoutBuilding(block: LighterASTNode, kind: KtFakeSourceElementKind? = null): FirBlockBuilder { | ||
| fun convertBlockExpressionWithoutBuilding(block: LighterASTNode, kind: KtFakeSourceElementKind? = null, generateHeader: Boolean = false): FirBlockBuilder { |
There was a problem hiding this comment.
- Put parameters on separarate lines for readability
- Rename
generateHeadertoconvertOnlyFirstStatement - Add a KDoc explaining why it's required
2 participants
