From acc5303731c0da6a218467edd771fedcfdfe570a Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Mon, 23 May 2016 12:17:56 +0300 Subject: [PATCH] KT-11960 Fix case of instantiation of local class via its inner class or via nested lambda. Move tests to more appropriate location. Fix bug in blackbox codegen generator for JVM, which does not allow to suppress tests. --- .../closureWithSelfInstantiation.kt} | 28 +++++++++++-------- .../localDataClass.kt | 3 -- .../codegen/BlackBoxCodegenTestGenerated.java | 24 ++++++++-------- .../kotlin/generators/tests/GenerateTests.kt | 2 +- .../semantics/LocalClassesTestGenerated.java | 12 ++++++++ .../SecondaryConstructorTestGenerated.java | 12 -------- .../js/translate/context/DeferredCallSite.kt | 6 +++- .../translate/context/TranslationContext.java | 2 +- .../translate/declaration/ClassTranslator.kt | 12 +++++--- 9 files changed, 56 insertions(+), 45 deletions(-) rename compiler/testData/codegen/box/{secondaryConstructors/localClassesWithSelfInstantiation.kt => localClasses/closureWithSelfInstantiation.kt} (69%) rename compiler/testData/codegen/box/{secondaryConstructors => localClasses}/localDataClass.kt (81%) diff --git a/compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt b/compiler/testData/codegen/box/localClasses/closureWithSelfInstantiation.kt similarity index 69% rename from compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt rename to compiler/testData/codegen/box/localClasses/closureWithSelfInstantiation.kt index b559b0d3bab..206ccc3d5f7 100644 --- a/compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt +++ b/compiler/testData/codegen/box/localClasses/closureWithSelfInstantiation.kt @@ -4,17 +4,20 @@ fun box(): String { val capturedInConstructor = 1 val capturedInBody = 10 + var log = "" class A(var x: Int) { var y = 0 fun copy(): A { + log += "A.copy;" val result = A(x) result.y += capturedInBody return result } init { + log += "A.;" y += x + capturedInConstructor } } @@ -23,35 +26,36 @@ fun box(): String { if (a.y != 111) return "fail1a: ${a.y}" if (a.x != 100) return "fail1b: ${a.x}" - // This does not work in JS backend due to some unrelated issue with lambdas - /* + class B(var x: Int) { var y = 0 - fun copier(): () -> B = { - val result = B(x) - result.y += capturedInBody - result + fun copier(): () -> B { + log += "B.copier;" + return { + log += "B.copy;" + val result = B(x) + result.y += capturedInBody + result + } } init { y += x + capturedInConstructor + log += "B.;" } } val b = B(100).copier()() if (b.y != 111) return "fail2a: ${b.y}" if (b.x != 100) return "fail2b: ${b.x}" - */ - // It's pretty hard to implement this properly for now. Presumably, one needs to inject closure fields into local classes - // after the entire code gets generated (which is possible, but not easy, in JS and, I believe, nearly impossible in JVM). - /* class C(var x: Int) { var y = 0 inner class D() { fun copyOuter(): C { + log += "D.copyOuter;" val result = C(x) result.y += capturedInBody return result @@ -59,6 +63,7 @@ fun box(): String { } init { + log += "C.;" y += x + capturedInConstructor } } @@ -66,7 +71,8 @@ fun box(): String { val c = C(100).D().copyOuter() if (c.y != 111) return "fail3a: ${c.y}" if (c.x != 100) return "fail3b: ${c.x}" - */ + + if (log != "A.;A.copy;A.;B.;B.copier;B.copy;B.;C.;D.copyOuter;C.;") return "fail_log: $log" return "OK" } \ No newline at end of file diff --git a/compiler/testData/codegen/box/secondaryConstructors/localDataClass.kt b/compiler/testData/codegen/box/localClasses/localDataClass.kt similarity index 81% rename from compiler/testData/codegen/box/secondaryConstructors/localDataClass.kt rename to compiler/testData/codegen/box/localClasses/localDataClass.kt index 552e12329bd..33cf16258d2 100644 --- a/compiler/testData/codegen/box/secondaryConstructors/localDataClass.kt +++ b/compiler/testData/codegen/box/localClasses/localDataClass.kt @@ -1,6 +1,3 @@ -// Enable for JVM backend when KT-8120 gets fixed -// TARGET_BACKEND: JS - fun box(): String { val capturedInConstructor = 1 diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index f7066c00479..a1dad801d40 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -7863,6 +7863,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) public static class LocalClasses extends AbstractBlackBoxCodegenTest { + @TestMetadata("closureWithSelfInstantiation.kt") + public void ignoredClosureWithSelfInstantiation() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/localClasses/closureWithSelfInstantiation.kt"); + doTest(fileName); + } + public void testAllFilesPresentInLocalClasses() throws Exception { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/localClasses"), Pattern.compile("^(.+)\\.kt$"), true); } @@ -7975,6 +7981,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("localDataClass.kt") + public void testLocalDataClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/localClasses/localDataClass.kt"); + doTest(fileName); + } + @TestMetadata("localExtendsInnerAndReferencesOuterMember.kt") public void testLocalExtendsInnerAndReferencesOuterMember() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/localClasses/localExtendsInnerAndReferencesOuterMember.kt"); @@ -13009,18 +13021,6 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } - @TestMetadata("localClassesWithSelfInstantiation.kt") - public void testLocalClassesWithSelfInstantiation() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt"); - doTest(fileName); - } - - @TestMetadata("localDataClass.kt") - public void testLocalDataClass() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/localDataClass.kt"); - doTest(fileName); - } - @TestMetadata("superCallPrimary.kt") public void testSuperCallPrimary() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/superCallPrimary.kt"); diff --git a/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt b/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt index 3a05c894205..9ee2331c3a1 100644 --- a/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt +++ b/generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt @@ -202,7 +202,7 @@ fun main(args: Array) { GenerateRangesCodegenTestData.main(arrayOf()) testClass() { - model("codegen/box") + model("codegen/box", targetBackend = TargetBackend.JVM) } testClass("BlackBoxInlineCodegenTestGenerated") { diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/LocalClassesTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/LocalClassesTestGenerated.java index 278abc502ce..341842c3217 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/LocalClassesTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/LocalClassesTestGenerated.java @@ -47,6 +47,12 @@ public class LocalClassesTestGenerated extends AbstractLocalClassesTest { doTest(fileName); } + @TestMetadata("closureWithSelfInstantiation.kt") + public void testClosureWithSelfInstantiation() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/localClasses/closureWithSelfInstantiation.kt"); + doTest(fileName); + } + @TestMetadata("inExtensionFunction.kt") public void testInExtensionFunction() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/localClasses/inExtensionFunction.kt"); @@ -143,6 +149,12 @@ public class LocalClassesTestGenerated extends AbstractLocalClassesTest { doTest(fileName); } + @TestMetadata("localDataClass.kt") + public void testLocalDataClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/localClasses/localDataClass.kt"); + doTest(fileName); + } + @TestMetadata("localExtendsInnerAndReferencesOuterMember.kt") public void testLocalExtendsInnerAndReferencesOuterMember() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/localClasses/localExtendsInnerAndReferencesOuterMember.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/SecondaryConstructorTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/SecondaryConstructorTestGenerated.java index 88720ade084..e2c64f89ada 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/SecondaryConstructorTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/SecondaryConstructorTestGenerated.java @@ -161,18 +161,6 @@ public class SecondaryConstructorTestGenerated extends AbstractSecondaryConstruc doTest(fileName); } - @TestMetadata("localClassesWithSelfInstantiation.kt") - public void testLocalClassesWithSelfInstantiation() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/localClassesWithSelfInstantiation.kt"); - doTest(fileName); - } - - @TestMetadata("localDataClass.kt") - public void testLocalDataClass() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/localDataClass.kt"); - doTest(fileName); - } - @TestMetadata("superCallPrimary.kt") public void testSuperCallPrimary() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/secondaryConstructors/superCallPrimary.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/DeferredCallSite.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/DeferredCallSite.kt index 9a3b8177855..fec2651c7b4 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/DeferredCallSite.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/DeferredCallSite.kt @@ -35,4 +35,8 @@ package org.jetbrains.kotlin.js.translate.context import com.google.dart.compiler.backend.js.ast.JsExpression import org.jetbrains.kotlin.descriptors.ConstructorDescriptor -class DeferredCallSite(val constructor: ConstructorDescriptor, val invocationArgs: MutableList) +class DeferredCallSite( + val constructor: ConstructorDescriptor, + val invocationArgs: MutableList, + val context: TranslationContext +) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java index 06d2c09143f..a487e9e32eb 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java @@ -537,6 +537,6 @@ public class TranslationContext { List callSites = staticContext.getDeferredCallSites().get(classDescriptor); if (callSites == null) throw new IllegalStateException("This method should be call only when `shouldBeDeferred` method " + "reports true for given constructor: " + constructor); - callSites.add(new DeferredCallSite(constructor, invocationArgs)); + callSites.add(new DeferredCallSite(constructor, invocationArgs, this)); } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt index 9d1f3fabbf7..06c4322aea3 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt @@ -276,15 +276,19 @@ class ClassTranslator private constructor( val capturedVars = (nonConstructorCapturedVars + constructorCapturedVars).distinct() val descriptor = constructor.descriptor + val classDescriptor = DescriptorUtils.getParentOfType(descriptor, ClassDescriptor::class.java, false)!! nonConstructorContext.putClassOrConstructorClosure(descriptor, capturedVars) val constructorCallSites = callSiteMap[constructor.descriptor].orEmpty() + for (callSite in constructorCallSites) { + val closureQualifier = callSite.context.getArgumentForClosureConstructor(classDescriptor.thisAsReceiverParameter) capturedVars.forEach { nonConstructorUsageTracker.used(it) } - val closureArgs = capturedVars - .map { nonConstructorUsageTracker.capturedDescriptorToJsName[it]!! } - .map { JsAstUtils.fqnWithoutSideEffects(it, JsLiteral.THIS) } - callSite.invocationArgs.addAll(0, closureArgs) + val closureArgs = capturedVars.map { + val name = nonConstructorUsageTracker.getNameForCapturedDescriptor(it)!! + JsAstUtils.fqnWithoutSideEffects(name, closureQualifier) + } + callSite.invocationArgs.addAll(0, closureArgs.toList()) } } }