From 0fcaaa80df1466d92f2b114e3e683134c930d3e8 Mon Sep 17 00:00:00 2001 From: Michael Bogdanov Date: Tue, 3 Nov 2015 15:49:20 +0300 Subject: [PATCH] Fix for KT-9140: Default parameters are passed as nulls when using indexing [ ] invocation #KT-9140 Fixed --- .../kotlin/codegen/DefaultCallMask.kt | 9 ++++ .../kotlin/codegen/ExpressionCodegen.java | 15 +++---- .../jetbrains/kotlin/codegen/StackValue.java | 42 +++++++++++++++---- .../convention/incWithDefaultInGetter.kt | 26 ++++++++++++ .../box/defaultArguments/convention/kt9140.kt | 11 +++++ .../plusAssignWithDefaultInGetter.kt | 31 ++++++++++++++ .../BlackBoxCodegenTestGenerated.java | 27 ++++++++++++ 7 files changed, 143 insertions(+), 18 deletions(-) create mode 100644 compiler/testData/codegen/box/defaultArguments/convention/incWithDefaultInGetter.kt create mode 100644 compiler/testData/codegen/box/defaultArguments/convention/kt9140.kt create mode 100644 compiler/testData/codegen/box/defaultArguments/convention/plusAssignWithDefaultInGetter.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/DefaultCallMask.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/DefaultCallMask.kt index a36f632e0b9..ce55fb9648b 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/DefaultCallMask.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/DefaultCallMask.kt @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.codegen +import org.jetbrains.org.objectweb.asm.Type import java.util.* class DefaultCallMask(val size: Int) { @@ -48,4 +49,12 @@ class DefaultCallMask(val size: Int) { return masks } + + public fun generateOnStackIfNeeded(callGenerator: CallGenerator): Boolean { + val toInts = toInts() + for (mask in toInts) { + callGenerator.putValueIfNeeded(null, Type.INT_TYPE, StackValue.constant(mask, Type.INT_TYPE)) + } + return toInts.isNotEmpty(); + } } \ No newline at end of file diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 85757c40b7c..dcf30d235a7 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -2477,18 +2477,15 @@ public class ExpressionCodegen extends KtVisitor impleme List valueArguments = resolvedCall.getValueArgumentsByIndex(); assert valueArguments != null : "Failed to arrange value arguments by index: " + resolvedCall.getResultingDescriptor(); - List masks = - argumentGenerator.generate(valueArguments, new ArrayList(resolvedCall.getValueArguments().values())) - .toInts(); + DefaultCallMask masks = + argumentGenerator.generate(valueArguments, new ArrayList(resolvedCall.getValueArguments().values())); if (tailRecursionCodegen.isTailRecursion(resolvedCall)) { tailRecursionCodegen.generateTailRecursion(resolvedCall); return; } - for (int mask : masks) { - callGenerator.putValueIfNeeded(null, Type.INT_TYPE, StackValue.constant(mask, Type.INT_TYPE)); - } + boolean defaultMaskWasGenerated = masks.generateOnStackIfNeeded(callGenerator); // Extra constructor marker argument if (callableMethod instanceof CallableMethod) { @@ -2500,7 +2497,7 @@ public class ExpressionCodegen extends KtVisitor impleme } } - callGenerator.genCall(callableMethod, resolvedCall, !masks.isEmpty(), this); + callGenerator.genCall(callableMethod, resolvedCall, defaultMaskWasGenerated, this); } @NotNull @@ -3532,7 +3529,7 @@ public class ExpressionCodegen extends KtVisitor impleme Callable callableMethod = resolveToCallableMethod(operationDescriptor, false); Type[] argumentTypes = callableMethod.getParameterTypes(); - StackValue collectionElementReceiver = createCollectionElementReceiver( + StackValue.CollectionElementReceiver collectionElementReceiver = createCollectionElementReceiver( expression, receiver, operationDescriptor, isGetter, resolvedGetCall, resolvedSetCall, callable ); @@ -3542,7 +3539,7 @@ public class ExpressionCodegen extends KtVisitor impleme } @NotNull - private StackValue createCollectionElementReceiver( + private StackValue.CollectionElementReceiver createCollectionElementReceiver( @NotNull KtArrayAccessExpression expression, @NotNull StackValue receiver, @NotNull FunctionDescriptor operationDescriptor, diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java index 5be1045765f..17ae92f2af8 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.codegen; import com.intellij.psi.tree.IElementType; +import kotlin.ArraysKt; import kotlin.Unit; import kotlin.jvm.functions.Function1; import org.jetbrains.annotations.Contract; @@ -35,6 +36,7 @@ import org.jetbrains.kotlin.psi.KtExpression; import org.jetbrains.kotlin.resolve.DescriptorUtils; import org.jetbrains.kotlin.resolve.ImportedFromObjectCallableDescriptor; import org.jetbrains.kotlin.resolve.annotations.AnnotationUtilKt; +import org.jetbrains.kotlin.resolve.calls.model.DefaultValueArgument; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; import org.jetbrains.kotlin.resolve.calls.model.ResolvedValueArgument; import org.jetbrains.kotlin.resolve.constants.ConstantValue; @@ -201,7 +203,7 @@ public abstract class StackValue { @NotNull public static StackValue collectionElement( - StackValue collectionElementReceiver, + CollectionElementReceiver collectionElementReceiver, Type type, ResolvedCall getter, ResolvedCall setter, @@ -769,11 +771,12 @@ public abstract class StackValue { private final boolean isGetter; private final ExpressionCodegen codegen; private final ArgumentGenerator argumentGenerator; - private final List valueArguments; + final List valueArguments; private final FrameMap frame; private final StackValue receiver; private final ResolvedCall resolvedGetCall; private final ResolvedCall resolvedSetCall; + DefaultCallMask mask; public CollectionElementReceiver( @NotNull Callable callable, @@ -803,7 +806,7 @@ public abstract class StackValue { ResolvedCall call = isGetter ? resolvedGetCall : resolvedSetCall; StackValue newReceiver = StackValue.receiver(call, receiver, codegen, callable); newReceiver.put(newReceiver.type, v); - argumentGenerator.generate(valueArguments, valueArguments); + mask = argumentGenerator.generate(valueArguments, valueArguments); } @Override @@ -912,7 +915,7 @@ public abstract class StackValue { private final FunctionDescriptor getterDescriptor; public CollectionElement( - @NotNull StackValue collectionElementReceiver, + @NotNull CollectionElementReceiver collectionElementReceiver, @NotNull Type type, @Nullable ResolvedCall resolvedGetCall, @Nullable ResolvedCall resolvedSetCall, @@ -933,11 +936,16 @@ public abstract class StackValue { if (getter == null) { throw new UnsupportedOperationException("no getter specified"); } - - getter.genInvokeInstruction(v); + CallGenerator callGenerator = codegen.defaultCallGenerator; + callGenerator.genCall(getter, resolvedGetCall, genDefaultMaskIfPresent(callGenerator), codegen); coerceTo(type, v); } + private boolean genDefaultMaskIfPresent(CallGenerator callGenerator) { + DefaultCallMask mask = ((CollectionElementReceiver) receiver).mask; + return mask.generateOnStackIfNeeded(callGenerator); + } + @Override public int receiverSize() { if (isStandardStack(codegen.typeMapper, resolvedGetCall, 1) && isStandardStack(codegen.typeMapper, resolvedSetCall, 2)) { @@ -985,9 +993,25 @@ public abstract class StackValue { throw new UnsupportedOperationException("no setter specified"); } - Type[] argumentTypes = setter.getParameterTypes(); - coerce(topOfStackType, argumentTypes[argumentTypes.length - 1], v); - setter.genInvokeInstruction(v); + Type lastParameterType = ArraysKt.last(setter.getParameterTypes()); + coerce(topOfStackType, lastParameterType, v); + //*Convention setter couldn't have default parameters, just getter can have it at last positions + //We should remove default parameters of getter from stack*/ + CollectionElementReceiver collectionElementReceiver = (CollectionElementReceiver) receiver; + if (collectionElementReceiver.isGetter) { + List arguments = collectionElementReceiver.valueArguments; + List types = getter.getValueParameterTypes(); + for (int i = arguments.size() - 1; i >= 0; i--) { + ResolvedValueArgument argument = arguments.get(i); + if (argument instanceof DefaultValueArgument) { + Type defaultType = types.get(i); + AsmUtil.swap(v, lastParameterType, defaultType); + AsmUtil.pop(v, defaultType); + } + } + } + + codegen.defaultCallGenerator.genCall(setter, resolvedSetCall, false, codegen); Type returnType = setter.getReturnType(); if (returnType != Type.VOID_TYPE) { pop(v, returnType); diff --git a/compiler/testData/codegen/box/defaultArguments/convention/incWithDefaultInGetter.kt b/compiler/testData/codegen/box/defaultArguments/convention/incWithDefaultInGetter.kt new file mode 100644 index 00000000000..a576bf5b852 --- /dev/null +++ b/compiler/testData/codegen/box/defaultArguments/convention/incWithDefaultInGetter.kt @@ -0,0 +1,26 @@ +var inc: String = "" + +class X { + var result: String = "fail" + + operator fun get(name: String, type: String = "none") = name + inc + type + + operator fun set(name: String, s: String) { + result = name + s; + } +} + +operator fun String.inc(): String { + inc = this + "1" + return this + "1" +} + +fun box(): String { + var x = X() + val res = ++x["a"] + if (x.result != "aanone1") return "fail 1: ${x.result}" + + if (res != "aanone1none") return "fail 2: ${res}" + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/defaultArguments/convention/kt9140.kt b/compiler/testData/codegen/box/defaultArguments/convention/kt9140.kt new file mode 100644 index 00000000000..ea677b74a84 --- /dev/null +++ b/compiler/testData/codegen/box/defaultArguments/convention/kt9140.kt @@ -0,0 +1,11 @@ +class X { + operator fun get(name: String, type: String = "none") = name + type +} + +fun box(): String { + if (X().get("a") != "anone") return "fail 1: ${X().get("a")}" + + if (X()["a"] != "anone") return "fail 2: ${X()["a"]}" + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/defaultArguments/convention/plusAssignWithDefaultInGetter.kt b/compiler/testData/codegen/box/defaultArguments/convention/plusAssignWithDefaultInGetter.kt new file mode 100644 index 00000000000..0b6f63450de --- /dev/null +++ b/compiler/testData/codegen/box/defaultArguments/convention/plusAssignWithDefaultInGetter.kt @@ -0,0 +1,31 @@ +class X { + var result: String = "fail" + + operator fun get(name: String, type: String = "none") = name + type + + operator fun set(name: String, s: String) { + result = name + s; + } +} + +class Y { + var result: String = "fail" + + operator fun get(name: String, type: String = "no", type2: String = "ne") = name + type + type2 + + operator fun set(name: String, s: String) { + result = name + s; + } +} + +fun box(): String { + var x = X() + x["a"] += "OK" + if (x.result != "aanoneOK") return "fail: ${x.result}" + + var y = Y() + y["a"] += "OK" + if (y.result != "aanoneOK") return "fail: ${y.result}" + + return "OK" +} \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java index 4a92125a07c..9896bb533df 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -2707,6 +2707,33 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { } } + @TestMetadata("compiler/testData/codegen/box/defaultArguments/convention") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Convention extends AbstractBlackBoxCodegenTest { + public void testAllFilesPresentInConvention() throws Exception { + JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/defaultArguments/convention"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("incWithDefaultInGetter.kt") + public void testIncWithDefaultInGetter() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/convention/incWithDefaultInGetter.kt"); + doTest(fileName); + } + + @TestMetadata("kt9140.kt") + public void testKt9140() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/convention/kt9140.kt"); + doTest(fileName); + } + + @TestMetadata("plusAssignWithDefaultInGetter.kt") + public void testPlusAssignWithDefaultInGetter() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/convention/plusAssignWithDefaultInGetter.kt"); + doTest(fileName); + } + } + @TestMetadata("compiler/testData/codegen/box/defaultArguments/function") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)