From fe571a7dfa886ba974188bf16f47fc9eabe83da7 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Tue, 25 Apr 2017 16:43:21 +0300 Subject: [PATCH] Normalize local returns before other inlining transformations Local returns normalization can generate POP instructions. These POP instructions can drop functional parameters, as in KT-17590, and should be processed in markPlacesForInlineAndRemoveInlinable just as other POP instructions. KT-17590 conditional return in inline function parameter argument causes compilation exception --- .../kotlin/codegen/inline/MethodInliner.java | 91 ++++++++++--------- .../codegen/box/controlStructures/kt17590.kt | 7 ++ .../box/controlStructures/kt17590_long.kt | 10 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 12 +++ .../codegen/BlackBoxCodegenTestGenerated.java | 12 +++ .../LightAnalysisModeTestGenerated.java | 12 +++ .../semantics/JsCodegenBoxTestGenerated.java | 12 +++ 7 files changed, 113 insertions(+), 43 deletions(-) create mode 100644 compiler/testData/codegen/box/controlStructures/kt17590.kt create mode 100644 compiler/testData/codegen/box/controlStructures/kt17590_long.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java index cfba90fc1a0..d7d477fef1a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.java @@ -421,8 +421,9 @@ public class MethodInliner { ) { node = prepareNode(node, finallyDeepShift); - Frame[] sources = analyzeMethodNodeBeforeInline(node); - LocalReturnsNormalizer localReturnsNormalizer = LocalReturnsNormalizer.createFor(node, labelOwner, sources); + normalizeLocalReturns(node, labelOwner); + + Frame[] sources = analyzeMethodNodeWithoutMandatoryTransformations(node); Set toDelete = SmartSet.create(); InsnList instructions = node.instructions; @@ -551,11 +552,39 @@ public class MethodInliner { } } - localReturnsNormalizer.transform(node); - return node; } + private void normalizeLocalReturns(@NotNull MethodNode node, @NotNull LabelOwner labelOwner) { + Frame[] frames = analyzeMethodNodeBeforeInline(node); + + LocalReturnsNormalizer localReturnsNormalizer = new LocalReturnsNormalizer(); + + AbstractInsnNode[] instructions = node.instructions.toArray(); + + for (int i = 0; i < instructions.length; ++i) { + Frame frame = frames[i]; + // Don't care about dead code, it will be eliminated + if (frame == null) continue; + + AbstractInsnNode insnNode = instructions[i]; + if (!InlineCodegenUtil.isReturnOpcode(insnNode.getOpcode())) continue; + + AbstractInsnNode insertBeforeInsn = insnNode; + + // TODO extract isLocalReturn / isNonLocalReturn, see processReturns + String labelName = getMarkedReturnLabelOrNull(insnNode); + if (labelName != null) { + if (!labelOwner.isMyLabel(labelName)) continue; + insertBeforeInsn = insnNode.getPrevious(); + } + + localReturnsNormalizer.addLocalReturnToTransform(insnNode, insertBeforeInsn, frame); + } + + localReturnsNormalizer.transform(node); + } + private boolean isAnonymousClassThatMustBeRegenerated(@Nullable Type type) { if (type == null || type.getSort() != Type.OBJECT) return false; AnonymousObjectTransformationInfo info = inliningContext.findAnonymousObjectTransformationInfo(type.getInternalName()); @@ -571,6 +600,10 @@ public class MethodInliner { throw wrapException(e, node, "couldn't inline method call"); } + return analyzeMethodNodeWithoutMandatoryTransformations(node); + } + + private static Frame[] analyzeMethodNodeWithoutMandatoryTransformations(@NotNull MethodNode node) { Analyzer analyzer = new Analyzer(new SourceInterpreter()) { @NotNull @Override @@ -590,7 +623,7 @@ public class MethodInliner { return analyzer.analyze("fake", node); } catch (AnalyzerException e) { - throw wrapException(e, node, "couldn't inline method call"); + throw new RuntimeException(e); } } @@ -872,7 +905,7 @@ public class MethodInliner { private final List localReturns = new SmartList(); - private boolean needsReturnVariable = false; + private int returnVariableSize = 0; private int returnOpcode = -1; private void addLocalReturnToTransform( @@ -887,55 +920,27 @@ public class MethodInliner { localReturns.add(new LocalReturn(returnInsn, insertBeforeInsn, sourceValueFrame)); - if (returnInsn.getOpcode() != Opcodes.RETURN && sourceValueFrame.getStackSize() > 1) { - needsReturnVariable = true; + if (returnInsn.getOpcode() != Opcodes.RETURN) { + if (returnInsn.getOpcode() == Opcodes.LRETURN || returnInsn.getOpcode() == Opcodes.DRETURN) { + returnVariableSize = 2; + } + else { + returnVariableSize = 1; + } } } public void transform(@NotNull MethodNode methodNode) { int returnVariableIndex = -1; - if (needsReturnVariable) { + if (returnVariableSize > 0) { returnVariableIndex = methodNode.maxLocals; - methodNode.maxLocals++; + methodNode.maxLocals += returnVariableSize; } for (LocalReturn localReturn : localReturns) { localReturn.transform(methodNode.instructions, returnVariableIndex); } } - - @NotNull - public static LocalReturnsNormalizer createFor( - @NotNull MethodNode methodNode, - @NotNull LabelOwner owner, - @NotNull Frame[] frames - ) { - LocalReturnsNormalizer result = new LocalReturnsNormalizer(); - - AbstractInsnNode[] instructions = methodNode.instructions.toArray(); - - for (int i = 0; i < instructions.length; ++i) { - Frame frame = frames[i]; - // Don't care about dead code, it will be eliminated - if (frame == null) continue; - - AbstractInsnNode insnNode = instructions[i]; - if (!InlineCodegenUtil.isReturnOpcode(insnNode.getOpcode())) continue; - - AbstractInsnNode insertBeforeInsn = insnNode; - - // TODO extract isLocalReturn / isNonLocalReturn, see processReturns - String labelName = getMarkedReturnLabelOrNull(insnNode); - if (labelName != null) { - if (!owner.isMyLabel(labelName)) continue; - insertBeforeInsn = insnNode.getPrevious(); - } - - result.addLocalReturnToTransform(insnNode, insertBeforeInsn, frame); - } - - return result; - } } @NotNull diff --git a/compiler/testData/codegen/box/controlStructures/kt17590.kt b/compiler/testData/codegen/box/controlStructures/kt17590.kt new file mode 100644 index 00000000000..15dba9e8546 --- /dev/null +++ b/compiler/testData/codegen/box/controlStructures/kt17590.kt @@ -0,0 +1,7 @@ +fun zap(s: String): String? = s + +inline fun tryZap(s: String, fn: (String) -> String): String { + return fn(zap(s) ?: return "null") +} + +fun box() = tryZap("OK") { it } \ No newline at end of file diff --git a/compiler/testData/codegen/box/controlStructures/kt17590_long.kt b/compiler/testData/codegen/box/controlStructures/kt17590_long.kt new file mode 100644 index 00000000000..521c7fd61cf --- /dev/null +++ b/compiler/testData/codegen/box/controlStructures/kt17590_long.kt @@ -0,0 +1,10 @@ +fun foo(x: Any?, y: Any?) = 0L + +inline fun test(value: Any?): Long { + return foo(null, value ?: return 1L) +} + +fun box(): String { + val t = test(null) + return if (t == 1L) "OK" else "fail: t=$t" +} \ No newline at end of file diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index dbe87dc1f27..10cdd05c76f 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -4364,6 +4364,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } + @TestMetadata("kt17590.kt") + public void testKt17590() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17590.kt"); + doTest(fileName); + } + + @TestMetadata("kt17590_long.kt") + public void testKt17590_long() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17590_long.kt"); + doTest(fileName); + } + @TestMetadata("kt1899.kt") public void testKt1899() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt1899.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index a4dceae7f2e..b1fe20a4424 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -4364,6 +4364,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("kt17590.kt") + public void testKt17590() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17590.kt"); + doTest(fileName); + } + + @TestMetadata("kt17590_long.kt") + public void testKt17590_long() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17590_long.kt"); + doTest(fileName); + } + @TestMetadata("kt1899.kt") public void testKt1899() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt1899.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 5afd28373e9..1caaf7c68bc 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -4364,6 +4364,18 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes doTest(fileName); } + @TestMetadata("kt17590.kt") + public void testKt17590() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17590.kt"); + doTest(fileName); + } + + @TestMetadata("kt17590_long.kt") + public void testKt17590_long() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17590_long.kt"); + doTest(fileName); + } + @TestMetadata("kt1899.kt") public void testKt1899() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt1899.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 9aafb072d5f..5ce0915f7f0 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 @@ -5037,6 +5037,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { doTest(fileName); } + @TestMetadata("kt17590.kt") + public void testKt17590() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17590.kt"); + doTest(fileName); + } + + @TestMetadata("kt17590_long.kt") + public void testKt17590_long() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt17590_long.kt"); + doTest(fileName); + } + @TestMetadata("kt1899.kt") public void testKt1899() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/kt1899.kt");