From 35c7c4afd88dcb104a98551ce320c36e86cc84ef Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 12 Feb 2014 19:08:43 +0400 Subject: [PATCH] Fix capturing outer this in some cases in JVM codegen When property initializer of some inner entity (e.g. anonymous object) contains a reference to some outer entity (say, a property of the outer class), we need to make sure we called "lookupInContext" on this entity's owner class, so that "setCaptureThis" was called on the appropriate closure #KT-4176 Fixed --- .../codegen/ImplementationBodyCodegen.java | 34 ++++++-------- .../captureFunctionInProperty.kt | 15 ++++++ .../captureOuterProperty/inFunction.kt | 13 ++++++ .../captureOuterProperty/inProperty.kt | 13 ++++++ .../inPropertyDeepObjectChain.kt | 18 ++++++++ .../inPropertyFromSuperClass.kt | 15 ++++++ .../inPropertyFromSuperSuperClass.kt | 17 +++++++ .../closures/captureOuterProperty/kt4176.kt | 17 +++++++ .../objectCaptureOuterConstructorProperty.kt | 29 ++++++++++++ .../BlackBoxCodegenTestGenerated.java | 46 ++++++++++++++++++- ...lackBoxWithStdlibCodegenTestGenerated.java | 5 ++ 11 files changed, 201 insertions(+), 21 deletions(-) create mode 100644 compiler/testData/codegen/box/closures/captureOuterProperty/captureFunctionInProperty.kt create mode 100644 compiler/testData/codegen/box/closures/captureOuterProperty/inFunction.kt create mode 100644 compiler/testData/codegen/box/closures/captureOuterProperty/inProperty.kt create mode 100644 compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyDeepObjectChain.kt create mode 100644 compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperClass.kt create mode 100644 compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperSuperClass.kt create mode 100644 compiler/testData/codegen/box/closures/captureOuterProperty/kt4176.kt create mode 100644 compiler/testData/codegen/boxWithStdlib/regressions/objectCaptureOuterConstructorProperty.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java index 2e1510a65ee..5d9f25d6d6e 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java @@ -18,7 +18,6 @@ package org.jetbrains.jet.codegen; import com.google.common.collect.Lists; import com.google.common.collect.Sets; -import com.google.protobuf.ExtensionRegistry; import com.intellij.openapi.progress.ProcessCanceledException; import com.intellij.openapi.util.Pair; import com.intellij.psi.PsiElement; @@ -42,8 +41,6 @@ import org.jetbrains.jet.descriptors.serialization.ClassData; import org.jetbrains.jet.descriptors.serialization.DescriptorSerializer; import org.jetbrains.jet.descriptors.serialization.ProtoBuf; import org.jetbrains.jet.lang.descriptors.*; -import org.jetbrains.jet.lang.descriptors.impl.MutableClassDescriptor; -import org.jetbrains.jet.lang.diagnostics.DiagnosticUtils; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.BindingContext; import org.jetbrains.jet.lang.resolve.BindingContextUtils; @@ -1328,27 +1325,24 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { @Override public void visitSimpleNameExpression(@NotNull JetSimpleNameExpression expr) { DeclarationDescriptor descriptor = bindingContext.get(BindingContext.REFERENCE_TARGET, expr); - if (descriptor instanceof VariableDescriptor && !(descriptor instanceof PropertyDescriptor)) { + + DeclarationDescriptor toLookup; + if (isLocalNamedFun(descriptor)) { + toLookup = descriptor; + } + else if (descriptor instanceof CallableMemberDescriptor) { + toLookup = descriptor.getContainingDeclaration(); + } + else if (descriptor instanceof VariableDescriptor) { ConstructorDescriptor constructorDescriptor = (ConstructorDescriptor) constructorContext.getContextDescriptor(); for (ValueParameterDescriptor parameterDescriptor : constructorDescriptor.getValueParameters()) { - //noinspection ConstantConditions - if (descriptor.equals(parameterDescriptor)) { - return; - } + if (descriptor.equals(parameterDescriptor)) return; } - constructorContext.lookupInContext(descriptor, null, state, true); - } else if (isLocalNamedFun(descriptor)) { - assert descriptor != null; - MutableClassDescriptor classDescriptor = - (MutableClassDescriptor) constructorContext.getParentContext().getContextDescriptor(); - - for (CallableMemberDescriptor memberDescriptor : classDescriptor.getAllCallableMembers()) { - if (descriptor.equals(memberDescriptor)) { - return; - } - } - constructorContext.lookupInContext(descriptor, null, state, true); + toLookup = descriptor; } + else return; + + constructorContext.lookupInContext(toLookup, null, state, true); } @Override diff --git a/compiler/testData/codegen/box/closures/captureOuterProperty/captureFunctionInProperty.kt b/compiler/testData/codegen/box/closures/captureOuterProperty/captureFunctionInProperty.kt new file mode 100644 index 00000000000..57d93cb270a --- /dev/null +++ b/compiler/testData/codegen/box/closures/captureOuterProperty/captureFunctionInProperty.kt @@ -0,0 +1,15 @@ +trait T { + fun result(): String +} + +class A(val x: String) { + fun getx() = x + + fun foo() = object : T { + val bar = getx() + + override fun result() = bar + } +} + +fun box() = A("OK").foo().result() diff --git a/compiler/testData/codegen/box/closures/captureOuterProperty/inFunction.kt b/compiler/testData/codegen/box/closures/captureOuterProperty/inFunction.kt new file mode 100644 index 00000000000..03bdb3b499a --- /dev/null +++ b/compiler/testData/codegen/box/closures/captureOuterProperty/inFunction.kt @@ -0,0 +1,13 @@ +trait T { + fun result(): String +} + +class A(val x: String) { + fun foo() = object : T { + fun bar() = x + + override fun result() = bar() + } +} + +fun box() = A("OK").foo().result() diff --git a/compiler/testData/codegen/box/closures/captureOuterProperty/inProperty.kt b/compiler/testData/codegen/box/closures/captureOuterProperty/inProperty.kt new file mode 100644 index 00000000000..15769f77148 --- /dev/null +++ b/compiler/testData/codegen/box/closures/captureOuterProperty/inProperty.kt @@ -0,0 +1,13 @@ +trait T { + fun result(): String +} + +class A(val x: String) { + fun foo() = object : T { + val bar = x + + override fun result() = bar + } +} + +fun box() = A("OK").foo().result() diff --git a/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyDeepObjectChain.kt b/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyDeepObjectChain.kt new file mode 100644 index 00000000000..2dfd7949ca6 --- /dev/null +++ b/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyDeepObjectChain.kt @@ -0,0 +1,18 @@ +trait T { + fun result(): String +} + +class A(val x: String) { + fun foo() = object : T { + fun bar() = object : T { + fun baz() = object : T { + val y = x + override fun result() = y + } + override fun result() = baz().result() + } + override fun result() = bar().result() + } +} + +fun box() = A("OK").foo().result() diff --git a/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperClass.kt b/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperClass.kt new file mode 100644 index 00000000000..934d0b58ec7 --- /dev/null +++ b/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperClass.kt @@ -0,0 +1,15 @@ +trait T { + fun result(): String +} + +open class B(val x: String) + +class A : B("OK") { + fun foo() = object : T { + val bar = x + + override fun result() = bar + } +} + +fun box() = A().foo().result() diff --git a/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperSuperClass.kt b/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperSuperClass.kt new file mode 100644 index 00000000000..4dc735a2bf7 --- /dev/null +++ b/compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperSuperClass.kt @@ -0,0 +1,17 @@ +trait T { + fun result(): String +} + +abstract class A(val x: Z) + +open class B : A("OK") + +class C : B() { + fun foo() = object : T { + val bar = x + + override fun result() = bar + } +} + +fun box() = C().foo().result() diff --git a/compiler/testData/codegen/box/closures/captureOuterProperty/kt4176.kt b/compiler/testData/codegen/box/closures/captureOuterProperty/kt4176.kt new file mode 100644 index 00000000000..129b3ad92c8 --- /dev/null +++ b/compiler/testData/codegen/box/closures/captureOuterProperty/kt4176.kt @@ -0,0 +1,17 @@ +open class Z(val s: Int) { + open fun a() {} +} + +class B(val x: Int) { + fun foo() { + class X : Z(x) { + + } + X() + } +} + +fun box(): String { + B(1).foo() + return "OK" +} diff --git a/compiler/testData/codegen/boxWithStdlib/regressions/objectCaptureOuterConstructorProperty.kt b/compiler/testData/codegen/boxWithStdlib/regressions/objectCaptureOuterConstructorProperty.kt new file mode 100644 index 00000000000..c25d8948d60 --- /dev/null +++ b/compiler/testData/codegen/boxWithStdlib/regressions/objectCaptureOuterConstructorProperty.kt @@ -0,0 +1,29 @@ +import kotlin.support.* + +trait Stream { + fun iterator(): Iterator +} + +class ZippingStream(val stream1: Stream, val stream2: Stream) : Stream> { + override fun iterator(): Iterator> = object : AbstractIterator>() { + val iterator1 = stream1.iterator() + val iterator2 = stream2.iterator() + override fun computeNext() { + if (iterator1.hasNext() && iterator2.hasNext()) { + setNext(iterator1.next() to iterator2.next()) + } else { + done() + } + } + } +} + + +object EmptyStream : Stream { + override fun iterator() = listOf().iterator() +} + +fun box(): String { + ZippingStream(EmptyStream, EmptyStream).iterator().hasNext() + return "OK" +} diff --git a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java index 684e45198a0..09f2d646008 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -1384,7 +1384,7 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { } @TestMetadata("compiler/testData/codegen/box/closures") - @InnerTestClasses({Closures.ClosureInsideClosure.class}) + @InnerTestClasses({Closures.CaptureOuterProperty.class, Closures.ClosureInsideClosure.class}) public static class Closures extends AbstractBlackBoxCodegenTest { public void testAllFilesPresentInClosures() throws Exception { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("compiler/testData/codegen/box/closures"), Pattern.compile("^(.+)\\.kt$"), true); @@ -1510,6 +1510,49 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest("compiler/testData/codegen/box/closures/simplestClosureAndBoxing.kt"); } + @TestMetadata("compiler/testData/codegen/box/closures/captureOuterProperty") + public static class CaptureOuterProperty extends AbstractBlackBoxCodegenTest { + public void testAllFilesPresentInCaptureOuterProperty() throws Exception { + JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("compiler/testData/codegen/box/closures/captureOuterProperty"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("captureFunctionInProperty.kt") + public void testCaptureFunctionInProperty() throws Exception { + doTest("compiler/testData/codegen/box/closures/captureOuterProperty/captureFunctionInProperty.kt"); + } + + @TestMetadata("inFunction.kt") + public void testInFunction() throws Exception { + doTest("compiler/testData/codegen/box/closures/captureOuterProperty/inFunction.kt"); + } + + @TestMetadata("inProperty.kt") + public void testInProperty() throws Exception { + doTest("compiler/testData/codegen/box/closures/captureOuterProperty/inProperty.kt"); + } + + @TestMetadata("inPropertyDeepObjectChain.kt") + public void testInPropertyDeepObjectChain() throws Exception { + doTest("compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyDeepObjectChain.kt"); + } + + @TestMetadata("inPropertyFromSuperClass.kt") + public void testInPropertyFromSuperClass() throws Exception { + doTest("compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperClass.kt"); + } + + @TestMetadata("inPropertyFromSuperSuperClass.kt") + public void testInPropertyFromSuperSuperClass() throws Exception { + doTest("compiler/testData/codegen/box/closures/captureOuterProperty/inPropertyFromSuperSuperClass.kt"); + } + + @TestMetadata("kt4176.kt") + public void testKt4176() throws Exception { + doTest("compiler/testData/codegen/box/closures/captureOuterProperty/kt4176.kt"); + } + + } + @TestMetadata("compiler/testData/codegen/box/closures/closureInsideClosure") public static class ClosureInsideClosure extends AbstractBlackBoxCodegenTest { public void testAllFilesPresentInClosureInsideClosure() throws Exception { @@ -1551,6 +1594,7 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { public static Test innerSuite() { TestSuite suite = new TestSuite("Closures"); suite.addTestSuite(Closures.class); + suite.addTestSuite(CaptureOuterProperty.class); suite.addTestSuite(ClosureInsideClosure.class); return suite; } diff --git a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java index 24ceea3f560..8e75815961e 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java @@ -1018,6 +1018,11 @@ public class BlackBoxWithStdlibCodegenTestGenerated extends AbstractBlackBoxCode doTestWithStdlib("compiler/testData/codegen/boxWithStdlib/regressions/kt864.kt"); } + @TestMetadata("objectCaptureOuterConstructorProperty.kt") + public void testObjectCaptureOuterConstructorProperty() throws Exception { + doTestWithStdlib("compiler/testData/codegen/boxWithStdlib/regressions/objectCaptureOuterConstructorProperty.kt"); + } + @TestMetadata("referenceToSelfInLocal.kt") public void testReferenceToSelfInLocal() throws Exception { doTestWithStdlib("compiler/testData/codegen/boxWithStdlib/regressions/referenceToSelfInLocal.kt");