bpo-46202: Remove opcode POP_EXCEPT_AND_RERAISE#30302
bpo-46202: Remove opcode POP_EXCEPT_AND_RERAISE#30302markshannon merged 10 commits intopython:mainfrom
Conversation
brandtbucher
left a comment
There was a problem hiding this comment.
Nice! Looks good to me, except for one weird bug I stumbled upon while reviewing it.
It's not directly related to these changes, but it still probably deserves to be fixed:
|
When you're done making the requested changes, leave the comment: |
|
I think the bot wants this in a new comment: I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @brandtbucher: please review the changes made to this pull request. |
ericsnowcurrently
left a comment
There was a problem hiding this comment.
This strictly relates to re-raising an exception, right?
try:
...
except Exception:
...
raise # re-raiseand not raising an exception while handing another:
try:
...
except Exception as exc:
...
raise exc
# or raise Exception
# or raise exc from exc2
It's for both - it's when you are about the exit a finally block, and you have a previous "handled exception" on the stack (because this finally is from a try-except or a with-statement which is nested in an except clause), and above it you have the lasti and an exception that you need to raise. So you need to pop the TOS and the lasti below it and raise, but first you need to take the THIRD item and use it to restore exc_info. |
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM
(assuming you address any other review comments, of course)
|
I think this PR lost focus a bit. I want to split the except* changes into a separate PR and just do a straightforward opcode removal in this one. |
|
I committed the except* changes separately so this PR now just removes POP_EXCEPT_AND_RERAISE. |
Following the reduction of exc_info to just one stack item, POP_EXCEPT_AND_RERAISE can be replaced by an equivalent sequence of other opcodes.
One of the except* usages can be simplified further because it doesn't need to restore lasti.(this was moved to another pr).https://bugs.python.org/issue46202