On 2/28/06, Mike Bland
On 2/28/06, Guido van Rossum
wrote: On 2/28/06, Mike Bland
wrote: On 2/28/06, Guido van Rossum
wrote: I just realized that there's a bug in the with-statement as currently checked in. __exit__ is supposed to re-raise the exception if there was one; if it returns normally, the finally clause is NOT to re-raise it. The fix is relatively simple (I believe) but requires updating lots of unit tests. It'll be a while.
Hmm. My understanding was that __exit__ was *not* to reraise it, but was simply given the opportunity to record the exception-in-progress.
Yes, that's what the PEP said. :-(
Unfortunately the way the PEP is specified, the intended equivalence between writing a try/except in a @contextmanager-decorated generator and writing things out explicitly is lost. The plan was that this:
@contextmanager def foo(): try: yield except Exception: pass
with foo(): 1/0
would be equivalent to this:
try: 1/0 except Exception: pass
IOW
with GENERATOR(): BLOCK
becomes a macro call, and GENERATOR() becomes a macro definition; its body is the macro expansion with "yield" replaced by BLOCK. But in order to get those semantics, it must be possible for __exit__() to signal that the exception passed into it should *not* be re-raised.
The current expansion uses roughly this:
finally: ctx.__exit__(*exc)
and here the finally clause will re-raise the exception (if there was one).
I ran into this when writing unit tests for @contextmanager.
This may just be my inexperience talking, and I don't have the code in front of me right this moment, but in my mind these semantics would simplify the original version of my patch, as we wouldn't have to juggle the stack at all. (Other than rotating the three exception objects, that is). We could then just pass the exception objects into __exit__ without having to leave a copy on the stack, and could forego the END_FINALLY. (I *think*.) Does that make sense?
Yes, it does. Except there's yet another wrinkle: non-local gotos (break, continue, return). The special WITH_CLEANUP opcode that I added instead of your ROT4 magic now considers the following cases: - if the "exception indicator" is None or an int, leave it, and push three Nones - otherwise, replace the exception indicator and the two elements below it with a single None (thus reducing the stack level by 2), *and* push the exception indicator and those two elements back onto the stack, in reverse order. To clarify, let's look at the four cases. I'm drawing the stack top on the right: (return or continue; the int is WHY_RETURN or WHY_CONTINUE) BEFORE: retval; int; __exit__ AFTER: retval; int; __exit__; None; None; None (break; the int is WHY_BREAK) BEFORE: int; __exit__ AFTER: int; __exit__; None; None; None (no exception) BEFORE: None; __exit__ AFTER: None; __exit__; None; None; None (exception) BEFORE: traceback; value; type; __exit__ AFTER: None; __exit__; type; value; traceback The code generated in the finally clause looks as follows: WITH_CLEANUP (this does the above transform) CALL_FUNCTION 3 (calls __exit__ with three arguments) POP_TOP (throws away the result) END_FINALLY (interprets the int or None now on top appropriately) Hope this helps (if not you, future generations :-). -- --Guido van Rossum (home page: http://www.python.org/~guido/)