From 44d56cb2789509f90b15e7287232080b013577e2 Mon Sep 17 00:00:00 2001 From: Anton Bannykh Date: Tue, 9 Oct 2018 14:52:33 +0300 Subject: [PATCH] JS: fix val with backing field initialization in secondary constructors This change reverts the AssignmentTranslator logic to a previous state of "if we assign to a val, tranlate to backing field". Previously a check whether or not we are inside of a constructor was added. The check didn't detect secondary constructors, hence initializing of val's with backing field started to work incorrectly. The check itself was added in an attempt to prevent augmented assignment operators to reference the backing field. The check seems to have been wrong, because an augmented assignment could happen inside a construcotr. A more correct fix was added later. It seems that it is safe now to revert the logic back and rely on the frontend to only allow assignment to a val property during initilization. --- .../withoutPrimarySimple.kt | 10 +++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 +++ .../LightAnalysisModeTestGenerated.java | 5 +++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 +++ .../IrJsCodegenBoxTestGenerated.java | 5 +++ .../semantics/JsCodegenBoxTestGenerated.java | 5 +++ .../operation/AssignmentTranslator.java | 44 ++++--------------- 7 files changed, 44 insertions(+), 35 deletions(-) create mode 100644 compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt diff --git a/compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt b/compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt new file mode 100644 index 00000000000..35092448120 --- /dev/null +++ b/compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt @@ -0,0 +1,10 @@ +class A { + val value: String + get() = field + "K" + + constructor(o: String) { + value = o + } +} + +fun box() = A("O").value \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 390cf3fea0a..50a6f0beca5 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -21347,6 +21347,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { public void testWithoutPrimary() throws Exception { runTest("compiler/testData/codegen/box/secondaryConstructors/withoutPrimary.kt"); } + + @TestMetadata("withoutPrimarySimple.kt") + public void testWithoutPrimarySimple() throws Exception { + runTest("compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt"); + } } @TestMetadata("compiler/testData/codegen/box/signatureAnnotations") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index bed52d66f09..866f2b0bff3 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -21347,6 +21347,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes public void testWithoutPrimary() throws Exception { runTest("compiler/testData/codegen/box/secondaryConstructors/withoutPrimary.kt"); } + + @TestMetadata("withoutPrimarySimple.kt") + public void testWithoutPrimarySimple() throws Exception { + runTest("compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt"); + } } @TestMetadata("compiler/testData/codegen/box/signatureAnnotations") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 6cf3f707833..940085c5117 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -21352,6 +21352,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes public void testWithoutPrimary() throws Exception { runTest("compiler/testData/codegen/box/secondaryConstructors/withoutPrimary.kt"); } + + @TestMetadata("withoutPrimarySimple.kt") + public void testWithoutPrimarySimple() throws Exception { + runTest("compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt"); + } } @TestMetadata("compiler/testData/codegen/box/signatureAnnotations") diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java index 81e420cb6be..891cb567c16 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java @@ -19282,6 +19282,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { public void testWithReturnUnit() throws Exception { runTest("compiler/testData/codegen/box/secondaryConstructors/withReturnUnit.kt"); } + + @TestMetadata("withoutPrimarySimple.kt") + public void testWithoutPrimarySimple() throws Exception { + runTest("compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt"); + } } @TestMetadata("compiler/testData/codegen/box/signatureAnnotations") diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 2b2e8300d0e..ba28239adef 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -20327,6 +20327,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { public void testWithReturnUnit() throws Exception { runTest("compiler/testData/codegen/box/secondaryConstructors/withReturnUnit.kt"); } + + @TestMetadata("withoutPrimarySimple.kt") + public void testWithoutPrimarySimple() throws Exception { + runTest("compiler/testData/codegen/box/secondaryConstructors/withoutPrimarySimple.kt"); + } } @TestMetadata("compiler/testData/codegen/box/signatureAnnotations") diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/AssignmentTranslator.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/AssignmentTranslator.java index 6f25e10c25e..0c2764f6101 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/AssignmentTranslator.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/AssignmentTranslator.java @@ -18,10 +18,7 @@ package org.jetbrains.kotlin.js.translate.operation; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.kotlin.descriptors.CallableDescriptor; -import org.jetbrains.kotlin.descriptors.ClassDescriptor; -import org.jetbrains.kotlin.descriptors.DeclarationDescriptor; -import org.jetbrains.kotlin.descriptors.PropertyDescriptor; +import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.js.backend.ast.JsExpression; import org.jetbrains.kotlin.js.translate.context.TranslationContext; import org.jetbrains.kotlin.js.translate.general.AbstractTranslator; @@ -70,7 +67,7 @@ public abstract class AssignmentTranslator extends AbstractTranslator { } protected final AccessTranslator createAccessTranslator(@NotNull KtExpression left, boolean forceOrderOfEvaluation) { - if (isReferenceToBackingFieldFromConstructor(left, context())) { + if (isValProperty(left, context())) { KtSimpleNameExpression simpleName = getSimpleName(left); assert simpleName != null; return BackingFieldAccessTranslator.newInstance(simpleName, context()); @@ -80,40 +77,17 @@ public abstract class AssignmentTranslator extends AbstractTranslator { } } - private static boolean isReferenceToBackingFieldFromConstructor( + private static boolean isValProperty( @NotNull KtExpression expression, @NotNull TranslationContext context ) { - if (expression instanceof KtSimpleNameExpression) { - KtSimpleNameExpression nameExpression = (KtSimpleNameExpression) expression; - DeclarationDescriptor descriptor = getDescriptorForReferenceExpression(context.bindingContext(), nameExpression); - return isReferenceToBackingFieldFromConstructor(descriptor, context); - } - else if (expression instanceof KtDotQualifiedExpression) { - KtDotQualifiedExpression qualifiedExpression = (KtDotQualifiedExpression) expression; - if (qualifiedExpression.getReceiverExpression() instanceof KtThisExpression && - qualifiedExpression.getSelectorExpression() instanceof KtSimpleNameExpression - ) { - KtSimpleNameExpression nameExpression = (KtSimpleNameExpression) qualifiedExpression.getSelectorExpression(); - DeclarationDescriptor descriptor = getDescriptorForReferenceExpression(context.bindingContext(), nameExpression); - return isReferenceToBackingFieldFromConstructor(descriptor, context); - } + KtSimpleNameExpression simpleNameExpression = getSimpleName(expression); + + if (simpleNameExpression != null) { + DeclarationDescriptor descriptor = getDescriptorForReferenceExpression(context.bindingContext(), simpleNameExpression); + return descriptor instanceof PropertyDescriptor && !((PropertyDescriptor) descriptor).isVar(); } + return false; } - - private static boolean isReferenceToBackingFieldFromConstructor( - @Nullable DeclarationDescriptor descriptor, - @NotNull TranslationContext context - ) { - if (!(descriptor instanceof PropertyDescriptor)) return false; - - PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor; - if (!(context.getDeclarationDescriptor() instanceof ClassDescriptor)) return false; - - ClassDescriptor classDescriptor = (ClassDescriptor) context.getDeclarationDescriptor(); - if (classDescriptor != propertyDescriptor.getContainingDeclaration()) return false; - - return !propertyDescriptor.isVar(); - } }