From f364d23ee96dd4b8bd5123937ebe11524eb66007 Mon Sep 17 00:00:00 2001 From: dnpetrov Date: Fri, 19 Jun 2015 10:51:14 +0300 Subject: [PATCH] KT-5347 VerifyError on local data class with closure - generated copy() should push captured fields on stack - refactor context lookup - handle vars in local classes using instance fields #KT-5347 Fixed --- .../kotlin/codegen/ExpressionCodegen.java | 13 +++++- .../codegen/ImplementationBodyCodegen.java | 34 +++++++++++--- .../jetbrains/kotlin/codegen/StackValue.java | 14 ++++-- .../context/EnclosedValueDescriptor.java | 30 ++++++++----- .../kotlin/codegen/context/LocalLookup.java | 17 +++++-- .../testData/codegen/box/classes/kt5347.kt | 44 +++++++++++++++++++ .../BlackBoxCodegenTestGenerated.java | 6 +++ 7 files changed, 134 insertions(+), 24 deletions(-) create mode 100644 compiler/testData/codegen/box/classes/kt5347.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 77ea4aa6ebd..c7f0f2dc69e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -1528,7 +1528,7 @@ public class ExpressionCodegen extends JetVisitor implem if (sharedVarType == null) { sharedVarType = typeMapper.mapType((VariableDescriptor) entry.getKey()); } - StackValue capturedVar = entry.getValue().getOuterValue(this); + StackValue capturedVar = lookupOuterValue(entry.getValue()); callGenerator.putCapturedValueOnStack(capturedVar, sharedVarType, paramIndex++); } @@ -1551,6 +1551,17 @@ public class ExpressionCodegen extends JetVisitor implem return generateBlock(expression.getStatements(), isStatement, null, null); } + @NotNull + public StackValue lookupOuterValue(EnclosedValueDescriptor d) { + DeclarationDescriptor descriptor = d.getDescriptor(); + for (LocalLookup.LocalLookupCase aCase : LocalLookup.LocalLookupCase.values()) { + if (aCase.isCase(descriptor)) { + return aCase.outerValue(d, this); + } + } + throw new IllegalStateException("Can't get outer value in " + this + " for " + d); + } + private StackValue generateBlock( List statements, boolean isStatement, diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index dc5df70fd9d..1d7643197a4 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java @@ -727,12 +727,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { MutableClosure closure = ImplementationBodyCodegen.this.context.closure; if (closure != null) { - ClassDescriptor captureThis = closure.getCaptureThis(); - if (captureThis != null) { - iv.load(0, classAsmType); - Type type = typeMapper.mapType(captureThis); - iv.getfield(classAsmType.getInternalName(), CAPTURED_THIS_FIELD, type.getDescriptor()); - } + pushCapturedFieldsOnStack(iv, closure); } int parameterIndex = 1; // localVariable 0 = this @@ -747,6 +742,33 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { iv.areturn(thisDescriptorType); } + + private void pushCapturedFieldsOnStack(InstructionAdapter iv, MutableClosure closure) { + ClassDescriptor captureThis = closure.getCaptureThis(); + if (captureThis != null) { + iv.load(0, classAsmType); + Type type = typeMapper.mapType(captureThis); + iv.getfield(classAsmType.getInternalName(), CAPTURED_THIS_FIELD, type.getDescriptor()); + } + + JetType captureReceiver = closure.getCaptureReceiverType(); + if (captureReceiver != null) { + iv.load(0, classAsmType); + Type type = typeMapper.mapType(captureReceiver); + iv.getfield(classAsmType.getInternalName(), CAPTURED_RECEIVER_FIELD, type.getDescriptor()); + } + + for (Map.Entry entry : closure.getCaptureVariables().entrySet()) { + DeclarationDescriptor declarationDescriptor = entry.getKey(); + EnclosedValueDescriptor enclosedValueDescriptor = entry.getValue(); + StackValue capturedValue = enclosedValueDescriptor.getInstanceValue(); + Type sharedVarType = typeMapper.getSharedVarType(declarationDescriptor); + if (sharedVarType == null) { + sharedVarType = typeMapper.mapType((VariableDescriptor) declarationDescriptor); + } + capturedValue.put(sharedVarType, iv); + } + } }); functionCodegen.generateDefaultIfNeeded( diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java index 9b3c73ea6a8..f99bc87ba1d 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java @@ -412,15 +412,23 @@ public abstract class StackValue { return None.INSTANCE; } - public static FieldForSharedVar fieldForSharedVar( + public static Field receiverWithRefWrapper( @NotNull Type localType, @NotNull Type classType, @NotNull String fieldName, @NotNull StackValue receiver, @Nullable DeclarationDescriptor descriptor ) { - Field receiverWithRefWrapper = field(sharedTypeForType(localType), classType, fieldName, false, receiver, descriptor); - return new FieldForSharedVar(localType, classType, fieldName, receiverWithRefWrapper); + return field(sharedTypeForType(localType), classType, fieldName, false, receiver, descriptor); + } + + public static FieldForSharedVar fieldForSharedVar( + @NotNull Type localType, + @NotNull Type classType, + @NotNull String fieldName, + @NotNull Field refWrapper + ) { + return new FieldForSharedVar(localType, classType, fieldName, refWrapper); } @NotNull diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/EnclosedValueDescriptor.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/EnclosedValueDescriptor.java index 3f8d626af6c..c1561c4c1a1 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/EnclosedValueDescriptor.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/EnclosedValueDescriptor.java @@ -27,6 +27,7 @@ public final class EnclosedValueDescriptor { private final String fieldName; private final DeclarationDescriptor descriptor; private final StackValue.StackValueWithSimpleReceiver innerValue; + private final StackValue instanceValue; private final Type type; public EnclosedValueDescriptor( @@ -38,6 +39,21 @@ public final class EnclosedValueDescriptor { this.fieldName = fieldName; this.descriptor = descriptor; this.innerValue = innerValue; + this.instanceValue = innerValue; + this.type = type; + } + + public EnclosedValueDescriptor( + @NotNull String name, + @Nullable DeclarationDescriptor descriptor, + @NotNull StackValue.StackValueWithSimpleReceiver innerValue, + @NotNull StackValue.Field instanceValue, + @NotNull Type type + ) { + this.fieldName = name; + this.descriptor = descriptor; + this.innerValue = innerValue; + this.instanceValue = instanceValue; this.type = type; } @@ -57,19 +73,13 @@ public final class EnclosedValueDescriptor { } @NotNull - public Type getType() { - return type; + public StackValue getInstanceValue() { + return instanceValue; } @NotNull - public StackValue getOuterValue(@NotNull ExpressionCodegen codegen) { - for (LocalLookup.LocalLookupCase aCase : LocalLookup.LocalLookupCase.values()) { - if (aCase.isCase(descriptor)) { - return aCase.outerValue(this, codegen); - } - } - - throw new IllegalStateException("Can't get outer value in " + codegen + " for " + this); + public Type getType() { + return type; } @Override diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/LocalLookup.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/LocalLookup.java index 7ece0709a11..401a7314ebd 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/LocalLookup.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/LocalLookup.java @@ -61,12 +61,21 @@ public interface LocalLookup { String fieldName = "$" + vd.getName(); StackValue.Local thiz = StackValue.LOCAL_0; - StackValue.StackValueWithSimpleReceiver innerValue = sharedVarType != null - ? StackValue.fieldForSharedVar(localType, classType, fieldName, thiz, vd) - : StackValue.field(type, classType, fieldName, false, thiz, vd); + + StackValue.StackValueWithSimpleReceiver innerValue; + EnclosedValueDescriptor enclosedValueDescriptor; + if (sharedVarType != null) { + StackValue.Field wrapperValue = StackValue.receiverWithRefWrapper(localType, classType, fieldName, thiz, vd); + innerValue = StackValue.fieldForSharedVar(localType, classType, fieldName, wrapperValue); + enclosedValueDescriptor = new EnclosedValueDescriptor(fieldName, d, innerValue, wrapperValue, type); + } + else { + innerValue = StackValue.field(type, classType, fieldName, false, thiz, vd); + enclosedValueDescriptor = new EnclosedValueDescriptor(fieldName, d, innerValue, type); + } closure.recordField(fieldName, type); - closure.captureVariable(new EnclosedValueDescriptor(fieldName, d, innerValue, type)); + closure.captureVariable(enclosedValueDescriptor); return innerValue; } diff --git a/compiler/testData/codegen/box/classes/kt5347.kt b/compiler/testData/codegen/box/classes/kt5347.kt new file mode 100644 index 00000000000..d97668acadd --- /dev/null +++ b/compiler/testData/codegen/box/classes/kt5347.kt @@ -0,0 +1,44 @@ +fun test1(str: String): String { + @data class A(val x: Int) { + fun foo() = str + } + return A(0).copy().foo() +} + +class TestClass(val x: String) { + fun foo(): String { + @data class A(val x: Int) { + fun foo() = this@TestClass.x + } + return A(0).copy().foo() + } +} + +fun test2(str: String): String = TestClass(str).foo() + +fun test3(str: String): String { + var xx = "" + @data class A(val x: Int) { + fun foo(): String { xx = str; return xx } + } + return A(0).copy().foo() +} + +fun test4(str: String): String { + var xx = "" + fun bar(s: String): String { xx = s; return xx } + @data class A(val x: Int) { + fun foo(): String = bar(str) + } + return A(0).copy().foo() +} + +fun box(): String { + return when { + test1("test1") != "test1" -> "Failed #1 (parameter capture)" + test2("test2") != "test2" -> "Failed #2 ('this' capture)" + test3("test3") != "test3" -> "Failed #3 ('var' capture)" + test4("test4") != "test4" -> "Failed #4 (local function capture)" + else -> "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 35e3b9989f6..5be3da6ada1 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -1378,6 +1378,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("kt5347.kt") + public void testKt5347() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/kt5347.kt"); + doTest(fileName); + } + @TestMetadata("kt6136.kt") public void testKt6136() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/kt6136.kt");