From 7041a10ecf4b83a5a910f85ddef8f11e7074fe1c Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 13 Jan 2014 21:23:38 +0400 Subject: [PATCH] Fix VerifyError on primitive override properties Don't just blindly map property type in PropertyCodegen when generating getter: we already have its full signature, so we can just as well take the type from it. The former also isn't correct for properties which are overrides with a primitive type. Simple mapType is safe for setter though, because the type of a method parameter cannot change with override on JVM The change in StackValue relates to call sites of properties which return Unit: in that case StackValue's this.type is V, whereas the return type of its getter is Ljet/Unit; -- so coerceTo was coercing from the wrong type in case of getter calls #KT-4373 Fixed #KT-4383 Fixed --- .../jet/codegen/PropertyCodegen.java | 15 +++++--------- .../org/jetbrains/jet/codegen/StackValue.java | 3 ++- .../testData/codegen/box/properties/kt4373.kt | 14 +++++++++++++ .../testData/codegen/box/properties/kt4383.kt | 16 +++++++++++++++ .../primitiveOverrideDefaultAccessor.kt | 11 ++++++++++ .../primitiveOverrideDelegateAccessor.kt | 18 +++++++++++++++++ .../BlackBoxCodegenTestGenerated.java | 20 +++++++++++++++++++ 7 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 compiler/testData/codegen/box/properties/kt4373.kt create mode 100644 compiler/testData/codegen/box/properties/kt4383.kt create mode 100644 compiler/testData/codegen/box/properties/primitiveOverrideDefaultAccessor.kt create mode 100644 compiler/testData/codegen/box/properties/primitiveOverrideDelegateAccessor.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java index 52b6a2fb15c..cd29cfeb183 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java @@ -316,7 +316,6 @@ public class PropertyCodegen extends GenerationStateAware { public void doGenerateBody(@NotNull ExpressionCodegen codegen, @NotNull JvmMethodSignature signature) { InstructionAdapter v = codegen.v; PropertyDescriptor propertyDescriptor = callableDescriptor.getCorrespondingProperty(); - Type type = codegen.typeMapper.mapType(propertyDescriptor); int paramCode = 0; if (codegen.context.getContextKind() != OwnerKind.NAMESPACE) { @@ -327,6 +326,7 @@ public class PropertyCodegen extends GenerationStateAware { StackValue property = codegen.intermediateValueForProperty(propertyDescriptor, true, null); if (callableDescriptor instanceof PropertyGetterDescriptor) { + Type type = signature.getReturnType(); property.put(type, v); v.areturn(type); } @@ -335,8 +335,8 @@ public class PropertyCodegen extends GenerationStateAware { if (receiverParameter != null) { paramCode += codegen.typeMapper.mapType(receiverParameter.getType()).getSize(); } + Type type = codegen.typeMapper.mapType(propertyDescriptor); v.load(paramCode, type); - property.store(type, v); v.visitInsn(RETURN); } else { @@ -370,14 +370,9 @@ public class PropertyCodegen extends GenerationStateAware { StackValue delegatedProperty = codegen.intermediateValueForProperty(propertyDescriptor, true, null); StackValue lastValue = codegen.invokeFunction(call, delegatedProperty, resolvedCall); - if (lastValue.type != Type.VOID_TYPE) { - Type asmType = codegen.typeMapper.mapType(propertyDescriptor); - lastValue.put(asmType, v); - v.areturn(asmType); - } - else { - v.areturn(Type.VOID_TYPE); - } + Type asmType = signature.getReturnType(); + lastValue.put(asmType, v); + v.areturn(asmType); } } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java index f5b915bfd72..c0e5ac16306 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java @@ -865,12 +865,13 @@ public abstract class StackValue { v.visitFieldInsn(isStatic ? GETSTATIC : GETFIELD, methodOwner.getInternalName(), getPropertyName(), this.type.getDescriptor()); genNotNullAssertionForField(v, state, descriptor); + coerceTo(type, v); } else { Method method = getter.getAsmMethod(); v.visitMethodInsn(getter.getInvokeOpcode(), getter.getOwner().getInternalName(), method.getName(), method.getDescriptor()); + coerce(method.getReturnType(), type, v); } - coerceTo(type, v); } @Override diff --git a/compiler/testData/codegen/box/properties/kt4373.kt b/compiler/testData/codegen/box/properties/kt4373.kt new file mode 100644 index 00000000000..74d2ae00485 --- /dev/null +++ b/compiler/testData/codegen/box/properties/kt4373.kt @@ -0,0 +1,14 @@ +trait Tr { + val prop: T +} + +class A(a: Tr) : Tr by a + +fun eat(x: Int) {} + +fun box(): String { + eat(A(object : Tr { + override val prop = 42 + }).prop) + return "OK" +} diff --git a/compiler/testData/codegen/box/properties/kt4383.kt b/compiler/testData/codegen/box/properties/kt4383.kt new file mode 100644 index 00000000000..c29f6d42147 --- /dev/null +++ b/compiler/testData/codegen/box/properties/kt4383.kt @@ -0,0 +1,16 @@ +class D { + fun get(a: Any, p: PropertyMetadata) { } +} + +object P { + val u = Unit.VALUE + val v by D() + var w = Unit.VALUE +} + +fun box(): String { + if (P.u != P.v) return "Fail uv" + P.w = Unit.VALUE + if (P.w != P.u) return "Fail w" + return "OK" +} diff --git a/compiler/testData/codegen/box/properties/primitiveOverrideDefaultAccessor.kt b/compiler/testData/codegen/box/properties/primitiveOverrideDefaultAccessor.kt new file mode 100644 index 00000000000..ab8f284c04e --- /dev/null +++ b/compiler/testData/codegen/box/properties/primitiveOverrideDefaultAccessor.kt @@ -0,0 +1,11 @@ +trait R> { + var value: T +} + +class A(override var value: Int): R + +fun box(): String { + val a = A(239) + a.value = 42 + return if (a.value == 42) "OK" else "Fail 1" +} diff --git a/compiler/testData/codegen/box/properties/primitiveOverrideDelegateAccessor.kt b/compiler/testData/codegen/box/properties/primitiveOverrideDelegateAccessor.kt new file mode 100644 index 00000000000..4801d9b5df6 --- /dev/null +++ b/compiler/testData/codegen/box/properties/primitiveOverrideDelegateAccessor.kt @@ -0,0 +1,18 @@ +class Holder(var value: Int) { + fun get(that: Any?, desc: PropertyMetadata) = value + fun set(that: Any?, desc: PropertyMetadata, newValue: Int) { value = newValue } +} + +trait R> { + var value: T +} + +class A(start: Int) : R { + override var value: Int by Holder(start) +} + +fun box(): String { + val a = A(239) + a.value = 42 + return if (a.value == 42) "OK" else "Fail 1" +} diff --git a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java index da9a5508ecd..dc2fbd8f2f4 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -4470,11 +4470,31 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest("compiler/testData/codegen/box/properties/kt4252_2.kt"); } + @TestMetadata("kt4373.kt") + public void testKt4373() throws Exception { + doTest("compiler/testData/codegen/box/properties/kt4373.kt"); + } + + @TestMetadata("kt4383.kt") + public void testKt4383() throws Exception { + doTest("compiler/testData/codegen/box/properties/kt4383.kt"); + } + @TestMetadata("kt613.kt") public void testKt613() throws Exception { doTest("compiler/testData/codegen/box/properties/kt613.kt"); } + @TestMetadata("primitiveOverrideDefaultAccessor.kt") + public void testPrimitiveOverrideDefaultAccessor() throws Exception { + doTest("compiler/testData/codegen/box/properties/primitiveOverrideDefaultAccessor.kt"); + } + + @TestMetadata("primitiveOverrideDelegateAccessor.kt") + public void testPrimitiveOverrideDelegateAccessor() throws Exception { + doTest("compiler/testData/codegen/box/properties/primitiveOverrideDelegateAccessor.kt"); + } + @TestMetadata("privatePropertyWithoutBackingField.kt") public void testPrivatePropertyWithoutBackingField() throws Exception { doTest("compiler/testData/codegen/box/properties/privatePropertyWithoutBackingField.kt");