Skip to content
This repository has been archived by the owner on Apr 10, 2022. It is now read-only.

Staged introduction #9

Closed
gvanrossum opened this issue Oct 27, 2020 · 22 comments
Closed

Staged introduction #9

gvanrossum opened this issue Oct 27, 2020 · 22 comments

Comments

@gvanrossum
Copy link
Member

It's looking like this is going to be a very complicated PEP, and I'm not sure that we'll be able to get everything right even if we pay careful attention to what lessons we can learn from Trio.

Maybe we can do something in multiple stages, for example:

  1. No interpreter changes, only a user-mode ExceptionGroup class. We'll provide a context manager to handle exceptions, similar to Trio, and a helper function to print forked tracebacks. When using the system or stdlib facilities to print tracebacks, they appear to end at the ExceptionGroup. (The repr()/str() of the latter should at least hint that more is going on.) This means all new code is contained in a single module, and can be provided on PyPI for older Python versions.

  2. Update traceback.py to print nice forked tracebacks too, as long as you pass in the exception (presumably using the helper from step 1). But the C code to print tracebacks will still stop at an exception group (this code is too brittle to mess with in this stage).

  3. Traceback groups, implemented in C. Now the C code can print nice tracebacks too.

  4. except * syntax. This is the culmination of the design.

I'm not saying that these should correspond to releases (I don't want to have to wait until 3.13 :-) but I think it's unrealistic to expect that we'll get through stage 4 in 3.10. We can release stage 1 on PyPI any time we're ready. (We could release stage 2 there too via monkey-patching -- Trio does this.) Stage 3 seems simple enough to make it into 3.10 if we get positive results from stages 1-2. Then stage 4 could be targeting release 3.11.

@gvanrossum
Copy link
Member Author

Also, if we release stuff on PyPI, maybe we could convince @njsmith to switch the next Trio release to use our version, which should be a good way to validate the design (both convincing @njsmith and releasing it with Trio :-).

@gvanrossum
Copy link
Member Author

Another advantage of releasing via PyPI first is that we can do that without waiting for the SC.

@1st1
Copy link
Member

1st1 commented Oct 28, 2020

I'm not saying that these should correspond to releases (I don't want to have to wait until 3.13 :-) but I think it's unrealistic to expect that we'll get through stage 4 in 3.10. We can release stage 1 on PyPI any time we're ready.

My only wish is that we find a way for asyncio to have TaskGroups in 3.10. Of course in the worst case we can have TaskGroups on PyPI too, but the community really wants them to be in the core.

@gvanrossum
Copy link
Member Author

gvanrossum commented Oct 28, 2020

My only wish is that we find a way for asyncio to have TaskGroups in 3.10.

Then this should be inserted between 1 and 2, in terms of priority -- TaskGroups depends on ExceptionGroups. Having EG and TG on PyPI gives it to the community sooner -- the first PyPI release could be a few weeks away! That gives us a chance to iterate on the API a few times with some real users. Maybe not many real users, but definitely more than 3.10alpha gets. :-)

3.10beta1 is coming May 2021, so you have roughly 6 months to get it right for the stdlib.

@1st1
Copy link
Member

1st1 commented Oct 28, 2020

Then this should be inserted between 1 and 2, in terms of priority -- TaskGroups depends on ExceptionGroups. Having EG and TG on PyPI gives it to the community sooner -- the first PyPI release could be a few weeks away!

Having EG & TG on PyPI is better than nothing, but asyncio users will still have questions about it, like:

  1. Is it stable enough to use in my app?
  2. Should I use this in my library as a dependency?
  3. What if the final design that lands in Python is different? How difficult it will be to migrate my code?
  4. I see it patches the system except hook, is it safe to use in prod?
  5. Are EGs opaque for Sentry and logging?

That said, I don't want to sound like I'm against releasing on PyPI. I just don't expect it to get a lot of use. :/

What I can promise is that I can myself start using an early but relatively standard prototype in the EdgeDB code base.

3.10beta1 is coming May 2021, so you have roughly 6 months to get it right for the stdlib.

Yeah, the schedule is very tight, I understand.

@gvanrossum
Copy link
Member Author

Those questions will be hard to answer for an untested design in the stdlib.

@iritkatriel
Copy link
Member

I like this incremental plan. I started working on a branch for stage1, with immutable ExceptionGroups. Should send something over later today.

@iritkatriel
Copy link
Member

Branch for stage1: https://github.com/iritkatriel/cpython/tree/exceptionGroup-stage1

