Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(10491)

Issue 1556042: T541 - Exception catching semantic change in Python 3

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 10 months ago by Haoyu Bai
Modified:
13 years, 8 months ago
Reviewers:
craigcitro
Visibility:
Public.

Patch Set 1 : updated patch #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -1 line) Patch
M Cython/Compiler/Main.py View 2 chunks +2 lines, -1 line 0 comments Download
M Cython/Compiler/ParseTreeTransforms.py View 1 chunk +31 lines, -0 lines 4 comments Download
M tests/run/cython3.pyx View 1 chunk +10 lines, -0 lines 1 comment Download

Messages

Total messages: 1
craigcitro
13 years, 8 months ago (2010-08-18 08:04:45 UTC) #1
This looks good, but I think we can go a little further. In particular, I think
we can set things to None (since we can't del), and we can make sure that we
cover all the various syntax variations the PEP describes.

http://codereview.appspot.com/1556042/diff/2001/3002
File Cython/Compiler/ParseTreeTransforms.py (right):

http://codereview.appspot.com/1556042/diff/2001/3002#newcode838
Cython/Compiler/ParseTreeTransforms.py:838: # Transform the except clause body
by following the PEP3110 sematic
sematic -> semantics

http://codereview.appspot.com/1556042/diff/2001/3002#newcode839
Cython/Compiler/ParseTreeTransforms.py:839: # of excpetion catching
excpetion -> exception

http://codereview.appspot.com/1556042/diff/2001/3002#newcode840
Cython/Compiler/ParseTreeTransforms.py:840: # XXX(haoyu) There should be 'del
EXC' but del is not supported yet
It's true that we don't support del -- however, we *can* at least set EXC to
None, and confirm that it's been set to None. It's bad that the name is still
defined, but this will still solve the reference counting problem (which, as I
understand, is part of the purpose of the PEP). 

Also, XXX -> TODO

http://codereview.appspot.com/1556042/diff/2001/3002#newcode851
Cython/Compiler/ParseTreeTransforms.py:851: # Only do this tranform for Python 3
tranform -> transform

http://codereview.appspot.com/1556042/diff/2001/3003
File tests/run/cython3.pyx (right):

http://codereview.appspot.com/1556042/diff/2001/3003#newcode29
tests/run/cython3.pyx:29: return
So this confirms that we can use the new syntax -- but shouldn't we also check
that foo has been set to None? 

Also, several other forms of try/except have been disallowed as part of the PEP;
we should have tests for all of them. Even if we don't support them, it's good
to know what we do and don't support.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b