bpo-46458: emit code for else of a try block immediately after the try body #30751
Conversation
|
Just to check. There is no change in behavior and all the new tests pass on main as well? |
The new tests in test_exception_variations and test_sys_settrace indeed pass on main. |
@markshannon Any further thoughts on this? My intention is to merge this as a first step and then move on to the larger refactor that removes the jump from the end of orelse to the finally clause of the happy path. Then I want to try and ensure that we have only one copy of finally for error cases (because there is no When possible, I prefer to break changes up into steps which are easier to review (and later debug if necessary). |
|
Looks good to me.
I agree. |
Misc/NEWS.d/next/Core and Builtins/2022-01-27-10-49-34.bpo-46458.5Gm3Gv.rst
Outdated
Show resolved
Hide resolved
| self.assertFalse(hit_inner_else) | ||
| self.assertFalse(hit_else) |
There was a problem hiding this comment.
Would these tests be slightly easier to read if the self.assertFalses were inlined as self.fail()? It might be easier to visualise the correct path with just a short glance.
There was a problem hiding this comment.
You could do that for the assertFalse, but not for the assertTrue.
There was a problem hiding this comment.
Yes, that was the point :) The self.fails will clearly indicate the wrong path; you'll quickly be able to tell the correct path from the incorrect path.
There was a problem hiding this comment.
Well, possibly that would be better. I followed the style of the existing test here.
There was a problem hiding this comment.
IMO, readability is more important than following the existing style. But, I'm nitpicking. It's your pr :)
Thank you for the review @markshannon. |
This change makes us emit the code for an else of a try immediately after the try body so it avoids the jump to the else block. It adds a jump from the end of the else block to after the except blocks, so we are net neutral in terms of jumps.
However, since there is only one exit from the try-except-else part, the finally block doesn't get inlined so we gain a bit in terms of code size (but we still have the same number of jumps).
https://bugs.python.org/issue46458