[Patches] [ python-Patches-1355913 ] PEP 341 - Unification of try/except and try/finally

[SourceForge.net]

Patches item #1355913, was opened at 2005-11-14 00:59
Message generated for change (Comment added) made by ncoghlan
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1355913&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Submitted By: Thomas Lee (krumms)
Assigned to: Reinhold Birkenfeld (birkenfeld)
Summary: PEP 341 - Unification of try/except and try/finally

Initial Comment:
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. 


----------------------------------------------------------------------

>Comment By: Nick Coghlan (ncoghlan)
Date: 2005-11-14 18:54

Message:
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".



----------------------------------------------------------------------

Comment By: Thomas Lee (krumms)
Date: 2005-11-14 15:18

Message:
Logged In: YES 
user_id=315535

Oops - I mean Neal :)

----------------------------------------------------------------------

Comment By: Thomas Lee (krumms)
Date: 2005-11-14 15:17

Message:
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?


----------------------------------------------------------------------

Comment By: Thomas Lee (krumms)
Date: 2005-11-14 10:02

Message:
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.

----------------------------------------------------------------------

Comment By: Thomas Lee (krumms)
Date: 2005-11-14 09:12

Message:
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.

----------------------------------------------------------------------

Comment By: Thomas Lee (krumms)
Date: 2005-11-14 09:12

Message:
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.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2005-11-14 09:10

Message:
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?

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2005-11-14 07:27

Message:
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.

----------------------------------------------------------------------

Comment By: Thomas Lee (krumms)
Date: 2005-11-14 01:02

Message:
Logged In: YES 
user_id=315535

Attaching a unit test checking the various different types
of exception handling syntax.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1355913&group_id=5470