GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-46409: Make generators in bytecode by markshannon · Pull Request #30633 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Jan 17, 2022 •
edited by gvanrossum
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Adds a RETURN_GENERATOR bytecode which makes a generator (or coroutine) from the current frame.
This has a number of advantages:

  1. Generator frames cannot be incomplete, as the frame is fully initialized before creating the generator
  2. It simplifies the code for calling a Python function, as there is no need for a special case for generator functions.
  3. It removes the need to specialize calls to generator functions, they can be specialized as any other call to a Python function.

This PR also adds a JUMP_NO_INTERRUPT bytecode. This is the same as JUMP_ABSOLUTE without any checks for interrupts. Checking for interrupts in yield from would destroy the flaky pretense that Python coroutines are stackful. (They are not, but PEPs 380 and 489 like to pretend that they are), so we need the extra, seemingly redundant instruction.

We use RETURN_GENERATOR rather than MAKE_GENERATOR; RETURN_VALUE as the RETURN_VALUE would make the compiler treat all the following code as unreachable. We could fix that using artificial instructions, but I think that is best left to another PR.

https://bugs.python.org/issue46409

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
markshannon added 5 commits January 17, 2022 10:59
@markshannon
Add new instruction to make generator or coroutine from current frame.
bf0394d
@markshannon
Set line number for RETURN_GENERATOR instruction.
23877ec
@markshannon
Fix magic number
381f3d3
@markshannon
Remove unused code.
c2b141e
@markshannon
Rename YIELD_FROM_LOOP to JUMP_NO_INTERRUPT: describe what it does, n…
8b73d0d
…ot where it might be used.
@markshannon
markshannon requested a review from 1st1 as a code owner January 17, 2022 12:54
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jan 17, 2022
@bedevere-bot bedevere-bot added the awaiting core review label Jan 17, 2022
markshannon added 2 commits January 17, 2022 12:56
@markshannon
Document new bytecodes.
7cb3dad
@markshannon
Add news
79b6d11
@markshannon

markshannon commented Jan 17, 2022

Copy link
Copy Markdown
Member Author

Fixes https://bugs.python.org/issue46389 and https://bugs.python.org/issue46374, but does not include tests for those issues.
I'll add the tests in separate PRs to close those issues.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

markshannon added 2 commits January 17, 2022 13:54
@markshannon
Merge branch 'main' into make-gen-in-bytecode
039b14d
@markshannon
Fix whitespace in opcode.py
c33abbf
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 17, 2022
@bedevere-bot

bedevere-bot commented Jan 17, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @markshannon for commit c33abbf 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 17, 2022
markshannon added 5 commits January 18, 2022 12:27
@markshannon
Trim frame and generator by word each.
d105cac
@markshannon
Minor refactor of frame.c
e7c4bf2
@markshannon
Update test.test_sys to account for smaller frames.
c080089
@markshannon
Skip clearing of old frame when returning a generator.
1b177e3
@markshannon
Treat generator functions as normal functions when evaluating and spe…
243e441
…cializing.
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 18, 2022
@bedevere-bot

bedevere-bot commented Jan 18, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 243e441 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 18, 2022
@markshannon
Attempt to streamline eval-breaker checks a bit.
a79e36b
gvanrossum
gvanrossum reviewed Jan 19, 2022
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still trying to follow the rest of the C code here, but in case I fail, here's the rest of my review. :-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Misc/NEWS.d/next/Core and Builtins/2022-01-17-12-57-27.bpo-46409.HouS6m.rst Outdated
@@ -0,0 +1,2 @@
Add new bytecode to make generators. Simplifies calling Python functions in

@gvanrossum gvanrossum Jan 18, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd name the new bytecode(s) explicitly here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Objects/frameobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Include/cpython/genobject.h
and coroutine objects. */
#define _PyGenObject_HEAD(prefix) \
PyObject_HEAD \
/* Note: gi_frame can be NULL if the generator is "finished" */ \

