GNOME Bugzilla – Bug 579101
Return inside of try block routes around finally block
Last modified: 2009-09-30 08:53:12 UTC
Please describe the problem: When a return statement is inside a try block, the code in the finally block is not executed. Steps to reproduce: void main(string[] args) { try { stdout.printf("foo"); return; } finally { stdout.printf("bar"); } } Actual results: "foo" Expected results: "foobar" Does this happen every time? Yes. Other information: I don't know if this is related to this being a try block with no catch block.
I should add: I'm using valac 0.6.1.
It occurs with valac 0.7.1.
This does not happen anymore with latest git version.
It still happens.
Created attachment 138621 [details] a simple testcase This is a simple test case. Just look at the generated C code, it's a total mess. Since it's also a great opportunity to learn something about the compiler, I'm trying to fix it myself right now.
Created attachment 139211 [details] [review] Copy the finally block in front of every jump This patch simply duplicates the whole finally block (the whole chain, actually) in front of jumps. Quite redundant, but considering that most finally blocks are one, maybe two lines, it's perhaps not so bad. Originally, I was working on completely redoing the way scope cleanup is done, by concentrating all the cleanup code at the end of scope, instead of doing it in front of jumps. Now I'm not sure whether it's worthwhile at the moment. It removes redundancy, but it also adds a lot of goto statements and a helper variable to track the reason of scope exit. Any opinions on that?
The patch I posted is wrong. Ignore it until I attach fixed version.
Created attachment 139213 [details] [review] Copy the finally block in front of every jump Fixed the problems. Now it should hopefully work correctly.
Thanks for the patch. It looks fine in general, but I have a few comments: The change in GErroModule.visit_try_statement looks wrong. I haven't tested it but I suspect that this will cause issues if code in the catch clause or the finally block throws an error (as long as `current_try' is set, the compiler assumes that errors should be caught by the corresponding catch clauses). The patch adds tracking of variables modified in finally blocks to avoid negative side-effects by the shortcut of the return statement (avoiding ref/unref or dup/free pairs). I'm not sure we want to add this in this form. The reason is that this adds some complexity just for a tiny optimization. As you've probably noticed, the code generator is already complex enough as it is, I'm trying to simplify it. An alternative to variable tracking is to just disable the shortcut of the return statement when the return statement is within a try that has a finally block. This keeps it a bit simpler without big disadvantages, in my opinion.
Created attachment 139412 [details] [review] Copy the finally block in front of every jump You were right about the catch clause. I forgot about that. It seems throws in catch clauses were circumventing finally block, too. It's fixed now. I've also replaced the modification checking with simple accessibility check, as you wished. You're probably right there is not too much to worry about. Hopefully, everything should be in order now. :)
commit 068d310a4943b04747f0fc9ad7b645709405ff53 Author: Jiří Zárevúcky <zarevucky.jiri@gmail.com> Date: Tue Jul 28 22:32:51 2009 +0200 Fix jump statements in try with finally Fixes bug 579101.
commit 9f21d0b182edad861f93a91674787b8b3b4fc2c5 Author: Jürg Billeter <j@bitron.ch> Date: Wed Sep 30 10:52:27 2009 +0200 Add test for bug 579101