Attached is a patch implementing PEP 341 against HEAD in subversion. It includes the following changes: 1. Grammar/Grammar updated as per the PEP 2. Python/ast.c wraps try/except blocks inside try/finally blocks if it detects the extended syntax. This patch is based heavily upon suggestions by Nick Coghlan on python-dev. 

Logged In: YES user_id=315535 Attaching a unit test checking the various different types of exception handling syntax.

Logged In: YES user_id=33168 Updated patch to correct memory leaks. Put everything in one file. Works for me. There needs to be doc changes done too.

Logged In: YES user_id=33168 I'm pretty sure this patch causes this problem: python: Objects/frameobject.c:187: frame_setlineno: Assertion `blockstack_top > 0' failed. I'm not sure if it's just my hacked up version or the original. Thomas, can you test your version?

Logged In: YES user_id=315535 Further correction of memory leaks. 'finally' and 'orelse' need to be deallocated if an error occurs in the "if (n_except > 0)" branch.

Logged In: YES user_id=315535 Further correction of memory leaks. 'finally' and 'orelse' need to be deallocated if an error occurs in the "if (n_except > 0)" branch.

Logged In: YES user_id=315535 Fixed a bad assumption when parsing a try-finally. Should be fixed now. I'm not sure if this fixes the problem you're experiencing, Neal. I'll have a look into it.

Logged In: YES user_id=315535 Fix for the problem you encountered in the v5 patch, Nick. All tests in the suite now pass for me, no assertion failures when using --with-pydebug. I was stupidly generating a TryFinally in every scenario. This breaks down in the 'trace' unit test when no 'finally' clause is present with the extended syntax. Oops. Also added a little more error checking code and updated the unit test to test nested exception handling. I'm using a few goto statements in the "if (n_except > 0)" branch to reduce duplication deallocating memory. If there's any great objections to this feel free to yell & I'll resubmit a patch without the use of goto. Can you please verify this works for you, Nick? 

Logged In: YES user_id=315535 Oops - I mean Neal :)