@gvanrossum gvanrossum Jan 18, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question about gi_iframe. Below (L31) it is defined as an array of length 1 of object pointers. But everywhere it's used, it is cast to InterpreterFrame *. That's not an object or object pointer AFAICT. So what's going on here? Is the declaration wrong? An intentional lie?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@markshannon markshannon Jan 19, 2022

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a lie.

We don't want to expose InterpreterFrame in public headers, so we can't do the sensible thing and declare gi_frame as:

    InterpreterFrame gi_frame;

as C won't allow incomplete types in structs, even as the last member.

We could improve this by breaking up the header, so the public API sees a different definition. Still a lie, but a more elegant one.
Public header:

typedef struct {
    _PyGenObject_HEAD(gi)
} PyGenObject;

Private header:

typedef struct {
    _PyGenObject_HEAD(gi)
    InterpreterFrame gi_frame;
} PyGenObject;

Probably best done in a different PR, though.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@gvanrossum gvanrossum Jan 19, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Any type other than PyObject* would be better though :-). And if you replace the casts with a macro it’s easier to fix later. I have an idea for the macro but it’s too painful to type on a phone.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@markshannon markshannon Jan 20, 2022

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first field of InterpreterFrame is PyFunctionObject * and the first few fields are all PyObject *, so declaring gi_frame as PyObject * gi_frame[1] seemed safe. Do let me know what your macro is, once you at a PC.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@gvanrossum gvanrossum Jan 21, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do let me know what your macro is, once you at a PC.

See https://bugs.python.org/issue40120#msg365465

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/dis.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/dis.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_generators.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Jan 19, 2022

Copy link
Copy Markdown
Member

PS. What do you mean by "coroutines are stackful"? (I was hoping that reading the code would clarify this, but either I didn't get to that point in the code yet or it's hidden by details.)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 19, 2022
View reviewed changes
Comment thread Objects/genobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 19, 2022
View reviewed changes
Comment thread Objects/genobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@markshannon
Revert change in test.
7392593
@markshannon
Remove unused macro.
17b453a
@markshannon

markshannon commented Jan 19, 2022

Copy link
Copy Markdown
Member Author

PS. What do you mean by "coroutines are stackful"? (I was hoping that reading the code would clarify this, but either I didn't get to that point in the code yet or it's hidden by details.)

Stackful coroutines have their own stack, like Lua coroutines or greenlets.
async def coroutines are "stackless", since they do not have their own stack.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@markshannon
markshannon force-pushed the make-gen-in-bytecode branch from e31703c to 17b453a Compare January 19, 2022 15:17
markshannon added 3 commits January 19, 2022 15:48
@markshannon
Add some clarifications and explanations.
baa1009
@markshannon
Name new instructions in news item.
44d3f47
@markshannon
Use Py_NewRef
06330b0
@markshannon
markshannon merged commit b04dfbb into python:main Jan 20, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Jan 20, 2022
@gvanrossum gvanrossum mentioned this pull request Jan 22, 2022
bpo-46470: remove unused branch from typing._remove_dups_flatten #30780
Merged
@tkren tkren mentioned this pull request Feb 26, 2023
gh-79940: add introspection API for asynchronous generators #11590
Merged
@andersk andersk mentioned this pull request May 16, 2023
With asynchronous exception raised, thread.is_alive() unexpectedly blocks in Python 3.11 with coverage coveragepy/coveragepy#1626
Open
@markshannon
markshannon deleted the make-gen-in-bytecode branch September 26, 2023 12:52
This was referenced Dec 19, 2025
The explanation of cr_suspended should be added to Types and members #142992
Closed
gi_suspended/cr_suspended is not specified as a change #142994
Closed
This was referenced Jan 8, 2026
[BUG] inspect.get*state() does not work for compiled objects on Python ≥3.11 cython/cython#7448
Open
types._GeneratorWrapper is no longer consistent with types.GeneratorType/types.CoroutineType #143552
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum left review comments
@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
@1st1 1st1 Awaiting requested review from 1st1

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@markshannon @bedevere-bot @gvanrossum @kumaraditya303 @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.