I'm not sure that this is worthwhile, at least not in this form.

The current approach is very fragile, as you rightly point out. However, this PR moves code from a cold pathgenerator.throw() onto the hot path of ayield from.

Although the points you raise are valid, they all safe for now (although they probably won't be in future).

It doesn't properly handle the case where SEND has an EXTENDED_ARG.

Until the compiler freely re-orders blocks, the oparg will only ever be very small as it only jumps over a couple of instructions.

It scans backwards, which is susceptible to accidentally reading inline cache entries as if they were valid instructions.

Again, it's OK because we know what instructions we are scanning over.

It jumps from a YIELD_VALUE instruction, which isn't really supposed to be possible.

Technically, it steps back to theSEND and then jumps.

IMO, the "proper" fix for this is to wrap theYIELD_VALUE after theSEND in a try-except that throws the exception into the inner generator, as the only way aYIELD_VALUE can raise is in athrow().

Now:

start:    SEND end    YIELD_VALUE    RESUME    JUMP_NO_INTERRUPT starthandler:    THROW_INTO # Maybe a call to an intrinsic function: CALL_INTRINSIC "throw_into", 1end:

With hidden try-except:

start:    SEND end    SETUP_FINALLY handler    YIELD_VALUE    POP_BLOCK    RESUME    JUMP_NO_INTERRUPT starthandler:    THROW_INTO # Throws the exception into the sub-generator.end:

This was part of the motivation for splitting outSEND in the first place.

Technically, it steps back to theSEND and then jumps.

This seems unhelpfully pedantic.

IMO, the "proper" fix for this is to wrap theYIELD_VALUE after theSEND in a try-except that throws the exception into the inner generator, as the only way aYIELD_VALUE can raise is in athrow().

I don’t think I agree (at least not yet). It's not too clear how your suggestion would work, so just to make sure my understanding is correct:

• _gen_throw will set the current exception to whatever we're throwing, then callPyEval_EvalFrameEx(gen_frame, /*throwflag=*/1).
• This will pop TOS (which is the last thing we yielded, and we don't care about anymore) and jump toTHROW_INTO, which performs the equivalent of unsetting the current exception and callingTOS.throw(exc_type, exc_value, exc_traceback).
• The above steps keep happening all the way down the chain.
• When one of the calls returns,THROW_INTO will handle three separate cases:
  • Thethrow() call raised an exception:
    • It's aStopIteration, soSET_TOP(return_value) and start the next instruction?
    • It's something else, sogoto error;.
  • It yielded the next value, so... push the new value, and jump back into theyield from/await loop?

I'm not sure if I got that right, since a lot of this code is new to me. If so, it seems much, much more complex and expensive than what I've proposed here: clearing the sub-iterator when it returns and adding a single, easily-predictableNULL check toSEND.

I hadn't considered the special case forStopIteration.THROW_INTO shouldn't have to do any more work thanSEND.

IfTHROW_INTO were the same asSEND, except that it callsTOS.throw() instead ofTOS.send(), then the following should work:

start:    SEND end    SETUP_FINALLY handler    YIELD_VALUE    POP_BLOCK    RESUME    JUMP_NO_INTERRUPT starthandler:    THROW_INTO end    RESUME    JUMP_NO_INTERRUPT startend:

Technically, it steps back to theSEND and then jumps.

This seems unhelpfully pedantic.

No, this is an important detail.
Because it means that we know the step backwards is correct, for the reasons already given, and that we are jumping from an instruction that is a jump, so that is also OK.

With the beta freeze looming, Istrongly feel that we should go with this approach (at least for 3.11). It's a working, very minimal change to the existing code that helps other projects (like JITs, PEP 523 hooks, etc.) that need this logic to be moved back into the bytecode. The additional branch is cheap, easily predictable, and fits naturally with the existing code.

For reference, I've now tried two other approaches with virtualtry/excepts. Both are considerably more complicated, and add a lot of additional instructions and logic to eachyield from/await:

Attempt #1 uses a newTHROW opcode in a virtualtry/except. It still fails the test suite with the following error that I've spent days trying to debug:

======================================================================ERROR:test_anext_iter (test.test_asyncgen.AsyncGenAsyncioTest.test_anext_iter) [pure-Pythonanext()]----------------------------------------------------------------------Traceback (mostrecentcalllast):File"/home/brandtbucher/cpython/Lib/test/test_asyncgen.py",line735,inrun_testtest(py_anext)^^^^^^^^^^^^^^File"/home/brandtbucher/cpython/Lib/test/test_asyncgen.py",line674,intest3g.close()^^^^^^^^^File"/home/brandtbucher/cpython/Lib/test/test_asyncgen.py",line78,inanext_implreturnawait__anext__(iterator)^^^^^^^^^^^^^^^^^^^^^^^^^RuntimeError:cannotreusealreadyawaited__anext__()/asend()----------------------------------------------------------------------

I'm also not confident that the rest of the logic is 100% correct.

Attempt #2 uses a more complicated virtualtry/except that catchesStopIteration and extracts itsvalue member. It does pass all tests, though.

I'll leave the decision up to you,@markshannon.

Well, this didn't make it into 3.11.
For the best, IMO. The current implementation might be clunky, but it does work. Best not to rush in last minute changes.

Approach 2 passes the tests, and has fewer changes to the instruction set. That seems better than approach 1.
@brandtbucher Which do you prefer?

One other question.
How much ofhttps://peps.python.org/pep-0380/#formal-semantics should we move into the compiler, and how much belongs in helper C code?

Closing in favor of#96010.