I've updated the exception_group.py script (and its output). @1st1 - can you check tg1.py?
I will now write some more interesting tests - it's missing one or two things for nested exception groups.

Making ExceptionGroup immutable is a massive simplification - TracebackGroups never need to be updated, they are just created from the ExceptionGroup.

@iritkatriel
Copy link
Member

The tracebacks should make more sense now - I removed the frame field from the EG, we get it from the __traceback__ which is set when the EG is raised.

Tracebacks of nested EGs look better as well.

@gvanrossum
Copy link
Member Author

gvanrossum commented Oct 28, 2020

Great! I reviewed your previous version and you already fixed at least one two of my suggestions. :-)

@1st1
Copy link
Member

1st1 commented Oct 28, 2020

Those questions will be hard to answer for an untested design in the stdlib.

I'll do my best to get early feedback from my project, promote the PyPI tool, and to get in touch with companies like Sentry to get early feedback.

Making ExceptionGroup immutable is a massive simplification - TracebackGroups never need to be updated, they are just created from the ExceptionGroup.

This sounds really great. I like the immutable approach a lot.

@iritkatriel
Copy link
Member

I started working on stage 2 (rendering in traceback.py): https://github.com/iritkatriel/cpython/pull/3/files

Basically I added TracebackExceptionGroup which generalizes TracebackException. It contains a sequence of (indent, TracebackException) pairs, which has len 1 for a simple exception and longer for an exception group.

Open:

  1. should we add an arg to limit the number of exceptions displayed (and on which end do we truncate?)
  2. Can the formatting be prettier? (I just did something simple.)

@iritkatriel
Copy link
Member

I started doing stage 3 here: iritkatriel/cpython#6

So far it has ExceptionGroup implemented in Objects/exceptions.c with the project() function. The Python ExceptionGroup class bacame ExceptionGroupHelper and has a few more functions that should move into C (like split and iteration).

The Catcher also needs to move to C, but that I think is part of stage 4 as it should tie in with the syntax.

@iritkatriel
Copy link
Member

I started working on stage 4 (the interpreter changes) and am stumped on something odd. I make this change: iritkatriel/cpython@0415e0c
which is to put a bool on the stack for the JUMP_IF_NOT_EXC_MATCH opcode (to tell it whether we're in an except or except*, we either do this or make a new opcode).

It seems simple enough, but there is some code path where the stack gets out of whack (import related? importlib.h etc were regenerated due to this). Already in the build I got this printed. Does it make sense?

is_star = False (<-- X many times)
is_star = False
is_star = <class 'AttributeError'>
is_star = False
is_star = <class 'AttributeError'>
is_star = False
is_star = False
is_star = False
is_star = False
is_star = False
is_star = False
is_star = False
is_star = False
is_star = False
is_star = False
is_star = False
is_star = <class 'ImportError'>
is_star = <class 'AttributeError'>

@gvanrossum
Copy link
Member Author

I suspect a new opcode is better (since for any particular occurrence of the opcode the flag always has the same value).

That said, the errors you get make me wonder if this opcode is implicitly the target of a goto in case an exception happens? Maybe the nice diagram on lines 2990-3017 in compile.c is helpful?

@gvanrossum
Copy link
Member Author

Another theory: you didn't change the magic number. In _bootstrap_external.py:

# MAGIC must change whenever the bytecode emitted by the compiler may no
# longer be understood by older implementations of the eval loop (usually
# due to the addition of new opcodes).

@iritkatriel
Copy link
Member

Another theory: you didn't change the magic number. In _bootstrap_external.py:

That seems to be it, thanks.

@iritkatriel
Copy link
Member

This is a very rough start, so don't spend too much time reviewing details. See my comment on the PR.
iritkatriel/cpython#7

@iritkatriel
Copy link
Member

I am still working on how to support raise inside an except* block, but I think all the other bits are there in iritkatriel/cpython#7.
(see Lib/test/test_except_star.py)

The major additions in this iteration are in
Grammar/python.gram
Python/ceval.c
Python/compile.c

There are probably many things wrong with what I did, so don't hold back - I'm pleased to have made it this far!

@iritkatriel
Copy link
Member

Are we still thinking about an incremental release plan?

@gvanrossum
Copy link
Member Author

I think not. We've done the development incrementally, but now would be a good time to get feedback from the core dev community, and the best way to do that would be a PEP. I had wanted to wait for Nathaniel, but I don't think we'll get his feedback, alas.

@iritkatriel
Copy link
Member

So closing this and we move on to #22 to finish a PEP.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants