GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||
GeezerVison is showing github.com — bpo-45292: [PEP-654] add except* by iritkatriel · Pull Request #29581 · python/cpython Conversation
|
The bug is actually related to the exception table lookup. This test fails at the 1/0: with: If I change except* to except it prints:
The except*/except change should not impact the level of the 1/0 instruction. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@markshannon Can you look at Irit’s last comment? Is it perhaps because the exception table is generated by magic opposes? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Here is the relevant dis: Old except: Except*: I think it's wrong that an exception from line 90 jumps to line 84. I'm missing something that tells it to go directly to line 100 from there. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…ssage for mixed except and except*. Add tests for custom errors
|
@markshannon I think I found the bug - there was a POP_BLOCK that needed to move out of the loop. I'll add a few unit tests and push it. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Erlend Egeberg Aasland <[email protected]>
There was an error while loading. Please reload this page.
…* cases in compiler
This reverts commit ca44878.
Co-authored-by: Guido van Rossum <[email protected]>
There was an error while loading. Please reload this page.
I hope so, but let us know if you are not planning to (@markshannon , @erlend-aasland). @ambv - you also had a close look/experimentation with this PR, any comments from you? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Fantastic job! Here's an "#*%!( lot of nitpicks; feel free to ignore them 🙂 But, more important: I found some branches that weren't covered by the test suite. If possible, we should increase coverage. I also left some comments regarding usage of some C API's.
Nitpicks come in two flavours:
Python/compile.c as well, but I guess it's ok to disregard PEP 7 for that file.)Py_Is, Py_IsNone, etc.). Do with these comments as you want 🗑️
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Thank you Erlend! Co-authored-by: Erlend Egeberg Aasland <[email protected]>
There was an error while loading. Please reload this page.
Co-authored-by: Erlend Egeberg Aasland <[email protected]>
…l/cpython into bpo-45292-except_star
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Merge whenever you feel ready!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I don't think that this was ready to merge. As I said before, this does too much in the interpreter that should be done in the compiler. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry if we jumped the gun. But this works well enough so that we can start writing code that uses it (which is a requirement the SC gave us when they approved PEP 654). I suppose we could do that in a branch, but that's very awkward -- it places a high barrier on those who just want to experiment with what this feels like. There will be plenty of opportunity to refactor the implementation -- we have many alpha releases left until beta 1 (late May), and even after that we can fix bugs until the release candidates start (in August). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@ambv has been doing it in a branch and did not report any bugs so far. We need more people to start using it, we need feedback on both the semantics and the implementation, and we need to give @1st1 time to develop the asyncio parts. Indeed, I am keen to hear @markshannon's ideas about refactoring/simplifying/optimizing the implementation. The first suggestion (2-3 weeks ago) didn't work because it overlooked that fact that we need to maintain state between the except* clauses. Hopefully we can find something that does work. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Let's move that discussion back to bpo-45292. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Unfortunately, this change results in a 1% slowdown |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.