Logged In: YES user_id=1038590 From a code inspection of v5 of the patch (I haven't actually run the patch anywhere), I'm a little uncomfortable with the mixture of the two error handling styles (that is, test-cleanup-return for most of the function, but test-goto-cleanup for the one specific case). I don't mind either style (and both are used in the Python source), but mixing them in one function bothers me :) Further, there appears to be a bug in the deallocation code, in that all sequences should be freed with adsl_seq_stmt_free. The current direct use of adsl_seq_free means any contained statements are not released. I suggest putting a single 'error' label at the end of the function that uses adsl_seq_stmt_free to clean up all the statement sequences , as well as free_stmt to clean up the nested except statements. As both of these functions handle nulls gracefully, the bodies of the current error cleanup and return branches can all then be replaced with "goto error". 

Logged In: YES user_id=315535 Thanks for your input Nick - I'll sort out a patch taking your suggestions into consideration tomorrow & resubmit it.

Logged In: YES user_id=315535 Okay, v6 is up. Now using test-goto-cleanup. Also using asdl_stmt_seq_free. Any more suggestions/feedback? Anybody having trouble using this patch?

Logged In: YES user_id=33168 Oh, so close, I'm sorry, I see one more problem. :-) Thomas, I hope you will write up this experience in coding this patch. IMO it clearly demonstrates a problem with the new AST code that needs to be addressed. ie, Memory management is not possible to get right. I've got a 700+ line patch to ast.c to correct many more memory issues (hopefully that won't cause conflicts with this patch). I would like to hear ideas of how the AST code can be improved to make it much easier to not leak memory and be safe at the same time. The problem is that handlers is not an asdl_seq of stmt's, but rather an asdl_seq of exceptionhandlers. There is no utility function to clean out this seq, so you need to do it manually. for (i = 0; i < asdl_seq_LEN(handlers); i++) free_exceptionhandler(asdl_seq_GET(handlers, i)); asdl_seq_free(handlers); I don't see any other problems, but perhaps I will when it gets checked in or I run valgrind on it. The Other Nick :-)

Logged In: YES user_id=315535 lol thanks Neal/Nick, that's fine. :) Nick Coghlan and yourself have been extremely helpful throughout this whole process, and it's been a great learning experience. I'll implement your suggested fix for 'handlers' right away. If you like, let me know when you've committed the 700+ line patch and I'll generate the PEP 341 patch from that. - Tom 

Logged In: YES user_id=315535 Here's the v7 patch against revision 41451, with Neal's last fix :) Using Marek's suggestion for try/except emulation in C from python-dev to avoid the "goto" crap. Any more problems here? *looks hopeful* :) - Tom

Logged In: YES user_id=33168 ISTM there are a couple of problems. Some of these were likely in there before and I may have told you to do some. :-( I probably made the same error too. :-( :-( One is a new one though. That starts in the for loop. You break in there, but that only breaks out of the for loop, doesn't it? So you really need a goto to break out I think. After setting except_st, it holds references to body, handlers, and orelse, so if you free except_st, it should free all the others. Therefore if except_st was created (after the if (except_st) break;), you should set body = handlers = orelse = NULL; Do you agree? I think if you don't you will get a double free. The same would go for after you asdl_seq_SET(inner, 0, except_st); you should set except_st = NULL; OTOH, you fixed a memory leak. Whenever one does return TryExcept(); (or any other similar AST call), if TryExcept fails, we will return NULL and all the local variables will be leaked. *sigh* Sorry you have to deal with this crap. You've been a very good guinea pig being the first. I really hope we do the arenas as discussed on python-dev. If you aren't too burnt out, it would be a really good way to learn the rest of the AST. :-)

Logged In: YES user_id=315535 Arrgh! :) Well ... I'll fix up this patch & submit it again. I might fall back to the 'goto' style of doing things just so we don't have the mix of "do ... break ... while" and goto (which I imagine could be almost as confusing as the mix of check-cleanup and goto-cleanup as described by Nick earlier). Again, if you could check the v8 patch for me it would be much appreciated. After that, I might hit python-dev and read up on what everybody's been saying about pools/arenas & maybe get started on something we can use for the AST ... anyway, I'll discuss that on python-dev. This isn't really the place for it :)

Logged In: YES user_id=315535 Okay, here we go: v8! *sighs* this is devestating for my ego :)

Logged In: YES user_id=1038590 Closest yet, but I don't think we're quite there yet. The 'except_st = NULL' part should be on the line immediately after except_st is inserted into the inner sequence. As soon as that insertion happens, we want to ensure that the statement is only freed once. What's currently there will technically work, but doesn't really have the right semantics. More importantly, the "body = handlers = orelse = NULL" part should only be executed if the call to TryExcept *succeeds*, not if it fails. Neil's right - we seriously need to find a better way to handle this memory management or it will be a source of significant pain (well, more pain than you've already experienced ;)

Logged In: YES user_id=315535 You're absolutely right with those bugs Nick. Fixed now. See how you go with v9 :) 

Logged In: YES user_id=33168 Maybe I'm just tired, but I can't find any problems. Assuming this patch gets in, it ought to be the least buggy code in compile.c. :-)

Logged In: YES user_id=315535 lol ... even if it doesn't get accepted, it was a great learning experience. The PEP itself hasn't even been approved as yet, I've just wanted this syntax in Python for a while :) Anyway ... might try my hand at a few ideas I have for that arena/pool stuff as discussed on python-dev when I get home tonight.

Logged In: YES user_id=1188172 Assigning to Guido for final word regarding PEP 341. I've not checked the patch for errors, I'm not firm enough with AST etc.

Logged In: YES user_id=6380 PEP 341 is accepted (sorry, I thought I had accepted it in May!). Assigning to Neal for final verification and checkin.

Logged In: YES user_id=33168 Thanks Thomas! Committed revision 41740.