Skip to content

Commit a7c9e14

Browse files
author
Mikhael Bogdanov
committed
Don't generate default arguments for inline call
1 parent 7690a8b commit a7c9e14

File tree

13 files changed

+81
-62
lines changed

13 files changed

+81
-62
lines changed

‎compiler/backend/src/org/jetbrains/kotlin/codegen/CallBasedArgumentGenerator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ protected void generateExpression(int i, @NotNull ExpressionValueArgument argume
6464

6565
@Override
6666
protected void generateDefault(int i, @NotNull DefaultValueArgument argument) {
67-
callGenerator.putValueIfNeeded(valueParameterTypes.get(i), createDefaulValue(valueParameterTypes.get(i)));
67+
callGenerator.putValueIfNeeded(valueParameterTypes.get(i), createDefaulValue(valueParameterTypes.get(i)), ValueKind.DEFAULT_PARAMETER);
6868
}
6969

7070
@Override

‎compiler/backend/src/org/jetbrains/kotlin/codegen/CallGenerator.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import org.jetbrains.org.objectweb.asm.Type
2323

2424
enum class ValueKind {
2525
GENERAL,
26+
DEFAULT_PARAMETER,
2627
DEFAULT_MASK,
27-
METHOD_HANDLE_IN_DEFAULT
28+
METHOD_HANDLE_IN_DEFAULT,
29+
CAPTURED
2830
}
2931

3032
abstract class CallGenerator {

‎compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.java

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ private InlineResult inlineCall(@NotNull SMAPAndMethodNode nodeAndSmap, boolean
414414
defaultSourceMapper.setCallSiteMarker(new CallSiteMarker(codegen.getLastLineNumber()));
415415
MethodNode node = nodeAndSmap.getNode();
416416
if (callDefault) {
417-
MethodInlinerUtilKt.expandMaskConditions(node, maskStartIndex, maskValues, methodHandleInDefaultMethodIndex);
417+
MethodInlinerUtilKt.expandMaskConditionsAndUpdateVariableNodes(node, maskStartIndex, maskValues, methodHandleInDefaultMethodIndex);
418418
}
419419
ReifiedTypeParametersUsages reificationResult = reifiedTypeInliner.reifyInstructions(node);
420420
generateClosuresBodies();
@@ -674,11 +674,17 @@ private void putArgumentOrCapturedToLocalVal(
674674
@NotNull Type type,
675675
@NotNull StackValue stackValue,
676676
int capturedParamIndex,
677-
int parameterIndex
677+
int parameterIndex,
678+
@NotNull ValueKind kind
678679
) {
680+
boolean isDefaultParameter = kind == ValueKind.DEFAULT_PARAMETER;
681+
if (!isDefaultParameter && shouldPutGeneralValue(type, stackValue)) {
682+
stackValue.put(type, codegen.v);
683+
}
684+
679685
if (!asFunctionInline && Type.VOID_TYPE != type) {
680686
//TODO remap only inlinable closure => otherwise we could get a lot of problem
681-
boolean couldBeRemapped = !shouldPutGeneralValue(type, stackValue);
687+
boolean couldBeRemapped = !shouldPutGeneralValue(type, stackValue) && kind != ValueKind.DEFAULT_PARAMETER;
682688
StackValue remappedValue = couldBeRemapped ? stackValue : null;
683689

684690
ParameterInfo info;
@@ -691,7 +697,7 @@ private void putArgumentOrCapturedToLocalVal(
691697
info = invocationParamBuilder.addNextValueParameter(type, false, remappedValue, parameterIndex);
692698
}
693699

694-
recordParameterValueInLocalVal(false, info);
700+
recordParameterValueInLocalVal(false, isDefaultParameter, info);
695701
}
696702
}
697703

@@ -725,7 +731,7 @@ private static boolean shouldPutGeneralValue(@NotNull Type type, @NotNull StackV
725731
return true;
726732
}
727733

728-
private Runnable recordParameterValueInLocalVal(boolean delayedWritingToLocals, @NotNull ParameterInfo... infos) {
734+
private Runnable recordParameterValueInLocalVal(boolean delayedWritingToLocals, boolean skipStore, @NotNull ParameterInfo... infos) {
729735
int[] index = new int[infos.length];
730736
for (int i = 0; i < infos.length; i++) {
731737
ParameterInfo info = infos[i];
@@ -743,7 +749,9 @@ private Runnable recordParameterValueInLocalVal(boolean delayedWritingToLocals,
743749
if (!info.isSkippedOrRemapped()) {
744750
Type type = info.type;
745751
StackValue.Local local = StackValue.local(index[i], type);
746-
local.store(StackValue.onStack(type), codegen.v);
752+
if (!skipStore) {
753+
local.store(StackValue.onStack(type), codegen.v);
754+
}
747755
if (info instanceof CapturedParamInfo) {
748756
info.setRemapValue(local);
749757
((CapturedParamInfo) info).setSynthetic(true);
@@ -773,7 +781,7 @@ public void processAndPutHiddenParameters(boolean justProcess) {
773781
invocationParamBuilder.markValueParametersStart();
774782
List<ParameterInfo> hiddenParameters = invocationParamBuilder.buildParameters().getParameters();
775783

776-
delayedHiddenWriting = recordParameterValueInLocalVal(justProcess, hiddenParameters.toArray(new ParameterInfo[hiddenParameters.size()]));
784+
delayedHiddenWriting = recordParameterValueInLocalVal(justProcess, false, hiddenParameters.toArray(new ParameterInfo[hiddenParameters.size()]));
777785
}
778786

779787
private void leaveTemps() {
@@ -924,11 +932,7 @@ private void putValueIfNeeded(@NotNull Type parameterType, @NotNull StackValue v
924932

925933
assert maskValues.isEmpty() : "Additional default call arguments should be last ones, but " + value;
926934

927-
if (shouldPutGeneralValue(parameterType, value)) {
928-
value.put(parameterType, codegen.v);
929-
}
930-
931-
putArgumentOrCapturedToLocalVal(parameterType, value, -1, index);
935+
putArgumentOrCapturedToLocalVal(parameterType, value, -1, index, kind);
932936
}
933937

934938
private boolean processDefaultMaskOrMethodHandler(@NotNull StackValue value, @NotNull ValueKind kind) {
@@ -953,10 +957,7 @@ private boolean processDefaultMaskOrMethodHandler(@NotNull StackValue value, @No
953957

954958
@Override
955959
public void putCapturedValueOnStack(@NotNull StackValue stackValue, @NotNull Type valueType, int paramIndex) {
956-
if (shouldPutGeneralValue(stackValue.type, stackValue)) {
957-
stackValue.put(stackValue.type, codegen.v);
958-
}
959-
putArgumentOrCapturedToLocalVal(stackValue.type, stackValue, paramIndex, paramIndex);
960+
putArgumentOrCapturedToLocalVal(stackValue.type, stackValue, paramIndex, paramIndex, ValueKind.CAPTURED);
960961
}
961962

962963
private void generateAndInsertFinallyBlocks(

‎compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInlinerUtil.kt

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,17 @@ private fun MethodInliner.getLambdaIfExistsAndMarkInstructions(
8989

9090
fun SourceValue.singleOrNullInsn() = insns.singleOrNull()
9191

92-
fun expandMaskConditions(node: MethodNode, maskStartIndex: Int, masks: List<Int>, methodHandlerIndex: Int) {
93-
class Condition(mask: Int, constant: Int, val maskInstruction: VarInsnNode, val jumpInstruction: JumpInsnNode) {
92+
fun expandMaskConditionsAndUpdateVariableNodes(node: MethodNode, maskStartIndex: Int, masks: List<Int>, methodHandlerIndex: Int) {
93+
class Condition(mask: Int, constant: Int, val maskInstruction: VarInsnNode, val jumpInstruction: JumpInsnNode, val varIndex: Int) {
9494
val expandNotDelete = mask and constant != 0
9595
}
96+
fun isMaskIndex(varIndex: Int): Boolean {
97+
return maskStartIndex <= varIndex && varIndex < maskStartIndex + masks.size
98+
}
9699

97100
val maskProcessingHeader = node.instructions.asSequence().takeWhile {
98101
if (it is VarInsnNode) {
99-
if (isMaskIndex(it, maskStartIndex, masks)) {
102+
if (isMaskIndex(it.`var`)) {
100103
/*if slot for default mask is updated than we occurred in actual function body*/
101104
return@takeWhile it.opcode == Opcodes.ILOAD
102105
}
@@ -108,43 +111,45 @@ fun expandMaskConditions(node: MethodNode, maskStartIndex: Int, masks: List<Int>
108111
}
109112

110113
val conditions = maskProcessingHeader.filterIsInstance<VarInsnNode>().mapNotNull {
111-
if (isMaskIndex(it, maskStartIndex, masks) &&
114+
if (isMaskIndex(it.`var`) &&
112115
it.next?.next?.opcode == Opcodes.IAND &&
113116
it.next.next.next?.opcode == Opcodes.IFEQ) {
117+
val jumpInstruction = it.next?.next?.next as JumpInsnNode
114118
Condition(
115119
masks[it.`var` - maskStartIndex],
116120
InlineCodegenUtil.getConstant(it.next),
117121
it,
118-
it.next?.next?.next as JumpInsnNode
122+
jumpInstruction,
123+
(jumpInstruction.label.previous as VarInsnNode).`var`
119124
)
120125
}
121126
else if (isMethodHandleIndex(methodHandlerIndex, it) &&
122127
it.next?.opcode == Opcodes.IFNULL &&
123128
it.next.next?.opcode == Opcodes.NEW) {
124129
//Always delete method handle for now
125130
//This logic should be updated when method handles would be supported
126-
Condition(0, 0, it,it.next as JumpInsnNode)
131+
Condition(0, 0, it,it.next as JumpInsnNode, -1)
127132
}
128133
else null
129134
}
130135

136+
val indexToVarNode = node.localVariables?.filter { it.index < maskStartIndex }?.associateBy { it.index } ?: emptyMap()
131137
val toDelete = arrayListOf<AbstractInsnNode>()
132138
conditions.forEach {
133139
val jumpInstruction = it.jumpInstruction
134140
InsnSequence(it.maskInstruction, (if (it.expandNotDelete) jumpInstruction.next else jumpInstruction.label)).forEach {
135141
toDelete.add(it)
136142
}
143+
if (it.expandNotDelete) {
144+
indexToVarNode[it.varIndex]?.let { varNode ->
145+
varNode.start = it.jumpInstruction.label
146+
}
147+
}
137148
}
138149

139150
toDelete.forEach {
140151
node.instructions.remove(it)
141152
}
142153
}
143154

144-
private fun isMethodHandleIndex(methodHandlerIndex: Int, it: VarInsnNode) = methodHandlerIndex == it.`var`
145-
146-
private fun isMaskIndex(variable: VarInsnNode, maskStartIndex: Int, masks: List<Int>): Boolean {
147-
val varIndex = variable.`var`
148-
return maskStartIndex <= varIndex && varIndex < maskStartIndex + masks.size
149-
}
150-
155+
private fun isMethodHandleIndex(methodHandlerIndex: Int, it: VarInsnNode) = methodHandlerIndex == it.`var`

‎compiler/testData/codegen/boxInline/defaultValues/maskElumination/32Parameters.kt renamed to ‎compiler/testData/codegen/boxInline/defaultValues/maskElimination/32Parameters.kt

File renamed without changes.

‎compiler/testData/codegen/boxInline/defaultValues/maskElumination/33Parameters.kt renamed to ‎compiler/testData/codegen/boxInline/defaultValues/maskElimination/33Parameters.kt

File renamed without changes.

‎compiler/testData/codegen/boxInline/defaultValues/maskElumination/simple.kt renamed to ‎compiler/testData/codegen/boxInline/defaultValues/maskElimination/simple.kt

File renamed without changes.

‎compiler/testData/codegen/bytecodeText/defaultArguments/maskAndArgumentElimination.kt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
inline fun test(p: String = "OK"): String {
2+
return p
3+
}
4+
5+
fun box() : String {
6+
return test()
7+
}
8+
9+
//mask check in test$default
10+
// 1 IFEQ
11+
12+
//total ifs
13+
// 1 IF
14+
15+
//no default argument on call site
16+
// 0 NULL
17+
18+
//proper variable start label: after assignment
19+
// 1 LOCALVARIABLE p\$iv Ljava/lang/String; L2 L3 0
20+
// 1 LDC "OK"\s*ASTORE 0\s*L2

‎compiler/testData/codegen/bytecodeText/defaultArguments/maskElumination.kt

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎compiler/testData/codegen/bytecodeText/defaultArguments/methodHandlerElumination.kt renamed to ‎compiler/testData/codegen/bytecodeText/defaultArguments/methodHandlerElimination.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//open modality to method handle check generation
12
open class A {
23
inline fun test(p: String = "OK"): String {
34
return p
@@ -12,4 +13,5 @@ fun box(): String {
1213
// 1 IFNULL
1314
//mask check in test$default
1415
// 1 IFEQ
16+
//total ifs
1517
// 2 IF

‎compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -929,29 +929,29 @@ public void testSimpleDefaultMethod() throws Exception {
929929
doTest(fileName);
930930
}
931931

932-
@TestMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElumination")
932+
@TestMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElimination")
933933
@TestDataPath("$PROJECT_ROOT")
934934
@RunWith(JUnit3RunnerWithInners.class)
935-
public static class MaskElumination extends AbstractBlackBoxInlineCodegenTest {
935+
public static class MaskElimination extends AbstractBlackBoxInlineCodegenTest {
936936
@TestMetadata("32Parameters.kt")
937937
public void test32Parameters() throws Exception {
938-
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElumination/32Parameters.kt");
938+
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElimination/32Parameters.kt");
939939
doTest(fileName);
940940
}
941941

942942
@TestMetadata("33Parameters.kt")
943943
public void test33Parameters() throws Exception {
944-
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElumination/33Parameters.kt");
944+
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElimination/33Parameters.kt");
945945
doTest(fileName);
946946
}
947947

948-
public void testAllFilesPresentInMaskElumination() throws Exception {
949-
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/defaultValues/maskElumination"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
948+
public void testAllFilesPresentInMaskElimination() throws Exception {
949+
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/defaultValues/maskElimination"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
950950
}
951951

952952
@TestMetadata("simple.kt")
953953
public void testSimple() throws Exception {
954-
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElumination/simple.kt");
954+
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElimination/simple.kt");
955955
doTest(fileName);
956956
}
957957
}

‎compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,15 +1091,15 @@ public void testKt11962() throws Exception {
10911091
doTest(fileName);
10921092
}
10931093

1094-
@TestMetadata("maskElumination.kt")
1095-
public void testMaskElumination() throws Exception {
1096-
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/defaultArguments/maskElumination.kt");
1094+
@TestMetadata("maskAndArgumentElimination.kt")
1095+
public void testMaskAndArgumentElimination() throws Exception {
1096+
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/defaultArguments/maskAndArgumentElimination.kt");
10971097
doTest(fileName);
10981098
}
10991099

1100-
@TestMetadata("methodHandlerElumination.kt")
1101-
public void testMethodHandlerElumination() throws Exception {
1102-
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/defaultArguments/methodHandlerElumination.kt");
1100+
@TestMetadata("methodHandlerElimination.kt")
1101+
public void testMethodHandlerElimination() throws Exception {
1102+
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/defaultArguments/methodHandlerElimination.kt");
11031103
doTest(fileName);
11041104
}
11051105
}

‎compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -929,29 +929,29 @@ public void testSimpleDefaultMethod() throws Exception {
929929
doTest(fileName);
930930
}
931931

932-
@TestMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElumination")
932+
@TestMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElimination")
933933
@TestDataPath("$PROJECT_ROOT")
934934
@RunWith(JUnit3RunnerWithInners.class)
935-
public static class MaskElumination extends AbstractCompileKotlinAgainstInlineKotlinTest {
935+
public static class MaskElimination extends AbstractCompileKotlinAgainstInlineKotlinTest {
936936
@TestMetadata("32Parameters.kt")
937937
public void test32Parameters() throws Exception {
938-
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElumination/32Parameters.kt");
938+
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElimination/32Parameters.kt");
939939
doTest(fileName);
940940
}
941941

942942
@TestMetadata("33Parameters.kt")
943943
public void test33Parameters() throws Exception {
944-
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElumination/33Parameters.kt");
944+
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElimination/33Parameters.kt");
945945
doTest(fileName);
946946
}
947947

948-
public void testAllFilesPresentInMaskElumination() throws Exception {
949-
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/defaultValues/maskElumination"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
948+
public void testAllFilesPresentInMaskElimination() throws Exception {
949+
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxInline/defaultValues/maskElimination"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
950950
}
951951

952952
@TestMetadata("simple.kt")
953953
public void testSimple() throws Exception {
954-
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElumination/simple.kt");
954+
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/maskElimination/simple.kt");
955955
doTest(fileName);
956956
}
957957
}

0 commit comments

Comments
 (0)