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.