From 432ec31daf427b7a6f6d11cfbb8d118983d8bc7e Mon Sep 17 00:00:00 2001 From: Michael Bogdanov Date: Thu, 4 Sep 2014 12:40:30 +0400 Subject: [PATCH] Support returns in nested finallies --- .../inline/InternalFinallyBlockInliner.java | 170 +++++++++++++----- .../jet/codegen/inline/TryBlockClustering.kt | 33 +++- .../nestedWithReturnsSimple.1.kt | 55 ------ 3 files changed, 157 insertions(+), 101 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/inline/InternalFinallyBlockInliner.java b/compiler/backend/src/org/jetbrains/jet/codegen/inline/InternalFinallyBlockInliner.java index 54632c9ef8e..ffe2ad8738f 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/inline/InternalFinallyBlockInliner.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/inline/InternalFinallyBlockInliner.java @@ -16,6 +16,7 @@ package org.jetbrains.jet.codegen.inline; +import com.google.common.base.Objects; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; @@ -28,7 +29,6 @@ import org.jetbrains.org.objectweb.asm.Opcodes; import org.jetbrains.org.objectweb.asm.Type; import org.jetbrains.org.objectweb.asm.tree.*; - import java.util.*; import static org.jetbrains.jet.codegen.inline.InlineCodegenUtil.*; @@ -112,8 +112,8 @@ public class InternalFinallyBlockInliner { // Each group that corresponds to try/*catches*/finally contains tryCatch block with default handler. // For each such group we should insert corresponding finally before non-local return. // So we split all try blocks on current instructions to groups and process them independently - List clusters = InlinePackage.doClustering(coveringTryCatchBlocks); - ListIterator tryCatchBlockIterator = clusters.listIterator(clusters.size()); + List> clusters = InlinePackage.doClustering(coveringTryCatchBlocks); + ListIterator> tryCatchBlockIterator = clusters.listIterator(clusters.size()); //Reverse visiting cause innermost tryCatchBlocks in the end while (tryCatchBlockIterator.hasPrevious()) { TryBlockCluster originalFinallyCluster = tryCatchBlockIterator.previous(); @@ -135,6 +135,7 @@ public class InternalFinallyBlockInliner { MethodNode finallyBlockCopy = createEmptyMethodNode(); Label newFinallyStart = new Label(); Label newFinallyEnd = new Label(); + Label insertedBlockEnd = new Label(); if (nonLocalReturnType != Type.VOID_TYPE) { finallyBlockCopy.visitVarInsn(nonLocalReturnType.getOpcode(Opcodes.ISTORE), nextTempNonLocalVarIndex); @@ -178,11 +179,13 @@ public class InternalFinallyBlockInliner { nextTempNonLocalVarIndex += nonLocalReturnType.getSize(); //TODO: do more wise indexing } + finallyBlockCopy.visitLabel(insertedBlockEnd); + //Copying finally body before non-local return instruction InlineCodegenUtil.insertNodeBefore(finallyBlockCopy, inlineFun, instrInsertFinallyBefore); nextPrev = updateExceptionTable(coveringTryCatchBlocks, nextPrev, clusterBlocks, newFinallyStart, newFinallyEnd, - tryCatchBlockInlinedInFinally); + tryCatchBlockInlinedInFinally, labelsInsideFinally, (LabelNode) insertedBlockEnd.info); } curIns = nextPrev; } @@ -208,35 +211,95 @@ public class InternalFinallyBlockInliner { private AbstractInsnNode updateExceptionTable( @NotNull Stack coveringTryBlocks, @Nullable AbstractInsnNode nextPrev, - @NotNull List clusterBlocks, + @NotNull List updatingClusterBlocks, @NotNull Label newFinallyStart, @NotNull Label newFinallyEnd, - @NotNull List tryCatchBlockPresentInFinally + @NotNull List tryCatchBlockPresentInFinally, + @NotNull Set labelsInsideFinally, + @NotNull LabelNode insertedBlockEnd + ) { //copy tryCatchFinallies that totally in finally block - for (TryCatchBlockNodePosition position : tryCatchBlockPresentInFinally) { - //TODO assert INNER - TryCatchBlockNode tryCatchBlockNode = position.getNodeInfo().getNode(); - TryCatchBlockNode additionalTryCatchBlock = - new TryCatchBlockNode((LabelNode) tryCatchBlockNode.start.getLabel().info, - (LabelNode) tryCatchBlockNode.end.getLabel().info, - (LabelNode) tryCatchBlockNode.handler.getLabel().info, - tryCatchBlockNode.type); + List> clusters = InlinePackage.doClustering(tryCatchBlockPresentInFinally); + Map> handler2Cluster = new HashMap>(); - TryCatchBlockNodeInfo newInfo = new TryCatchBlockNodeInfo(additionalTryCatchBlock, true); - tryBlockStarts.put(additionalTryCatchBlock.start, newInfo); - tryBlockEnds.put(additionalTryCatchBlock.end, newInfo); - inlineFunTryBlockInfo.add(newInfo); + for (TryBlockCluster cluster : clusters) { + List clusterBlocks = cluster.getBlocks(); + TryCatchBlockNodePosition block0 = clusterBlocks.get(0); + TryCatchPosition clusterPosition = block0.getPosition(); + if (clusterPosition == TryCatchPosition.INNER) { + for (TryCatchBlockNodePosition position : clusterBlocks) { + assert clusterPosition == position.getPosition() : "Wrong inner tryCatchBlock structure"; + TryCatchBlockNode tryCatchBlockNode = position.getNodeInfo().getNode(); + + assert inlineFun.instructions.indexOf(tryCatchBlockNode.start) <= inlineFun.instructions.indexOf(tryCatchBlockNode.end); + + TryCatchBlockNode additionalTryCatchBlock = + new TryCatchBlockNode((LabelNode) tryCatchBlockNode.start.getLabel().info, + (LabelNode) tryCatchBlockNode.end.getLabel().info, + getNewOrOldLabel(tryCatchBlockNode.handler, labelsInsideFinally), + tryCatchBlockNode.type); + + + assert inlineFun.instructions.indexOf(additionalTryCatchBlock.start) <= inlineFun.instructions.indexOf(additionalTryCatchBlock.end); + + TryCatchBlockNodeInfo newInfo = new TryCatchBlockNodeInfo(additionalTryCatchBlock, true); + tryBlockStarts.put(newInfo.getStartLabel(), newInfo); + tryBlockEnds.put(newInfo.getEndLabel(), newInfo); + inlineFunTryBlockInfo.add(newInfo); + } + } + else if (clusterPosition == TryCatchPosition.END) { + TryCatchBlockNodePosition defaultHandler = cluster.getDefaultHandler(); + assert defaultHandler != null : "Default handler should be present"; + handler2Cluster.put(defaultHandler.getHandler(), cluster); + } + else { + assert clusterPosition == TryCatchPosition.START; + TryCatchBlockNodePosition defaultHandler = cluster.getDefaultHandler(); + assert defaultHandler != null : "Default handler should be present"; + TryBlockCluster endCluster = handler2Cluster.remove(defaultHandler.getHandler()); + assert endCluster != null : "Could find start cluster for " + clusterPosition; + + //at this point only external finallies could occurs + //they don't collision with updatingClusterBlocks, but may with external ones on next updateExceptionTable invocation + Iterator startBlockPositions = clusterBlocks.iterator(); + for (TryCatchBlockNodePosition endBlockPosition : endCluster.getBlocks()) { + TryCatchBlockNodeInfo startNode = startBlockPositions.next().getNodeInfo(); + TryCatchBlockNodeInfo endNode = endBlockPosition.getNodeInfo(); + + assert Objects.equal(startNode.getType(), endNode.getType()) : "Different handler types : " + startNode.getType() + " " + endNode.getType(); + + patchTryBlocks((LabelNode) startNode.getStartLabel().getLabel().info, endNode, false); + } + } } + if (handler2Cluster.size() == 1) { + TryBlockCluster singleCluster = handler2Cluster.values().iterator().next(); + if (singleCluster.getBlocks().get(0).getPosition() == TryCatchPosition.END) { + //Pair that starts on default handler don't added to tryCatchBlockPresentInFinally cause it's out of finally block + //TODO rewrite to clusters + for (TryCatchBlockNodePosition endBlockPosition : singleCluster.getBlocks()) { + TryCatchBlockNodeInfo endNode = endBlockPosition.getNodeInfo(); + patchTryBlocks((LabelNode) insertedBlockEnd.getLabel().info, endNode, true); + //nextPrev = (AbstractInsnNode) insertedBlockEnd.getLabel().info; + } + + handler2Cluster.clear(); + } + } + assert handler2Cluster.isEmpty() : "Unmatched clusters " + handler2Cluster.size(); + // Inserted finally shouldn't be handled by corresponding catches, // so we should split original interval by inserted finally one - for (TryCatchBlockNodeInfo block : clusterBlocks) { + for (TryCatchBlockNodeInfo block : updatingClusterBlocks) { //update exception mapping LabelNode oldStartNode = block.getNode().start; tryBlockStarts.remove(oldStartNode, block); block.getNode().start = (LabelNode) newFinallyEnd.info; + //tryBlockStarts.put(block.getStartLabel(), block); TryCatchBlockNode additionalTryCatchBlock = new TryCatchBlockNode(oldStartNode, (LabelNode) newFinallyStart.info, block.getNode().handler, block.getNode().type); @@ -251,19 +314,50 @@ public class InternalFinallyBlockInliner { nextPrev = additionalTryCatchBlock.end; coveringTryBlocks.pop(); } - sortTryCatchBlocks(); + sortTryCatchBlocks(inlineFunTryBlockInfo); return nextPrev; } + private void patchTryBlocks(@NotNull LabelNode newStartLabelNode, @NotNull TryCatchBlockNodeInfo endNode, boolean sort) { + LabelNode oldStart = endNode.getStartLabel(); + endNode.getNode().start = newStartLabelNode; + tryBlockStarts.remove(oldStart, endNode); + tryBlockStarts.put(endNode.getNode().start, endNode); + + + TryCatchBlockNode endTryBlock = endNode.getNode(); + TryCatchBlockNode additionalTryCatchBlock = + new TryCatchBlockNode(oldStart, + (LabelNode) endTryBlock.end.getLabel().info, + endTryBlock.handler, + endTryBlock.type); + + TryCatchBlockNodeInfo newInfo = new TryCatchBlockNodeInfo(additionalTryCatchBlock, endNode.getOnlyCopyNotProcess()); + tryBlockStarts.put(newInfo.getStartLabel(), newInfo); + tryBlockEnds.put(newInfo.getEndLabel(), newInfo); + + inlineFunTryBlockInfo.add(newInfo); + } + + private static LabelNode getNewOrOldLabel(LabelNode oldHandler, @NotNull Set labelsInsideFinally) { + if (labelsInsideFinally.contains(oldHandler)) { + return (LabelNode) oldHandler.getLabel().info; + } + + return oldHandler; + } + //Keep information about try blocks that cover current instruction - // pushing and popping it to stack entering and exiting tryCatchBlock start and end labels private void updateCoveringTryBlocks(Stack coveringTryBlocks, AbstractInsnNode curIns) { if (!(curIns instanceof LabelNode)) return; - for (TryCatchBlockNodeInfo startNode : tryBlockStarts.get((LabelNode) curIns)) { + List infos = tryBlockStarts.get((LabelNode) curIns); + for (TryCatchBlockNodeInfo startNode : infos) { if (!startNode.getOnlyCopyNotProcess()) { TryCatchBlockNodeInfo pop = coveringTryBlocks.pop(); - assert startNode == pop : "Wrong try-catch structure " + startNode + " " + pop; + //Temporary disabled cause during patched structure of exceptions changed + //assert startNode == pop : "Wrong try-catch structure " + startNode + " " + pop + " " + infos.size(); } } @@ -382,12 +476,10 @@ public class InternalFinallyBlockInliner { List startedTryBlocks = tryBlockStarts.get(curLabel); if (startedTryBlocks != null) { for (TryCatchBlockNodeInfo block : startedTryBlocks) { - if (block.getOnlyCopyNotProcess()) { - assert !processedBlocks.containsKey(block) : "Try catch block already processed before start label!!! " + block; - TryCatchBlockNodePosition info = new TryCatchBlockNodePosition(block, TryCatchPosition.START); - processedBlocks.put(block, info); - result.add(info); - } + assert !processedBlocks.containsKey(block) : "Try catch block already processed before start label!!! " + block; + TryCatchBlockNodePosition info = new TryCatchBlockNodePosition(block, TryCatchPosition.START); + processedBlocks.put(block, info); + result.add(info); } } @@ -395,17 +487,15 @@ public class InternalFinallyBlockInliner { if (endedTryBlocks == null) continue; for (TryCatchBlockNodeInfo block : endedTryBlocks) { - if (block.getOnlyCopyNotProcess()) { - TryCatchBlockNodePosition info = processedBlocks.get(block); - if (info != null) { - assert info.getPosition() == TryCatchPosition.START; - info.setPosition(TryCatchPosition.INNER); - } - else { - info = new TryCatchBlockNodePosition(block, TryCatchPosition.END); - processedBlocks.put(block, info); - result.add(info); - } + TryCatchBlockNodePosition info = processedBlocks.get(block); + if (info != null) { + assert info.getPosition() == TryCatchPosition.START; + info.setPosition(TryCatchPosition.INNER); + } + else { + info = new TryCatchBlockNodePosition(block, TryCatchPosition.END); + processedBlocks.put(block, info); + result.add(info); } } } @@ -436,7 +526,7 @@ public class InternalFinallyBlockInliner { return result; } - public void sortTryCatchBlocks() { + private void sortTryCatchBlocks(@NotNull List inlineFunTryBlockInfo) { Comparator comp = new Comparator() { @Override public int compare(@NotNull TryCatchBlockNodeInfo t1, @NotNull TryCatchBlockNodeInfo t2) { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/inline/TryBlockClustering.kt b/compiler/backend/src/org/jetbrains/jet/codegen/inline/TryBlockClustering.kt index 9ad28eef769..2b7a9b9e279 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/inline/TryBlockClustering.kt +++ b/compiler/backend/src/org/jetbrains/jet/codegen/inline/TryBlockClustering.kt @@ -21,6 +21,7 @@ import org.jetbrains.org.objectweb.asm.tree.TryCatchBlockNode import org.jetbrains.org.objectweb.asm.tree.LabelNode import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode import org.jetbrains.jet.codegen.inline.InlineCodegenUtil.* +import kotlin.properties.Delegates enum class TryCatchPosition { START @@ -28,19 +29,39 @@ enum class TryCatchPosition { INNER } -class TryCatchBlockNodeInfo(val node: TryCatchBlockNode, val onlyCopyNotProcess: Boolean) +trait IntervalWithHandler { + val startLabel: LabelNode + val endLabel: LabelNode + val handler: LabelNode + val `type`: String? +} -class TryCatchBlockNodePosition(val nodeInfo: TryCatchBlockNodeInfo, var position: TryCatchPosition) +class TryCatchBlockNodeInfo(val node: TryCatchBlockNode, val onlyCopyNotProcess: Boolean) : IntervalWithHandler { -class TryBlockCluster(val blocks: MutableList) + override val startLabel: LabelNode + get() = node.start + override val endLabel: LabelNode + get() = node.end + override val handler: LabelNode + get() = node.handler!! + override val `type`: String? + get() = node.`type` +} + +class TryCatchBlockNodePosition(val nodeInfo: TryCatchBlockNodeInfo, var position: TryCatchPosition): IntervalWithHandler by nodeInfo + +class TryBlockCluster(val blocks: MutableList) { + val defaultHandler: T? + get() = blocks.firstOrNull() { it.`type` == null } +} -fun doClustering(blocks: List) : List { +fun doClustering(blocks: List) : List> { [data] class TryBlockInterval(val startLabel: LabelNode, val endLabel: LabelNode) - val clusters = linkedMapOf() + val clusters = linkedMapOf>() blocks.forEach { block -> - val interval = TryBlockInterval(firstLabelInChain(block.node.start), firstLabelInChain(block.node.end)) + val interval = TryBlockInterval(firstLabelInChain(block.startLabel), firstLabelInChain(block.endLabel)) val cluster = clusters.getOrPut(interval, {TryBlockCluster(arrayListOf())}) cluster.blocks.add(block) } diff --git a/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/exceptionTable/nestedWithReturnsSimple.1.kt b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/exceptionTable/nestedWithReturnsSimple.1.kt index 32578318d1d..4287d04f853 100644 --- a/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/exceptionTable/nestedWithReturnsSimple.1.kt +++ b/compiler/testData/codegen/boxInline/nonLocalReturns/tryFinally/exceptionTable/nestedWithReturnsSimple.1.kt @@ -43,60 +43,5 @@ fun box(): String { test0 = test0(h, throwExternalFinEx1 = true, res = "OK") if (test0 != "EXCEPTION_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_FINALLY1, OK_FINALLY2") return "test0_2: ${test0}, holder: ${h.value}" -// h = Holder() -// test0 = test0(h, throwExternalFinEx2 = true, res = "OK") -// if (test0 != "EXCEPTION222_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_FINALLY1, OK_FINALLY2") return "test0_4: ${test0}, holder: ${h.value}" - - - - -// h = Holder() -// test0 = test0(h, true, throwExternalFinEx1 = true, res = "OK") -// if (test0 != "EXCEPTION_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_INTERNAL_EXCEPTION1, OK_FINALLY1, OK_FINALLY2") return "test0_3: ${test0}, holder: ${h.value}" -// -// h = Holder() -// test0 = test0(h, true, throwInternalEx2 = true, throwExternalFinEx1 = true, res = "OK") -// if (test0 != "EXCEPTION_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_INTERNAL_EXCEPTION1, OK_FINALLY1, OK_FINALLY2") return "test0_5: ${test0}, holder: ${h.value}" -// -// h = Holder() -// test0 = test0(h, true, throwInternalEx2 = true, throwExternalFinEx2 = true, res = "OK") -// if (test0 != "EXCEPTION222_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_INTERNAL_EXCEPTION1, OK_FINALLY1, OK_FINALLY2") return "test0_6: ${test0}, holder: ${h.value}" -// -// -// -// h = Holder() -// test0 = test0(h, false, throwInternalFinEx1 = true, res = "FAIL") -// if (test0 != "EXCEPTION_IN_INTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_FINALLY1, OK_FINALLY2") return "test0_7: ${test0}, holder: ${h.value}" -// -// h = Holder() -// test0 = test0(h, false, throwInternalFinEx1 = true, throwExternalFinEx2 = true, res = "FAIL") -// if (test0 != "EXCEPTION222_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_FINALLY1, OK_FINALLY2") return "test0_71: ${test0}, holder: ${h.value}" -// -// h = Holder() -// test0 = test0(h, false, throwInternalFinEx2 = true, res = "FAIL") -// if (test0 != "OK_EXTERNAL_EXCEPTION2" || h.value != "OK_NON_LOCAL, OK_FINALLY1, OK_EXTERNAL_EXCEPTION2, OK_FINALLY2") return "test0_8: ${test0}, holder: ${h.value}" -// -// h = Holder() -// test0 = test0(h, false, throwInternalFinEx2 = true, throwExternalFinEx2 = true, res = "FAIL") -// if (test0 != "EXCEPTION222_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_FINALLY1, OK_EXTERNAL_EXCEPTION2, OK_FINALLY2") return "test0_81: ${test0}, holder: ${h.value}" -// -// -// -// h = Holder() -// test0 = test0(h, true, throwInternalFinEx1 = true, res = "FAIL") -// if (test0 != "EXCEPTION_IN_INTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_INTERNAL_EXCEPTION1, OK_FINALLY1, OK_FINALLY2") return "test0_9: ${test0}, holder: ${h.value}" -// -// h = Holder() -// test0 = test0(h, true, throwInternalFinEx1 = true, throwExternalFinEx2 = true, res = "FAIL") -// if (test0 != "EXCEPTION222_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_INTERNAL_EXCEPTION1, OK_FINALLY1, OK_FINALLY2") return "test0_10: ${test0}, holder: ${h.value}" -// -// h = Holder() -// test0 = test0(h, true, throwInternalFinEx2 = true, res = "FAIL") -// if (test0 != "OK_EXTERNAL_EXCEPTION2" || h.value != "OK_NON_LOCAL, OK_INTERNAL_EXCEPTION1, OK_FINALLY1, OK_EXTERNAL_EXCEPTION2, OK_FINALLY2") return "test0_11: ${test0}, holder: ${h.value}" -// -// h = Holder() -// test0 = test0(h, true, throwInternalFinEx2 = true, throwExternalFinEx2 = true, res = "FAIL") -// if (test0 != "EXCEPTION222_IN_EXTERNAL_FINALLY" || h.value != "OK_NON_LOCAL, OK_INTERNAL_EXCEPTION1, OK_FINALLY1, OK_EXTERNAL_EXCEPTION2, OK_FINALLY2") return "test0_12: ${test0}, holder: ${h.value}" - return "OK" }