From 1b04870fa4b9cf87fb672e849385ca37139a87b3 Mon Sep 17 00:00:00 2001 From: Alex Tkachman Date: Wed, 16 Nov 2011 14:26:16 +0200 Subject: [PATCH] proper try/catch/finally --- .../jet/codegen/ClassBuilderFactory.java | 13 +++- .../jet/codegen/ExpressionCodegen.java | 77 +++++++++++++------ .../jet/codegen/FunctionCodegen.java | 6 +- .../testData/codegen/regressions/kt496.jet | 52 +++++++++++++ .../testData/codegen/regressions/kt500.jet | 30 ++++++++ .../testData/codegen/regressions/kt501.jet | 24 ++++++ .../jetbrains/jet/codegen/ClassGenTest.java | 14 ++++ 7 files changed, 188 insertions(+), 28 deletions(-) create mode 100644 compiler/testData/codegen/regressions/kt496.jet create mode 100644 compiler/testData/codegen/regressions/kt500.jet create mode 100644 compiler/testData/codegen/regressions/kt501.jet diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ClassBuilderFactory.java b/compiler/backend/src/org/jetbrains/jet/codegen/ClassBuilderFactory.java index f5bf4be4dec..ef3d1e4cfe0 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ClassBuilderFactory.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ClassBuilderFactory.java @@ -39,7 +39,18 @@ public interface ClassBuilderFactory { ClassBuilderFactory BINARIES = new ClassBuilderFactory() { @Override public ClassBuilder newClassBuilder() { - return new ClassBuilder.Concrete(new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS)); + return new ClassBuilder.Concrete(new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS){ + @Override + protected String getCommonSuperClass(String type1, String type2) { + try { + return super.getCommonSuperClass(type1, type2); + } + catch (Throwable t) { + // @todo we might need at some point do more sofisticated handling + return "java/lang/Object"; + } + } + }); } @Override diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java index 7df7980364e..2f693489f49 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java @@ -64,6 +64,8 @@ public class ExpressionCodegen extends JetVisitor { private final CodegenContext context; private final IntrinsicMethods intrinsics; + private final ArrayList stackOfFinallyBlocks = new ArrayList(); + public ExpressionCodegen(MethodVisitor v, FrameMap myMap, Type returnType, @@ -712,11 +714,21 @@ public class ExpressionCodegen extends JetVisitor { } } + private void doFinallyOnReturnOrThrow() { + if(stackOfFinallyBlocks.size() > 0) { + JetTryExpression jetTryExpression = stackOfFinallyBlocks.remove(stackOfFinallyBlocks.size()-1); + gen(jetTryExpression.getFinallyBlock().getFinalExpression(), Type.VOID_TYPE); + doFinallyOnReturnOrThrow(); + stackOfFinallyBlocks.add(jetTryExpression); + } + } + @Override public StackValue visitReturnExpression(JetReturnExpression expression, StackValue receiver) { final JetExpression returnedExpression = expression.getReturnedExpression(); if (returnedExpression != null) { gen(returnedExpression, returnType); + doFinallyOnReturnOrThrow(); v.areturn(returnType); } else { @@ -1857,16 +1869,9 @@ public class ExpressionCodegen extends JetVisitor { DeclarationDescriptor cls = op.getContainingDeclaration(); if (isNumberPrimitive(cls) && (op.getName().equals("inc") || op.getName().equals("dec"))) { receiver.put(receiver.type, v); - //noinspection ConstantConditions - if (bindingContext.get(BindingContext.STATEMENT, expression)) { - generateIncrement(op, asmType, expression.getBaseExpression(), receiver); - return StackValue.none(); - } - else { - gen(expression.getBaseExpression(), asmType); // old value - generateIncrement(op, asmType, expression.getBaseExpression(), receiver); // increment in-place - return StackValue.onStack(asmType); // old value - } + gen(expression.getBaseExpression(), asmType); // old value + generateIncrement(op, asmType, expression.getBaseExpression(), receiver); // increment in-place + return StackValue.onStack(asmType); // old value } } throw new UnsupportedOperationException("Don't know how to generate this prefix expression"); @@ -2164,6 +2169,7 @@ public class ExpressionCodegen extends JetVisitor { @Override public StackValue visitThrowExpression(JetThrowExpression expression, StackValue receiver) { gen(expression.getThrownExpression(), JetTypeMapper.TYPE_OBJECT); + doFinallyOnReturnOrThrow(); v.athrow(); return StackValue.none(); } @@ -2184,14 +2190,31 @@ public class ExpressionCodegen extends JetVisitor { @Override public StackValue visitTryExpression(JetTryExpression expression, StackValue receiver) { + /* +The "returned" value of try expression with no finally is either the last expression in the try block or the last expression in the catch block +(or blocks). + +If finally block is present, its last expression is the value of try expression. + */ + JetFinallySection finallyBlock = expression.getFinallyBlock(); + if(finallyBlock != null) { + stackOfFinallyBlocks.add(expression); + } + + JetType jetType = bindingContext.get(BindingContext.EXPRESSION_TYPE, expression); + Type expectedAsmType = typeMapper.mapType(jetType); + Label tryStart = new Label(); v.mark(tryStart); - gen(expression.getTryBlock(), Type.VOID_TYPE); + v.nop(); // prevent verify error on empty try + if(finallyBlock == null) + gen(expression.getTryBlock(), expectedAsmType); + else + gen(expression.getTryBlock(), Type.VOID_TYPE); Label tryEnd = new Label(); v.mark(tryEnd); - JetFinallySection finallyBlock = expression.getFinallyBlock(); if (finallyBlock != null) { - gen(finallyBlock.getFinalExpression(), Type.VOID_TYPE); + gen(finallyBlock.getFinalExpression(), expectedAsmType); } Label end = new Label(); v.goTo(end); // TODO don't generate goto if there's no code following try/catch @@ -2206,31 +2229,36 @@ public class ExpressionCodegen extends JetVisitor { int index = lookupLocal(descriptor); v.store(index, descriptorType); - gen(clause.getCatchBody(), Type.VOID_TYPE); - v.goTo(end); // TODO don't generate goto if there's no code following try/catch + gen(clause.getCatchBody(), finallyBlock != null ? Type.VOID_TYPE : expectedAsmType); myFrameMap.leave(descriptor); + + if (finallyBlock != null) { + gen(finallyBlock.getFinalExpression(), expectedAsmType); + } + + v.goTo(end); // TODO don't generate goto if there's no code following try/catch + v.visitTryCatchBlock(tryStart, tryEnd, clauseStart, descriptorType.getInternalName()); } if (finallyBlock != null) { Label finallyStart = new Label(); v.mark(finallyStart); - int index = myFrameMap.enterTemp(); - v.store(index, THROWABLE_TYPE); - gen(finallyBlock.getFinalExpression(), Type.VOID_TYPE); - v.load(index, THROWABLE_TYPE); v.athrow(); - myFrameMap.leaveTemp(); - v.visitTryCatchBlock(tryStart, tryEnd, finallyStart, null); } v.mark(end); + v.nop(); - return StackValue.none(); + if(finallyBlock != null) { + stackOfFinallyBlocks.remove(stackOfFinallyBlocks.size()-1); + } + + return StackValue.onStack(expectedAsmType); } @Override @@ -2661,4 +2689,9 @@ public class ExpressionCodegen extends JetVisitor { super(message); } } + + @Override + public String toString() { + return context.getContextDescriptor().toString(); + } } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java index e98303ef503..cc5a593222f 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java @@ -126,12 +126,8 @@ public class FunctionCodegen { codegen.returnExpression(bodyExpressions); } - try { + mv.visitMaxs(0, 0); - } - catch (Throwable t) { - System.out.println(t); - } mv.visitEnd(); generateBridgeIfNeeded(owner, state, v, jvmSignature, functionDescriptor, kind); diff --git a/compiler/testData/codegen/regressions/kt496.jet b/compiler/testData/codegen/regressions/kt496.jet new file mode 100644 index 00000000000..209942d2672 --- /dev/null +++ b/compiler/testData/codegen/regressions/kt496.jet @@ -0,0 +1,52 @@ +fun test1() : Boolean { + try { + return true + } finally { + if(true) // otherwise we wisely have unreachable code + return false + } +} + +var x = true +fun test2() : Boolean { + try { + } finally { + x = false; + } + return x +} + +fun test3() : Int { + var y = 0 + try { + ++y + } finally { + ++y + } + return y +} + +var z = 0 +fun test4() : Int { + z = 0 + return try { + try { + z++ + } + finally { + z++ + } + } finally { + ++z + } +} + +fun box() : String { + if(test1()) return "test1 failed" + if(test2()) return "test2 failed" + if(test3() != 2) return "test3 failed" + System.out?.println(test4()) + if(test4() != 3) return "test4 failed" + + return "OK" +} diff --git a/compiler/testData/codegen/regressions/kt500.jet b/compiler/testData/codegen/regressions/kt500.jet new file mode 100644 index 00000000000..ce7c329c23f --- /dev/null +++ b/compiler/testData/codegen/regressions/kt500.jet @@ -0,0 +1,30 @@ +var GUEST_USER_ID = 3 +val USER_ID = + try { + getUserIdFromEnvironment() + } + catch (e : UnsupportedOperationException) { + ++GUEST_USER_ID + } + +val USER_ID_2 = + try { + getUserIdFromEnvironment() + } + catch (e : UnsupportedOperationException) { + GUEST_USER_ID + } + finally { + GUEST_USER_ID++ + } + +fun getUserIdFromEnvironment() : Int = throw UnsupportedOperationException() + +fun box() : String { + System.out?.println("G: " + GUEST_USER_ID + " U1:" + USER_ID + " U2: " + USER_ID_2) + if(USER_ID != 4) return "test0 failed" + if(USER_ID_2 != 4) return "test2 failed" + if(GUEST_USER_ID != 5) return "test3 failed" + + return "OK" +} diff --git a/compiler/testData/codegen/regressions/kt501.jet b/compiler/testData/codegen/regressions/kt501.jet new file mode 100644 index 00000000000..ef6c092cedb --- /dev/null +++ b/compiler/testData/codegen/regressions/kt501.jet @@ -0,0 +1,24 @@ +class Reluctant() { + { + throw Exception("I'm not coming out") + } +} + +fun p(o : Any?) = System.out?.println(o) + +fun test1() : String { + try { + val b = Reluctant() + return "Surprise!" + } + catch (ex : Exception) { + return "I told you so" + } +} + + +fun box() : String { + if(test1() != "I told you so") return "test1 failed" + + return "OK" +} diff --git a/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java b/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java index 062aea12e43..336fa396c47 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java @@ -7,6 +7,7 @@ import java.util.List; /** * @author yole + * @author alex.tkachman */ public class ClassGenTest extends CodegenTestCase { public void testPSVMClass() throws Exception { @@ -208,4 +209,17 @@ public class ClassGenTest extends CodegenTestCase { System.out.println(generateToText()); blackBox(); } + + public void testKt501 () throws Exception { + blackBoxFile("regressions/kt501.jet"); + } + + public void testKt496 () throws Exception { + blackBoxFile("regressions/kt496.jet"); + System.out.println(generateToText()); + } + + public void testKt500 () throws Exception { + blackBoxFile("regressions/kt500.jet"); + } }