[Patches] [ python-Patches-661719 ] allow py_compile to re-raise exceptions

SourceForge.net noreply@sourceforge.net
Mon, 06 Jan 2003 12:13:20 -0800


Patches item #661719, was opened at 2003-01-03 16:59
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=661719&group_id=5470

Category: Library (Lib)
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Gyro Funch (siva1311)
Assigned to: Nobody/Anonymous (nobody)
Summary: allow py_compile to re-raise exceptions

Initial Comment:
py_compile does not re-raise exceptions so that they
can be caught by the calling program. The following
example illustrates the problem:

-----begin bad_prog.py-----
print "hello
-----end   bad_prog.py-----


-----begin test_compile.py-----
from py_compile import compile

class CompileError(Exception): pass

fl = 'bad_prog.py'
try:
    compile(fl)
except Exception,msg:
    raise CompileError, '%s: %s' % (msg,fl)
-----end   test_compile.py-----

-----
[~]:python test_compile.py 
  File "bad_prog.py", line 1
    print "hello
               ^
SyntaxError: invalid token
-----
Note that CompileError is not raised.


The attached simple patch should take care of the
problem (and I hope not cause any bad side effects). 

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

>Comment By: Gyro Funch (siva1311)
Date: 2003-01-06 20:13

Message:
Logged In: YES 
user_id=679947

I changed 'except SyntaxError:' to 'except Exception:' since
compileall.py has the 'except:' clause in the following:
                try:
                    ok = py_compile.compile(fullname, None,
dfile)
                except KeyboardInterrupt:
                    raise KeyboardInterrupt
                except:
                    # XXX py_compile catches SyntaxErrors
                    if type(sys.exc_type) == type(''):
                        exc_type_name = sys.exc_type
                     ...

If this block is not necessary, then I can simplify things
somewhat. The test for string exceptions was taken from here
as well. 

Should I assume that only class-based and SyntaxError
exceptions can occur from __builtin__.compile? 
(I looked at compile.c, and from my
non-c-programmer-perspective, it looks like other exceptions
can be raised. _If I read the code correctly_, it looks like
ValueError, MemoryError, OverflowError, and SystemError can
also occur.)

Attached is the latest diff with most of your suggestions
implemented. I'll work on the documentation.


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

Comment By: Martin v. Löwis (loewis)
Date: 2003-01-06 15:30

Message:
Logged In: YES 
user_id=21627

I'm still unsure why you replace the except SyntaxError: with 
except Exception:. What other exceptions could occur in this 
place? Also, calling exc_info isn't necessary - you could 
obtain the exception value in the catch clause. Unless you 
are expecting string exceptions (which ones), code to 
support string exceptions should not be needed, and the type 
can be obtained by looking at __class__.

Don't compare types by comparing their names: to find out 
whether t is SyntaxError, write 't is SyntaxError', 
not 't.__name__ == "SyntaxError"'.

Exception instances should always have a .args attribute. 
This is best generated by calling the base class __init__ in 
an exception's __init__.

Please provide the documentation changes (docstrings, 
Doc/lib, and a NEWS entry) as well.

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

Comment By: Gyro Funch (siva1311)
Date: 2003-01-06 14:48

Message:
Logged In: YES 
user_id=679947

I have implemented a 'doraise' flag in py_compile.compile.
I think the patch should now be backwards-compatible.
However, I think that files that use py_compile in the
standard library (compileall.py and zipfile.py) should
perhaps use the new exception mechanism for clarity and as a
demonstration of how to use the exception.
What do you think?
I'm not sure if I've eliminated the 'few minor issues'. I
did  implement 'except Exception:' instead of a raw
'except:'. Is this what you meant.
Thanks for your comments.

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

Comment By: Martin v. Löwis (loewis)
Date: 2003-01-06 13:27

Message:
Logged In: YES 
user_id=21627

The patch looks fine so far, except for a few minor issues
(i.e. never to use bare except: clauses). You don't have to
implement a migration strategy: just elaborating it here
would be fine. To preserve compatibility, I could envision a
flag ('doraise' or some such), or a different entry point.
If you manage to provide full backwards compatibility, you
don't need to change any of the existing callers.

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

Comment By: Gyro Funch (siva1311)
Date: 2003-01-06 13:15

Message:
Logged In: YES 
user_id=679947

The following patch adds a 'PyCompileError' exception to
py_compile and adds the appropriate 'except' and 'raise'
clauses to py_compile.py, compileall.py, and zipfile.py (I
think that these are the only affected files in the standard
library).
The exception class messages were chosen to try to simulate
the current behavior in these files as closely as possible.

If this patch looks okay (or at least is in the right
direction), I'll begin to look at the documentation and
migration issues.


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

Comment By: Gyro Funch (siva1311)
Date: 2003-01-04 15:20

Message:
Logged In: YES 
user_id=679947

Thank you for the comments. 
I was trying to maintain some aspects of the current behavior and 
add the necessary exception raise. Clearly, this is not correct. 
I'll give this patch more thought and consider issues of 
documentation and migration as well. 

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

Comment By: Martin v. Löwis (loewis)
Date: 2003-01-03 23:06

Message:
Logged In: YES 
user_id=21627

This patch is wrong: Callers of py_compile.compile, such as
py_compile.main, now need to catch potential exceptions. As
this is an incompatible change, you need to provide
documentation, and ideally a migration strategy.

It is also unclear why you both print the exception and
raise it: If an exception is raise, it is normally the
responsibility of the caller to print the exception.

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

Comment By: Gyro Funch (siva1311)
Date: 2003-01-03 21:20

Message:
Logged In: YES 
user_id=679947

Sorry, the file should be attached now. I find the SourceForge 
web interface a little confusing.

-g

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

Comment By: Gyro Funch (siva1311)
Date: 2003-01-03 21:07

Message:
Logged In: YES 
user_id=679947

There's no uploaded file!  You have to check the
checkbox labeled "Check to Upload & Attach File"
when you upload a file.

Please try again.

(This is a SourceForge annoyance that we can do
nothing about. :-( )

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

Comment By: Martin v. Löwis (loewis)
Date: 2003-01-03 20:43

Message:
Logged In: YES 
user_id=21627

There's no uploaded file!  You have to check the
checkbox labeled "Check to Upload & Attach File"
when you upload a file.

Please try again.

(This is a SourceForge annoyance that we can do
nothing about. :-( )

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

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