From c1cc722ac4a5511a45e4ad7d83cb8b69cca389f2 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 9 Aug 2018 16:34:32 +0700 Subject: [PATCH] Turn off incorrect switch-optimization for when by enums #KT-24708 Fixed --- .../binding/CodegenAnnotatingVisitor.java | 17 +++++++---- .../org/jetbrains/kotlin/cfg/WhenChecker.kt | 9 ++++-- .../enumOptimization/differentEnumClasses.kt | 27 +++++++++++++++++ .../enumOptimization/differentEnumClasses2.kt | 16 ++++++++++ .../differentEnumClasses.kt | 30 +++++++++++++++++++ .../differentEnumClasses2.kt | 19 ++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 10 +++++++ .../codegen/BytecodeTextTestGenerated.java | 10 +++++++ .../LightAnalysisModeTestGenerated.java | 10 +++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 10 +++++++ .../IrJsCodegenBoxTestGenerated.java | 10 +++++++ .../semantics/JsCodegenBoxTestGenerated.java | 10 +++++++ .../js/translate/expression/WhenTranslator.kt | 14 ++++++--- 13 files changed, 181 insertions(+), 11 deletions(-) create mode 100644 compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt create mode 100644 compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt create mode 100644 compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses.kt create mode 100644 compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses2.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/binding/CodegenAnnotatingVisitor.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/binding/CodegenAnnotatingVisitor.java index d18b1414507..ffbd632763f 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/binding/CodegenAnnotatingVisitor.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/binding/CodegenAnnotatingVisitor.java @@ -959,11 +959,18 @@ class CodegenAnnotatingVisitor extends KtVisitorVoid { } private boolean isWhenWithEnums(@NotNull KtWhenExpression expression) { - return WhenChecker.isWhenByEnum(expression, bindingContext) && - switchCodegenProvider.checkAllItemsAreConstantsSatisfying( - expression, - constant -> constant instanceof EnumValue || constant instanceof NullValue - ); + ClassId enumClassId = WhenChecker.getClassIdForEnumSubject(expression, bindingContext); + if (enumClassId == null) return false; + + return switchCodegenProvider.checkAllItemsAreConstantsSatisfying( + expression, + constant -> isEnumEntryOrNull(enumClassId, constant) + ); + } + + private static boolean isEnumEntryOrNull(ClassId enumClassId, ConstantValue constant) { + return (constant instanceof EnumValue && ((EnumValue) constant).getEnumClassId().equals(enumClassId)) || + constant instanceof NullValue; } @NotNull diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt index bcb80ed4b1c..d6bed65a6ec 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/WhenChecker.kt @@ -35,6 +35,7 @@ import org.jetbrains.kotlin.resolve.DescriptorUtils.isEnumClass import org.jetbrains.kotlin.resolve.DescriptorUtils.isEnumEntry import org.jetbrains.kotlin.resolve.constants.CompileTimeConstant import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator +import org.jetbrains.kotlin.resolve.descriptorUtil.classId import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeUtils import java.util.* @@ -265,8 +266,12 @@ object WhenChecker { ) @JvmStatic - fun isWhenByEnum(expression: KtWhenExpression, context: BindingContext) = - getClassDescriptorOfTypeIfEnum(whenSubjectType(expression, context)) != null + fun getClassIdForEnumSubject(expression: KtWhenExpression, context: BindingContext) = + getClassIdForTypeIfEnum(whenSubjectType(expression, context)) + + @JvmStatic + fun getClassIdForTypeIfEnum(type: KotlinType?) = + getClassDescriptorOfTypeIfEnum(type)?.classId @JvmStatic fun getClassDescriptorOfTypeIfEnum(type: KotlinType?): ClassDescriptor? { diff --git a/compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt b/compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt new file mode 100644 index 00000000000..b0aedc491fb --- /dev/null +++ b/compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt @@ -0,0 +1,27 @@ +enum class A { + O, K +} + +enum class B { + O, K +} + +fun box(): String { + val a = A.O + val r1 = when (a) { + A.O -> "O" + A.K -> "K" + B.O -> "fail 1" + B.K -> "fail 2" + } + + val b = B.K + val r2 = when (b) { + A.O -> "fail 3" + A.K -> "fail 4" + B.O -> "O" + B.K -> "K" + } + + return r1 + r2 +} diff --git a/compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt b/compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt new file mode 100644 index 00000000000..65472cb7ad1 --- /dev/null +++ b/compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt @@ -0,0 +1,16 @@ +enum class A { + OK +} + +enum class B { + FAIL +} + +fun f() = A.OK + +fun box(): String { + return when (f()) { + B.FAIL -> "fail" + A.OK -> "OK" + } +} diff --git a/compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses.kt b/compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses.kt new file mode 100644 index 00000000000..0580537140f --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses.kt @@ -0,0 +1,30 @@ +enum class A { + O, K +} + +enum class B { + O, K +} + +fun box(): String { + val a = A.O + val r1 = when (a) { + A.O -> "O" + A.K -> "K" + B.O -> "fail 1" + B.K -> "fail 2" + } + + val b = B.K + val r2 = when (b) { + A.O -> "fail 3" + A.K -> "fail 4" + B.O -> "O" + B.K -> "K" + } + + return r1 + r2 +} + +// 0 TABLESWITCH +// 0 LOOKUPSWITCH diff --git a/compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses2.kt b/compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses2.kt new file mode 100644 index 00000000000..39d1d29a010 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses2.kt @@ -0,0 +1,19 @@ +enum class A { + OK +} + +enum class B { + FAIL +} + +fun f() = A.OK + +fun box(): String { + return when (f()) { + B.FAIL -> "fail" + A.OK -> "OK" + } +} + +// 0 TABLESWITCH +// 0 LOOKUPSWITCH diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 5928e6ebe60..869ba630316 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -22739,6 +22739,16 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/when/enumOptimization/bigEnum.kt"); } + @TestMetadata("differentEnumClasses.kt") + public void testDifferentEnumClasses() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt"); + } + + @TestMetadata("differentEnumClasses2.kt") + public void testDifferentEnumClasses2() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt"); + } + @TestMetadata("duplicatingItems.kt") public void testDuplicatingItems() throws Exception { runTest("compiler/testData/codegen/box/when/enumOptimization/duplicatingItems.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 1fbd7c0f1b0..3347b3690e8 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -2962,6 +2962,16 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/whenEnumOptimization/bigEnum.kt"); } + @TestMetadata("differentEnumClasses.kt") + public void testDifferentEnumClasses() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses.kt"); + } + + @TestMetadata("differentEnumClasses2.kt") + public void testDifferentEnumClasses2() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/whenEnumOptimization/differentEnumClasses2.kt"); + } + @TestMetadata("duplicatingItems.kt") public void testDuplicatingItems() throws Exception { runTest("compiler/testData/codegen/bytecodeText/whenEnumOptimization/duplicatingItems.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index d7b924da613..9a8d02abe52 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -22739,6 +22739,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/when/enumOptimization/bigEnum.kt"); } + @TestMetadata("differentEnumClasses.kt") + public void testDifferentEnumClasses() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt"); + } + + @TestMetadata("differentEnumClasses2.kt") + public void testDifferentEnumClasses2() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt"); + } + @TestMetadata("duplicatingItems.kt") public void testDuplicatingItems() throws Exception { runTest("compiler/testData/codegen/box/when/enumOptimization/duplicatingItems.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index f1aa441d9a5..958f744b261 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -22739,6 +22739,16 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/when/enumOptimization/bigEnum.kt"); } + @TestMetadata("differentEnumClasses.kt") + public void testDifferentEnumClasses() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt"); + } + + @TestMetadata("differentEnumClasses2.kt") + public void testDifferentEnumClasses2() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt"); + } + @TestMetadata("duplicatingItems.kt") public void testDuplicatingItems() throws Exception { runTest("compiler/testData/codegen/box/when/enumOptimization/duplicatingItems.kt"); 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 6eafa5c208d..37664095d4e 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 @@ -20554,6 +20554,16 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/when/enumOptimization/bigEnum.kt"); } + @TestMetadata("differentEnumClasses.kt") + public void testDifferentEnumClasses() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt"); + } + + @TestMetadata("differentEnumClasses2.kt") + public void testDifferentEnumClasses2() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt"); + } + @TestMetadata("duplicatingItems.kt") public void testDuplicatingItems() throws Exception { runTest("compiler/testData/codegen/box/when/enumOptimization/duplicatingItems.kt"); 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 83568ec1280..df4025f08b8 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 @@ -21619,6 +21619,16 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/when/enumOptimization/bigEnum.kt"); } + @TestMetadata("differentEnumClasses.kt") + public void testDifferentEnumClasses() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses.kt"); + } + + @TestMetadata("differentEnumClasses2.kt") + public void testDifferentEnumClasses2() throws Exception { + runTest("compiler/testData/codegen/box/when/enumOptimization/differentEnumClasses2.kt"); + } + @TestMetadata("duplicatingItems.kt") public void testDuplicatingItems() throws Exception { runTest("compiler/testData/codegen/box/when/enumOptimization/duplicatingItems.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/WhenTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/WhenTranslator.kt index 6dadc750488..e6a715c8727 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/WhenTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/WhenTranslator.kt @@ -7,6 +7,7 @@ package org.jetbrains.kotlin.js.translate.expression import org.jetbrains.kotlin.backend.common.CodegenUtil import org.jetbrains.kotlin.builtins.KotlinBuiltIns +import org.jetbrains.kotlin.cfg.WhenChecker import org.jetbrains.kotlin.config.languageVersionSettings import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor @@ -208,11 +209,16 @@ private constructor(private val whenExpression: KtWhenExpression, context: Trans entries: List, expectedType: KotlinType ): Pair, Int> { + val classId = WhenChecker.getClassIdForTypeIfEnum(expectedType) return collectConstantEntries( - fromIndex, entries, - { (it.toConstantValue(expectedType) as? EnumValue)?.enumEntryName?.identifier }, - { uniqueEnumNames.add(it) }, - { JsStringLiteral(it) } + fromIndex, entries, + { + (it.toConstantValue(expectedType) as? EnumValue) + ?.takeIf { enumEntry -> enumEntry.enumClassId == classId } + ?.enumEntryName?.identifier + }, + { uniqueEnumNames.add(it) }, + { JsStringLiteral(it) } ) }