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-43683: Handle generator entry in bytecode by markshannon · Pull Request #25138 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Apr 1, 2021 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Adds GEN_START bytecode to handle checking for None when starting a generator or coroutine.

This means that:

  • send() doesn't need to check the state of the generator.
  • Should speed up iteration as check is only performed once per generator, not once per iteration.

https://bugs.python.org/issue43683

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon
markshannon requested a review from 1st1 as a code owner April 1, 2021 15:23
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Apr 1, 2021
@bedevere-bot bedevere-bot added the awaiting core review label Apr 1, 2021
@markshannon
markshannon force-pushed the handle-generator-entry-in-bytecode branch from 4dff1d0 to 46c71f5 Compare April 2, 2021 10:42
willingc
willingc reviewed Apr 3, 2021
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Contributor

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/Core and Builtins/2021-04-01-12-30-30.bpo-43683.AjxOx2.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Core and Builtins/2021-04-01-12-30-30.bpo-43683.AjxOx2.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
markshannon added 2 commits April 6, 2021 10:52
@markshannon
Handle check for sending None to starting generator and coroutine int…
3a41a50 
…o bytecode.

Document new bytecode and make it fail gracefully if mis-compiled.

Add NEWS item

Bump magic number.
@markshannon
Improve wording in NEWS item.
1a29ec3
@markshannon
markshannon force-pushed the handle-generator-entry-in-bytecode branch from 46c71f5 to 1a29ec3 Compare April 6, 2021 09:56
@markshannon
markshannon merged commit b37181e into python:master Apr 6, 2021
@bedevere-bot bedevere-bot removed the awaiting core review label Apr 6, 2021
@markshannon markshannon mentioned this pull request Apr 6, 2021
Add bytecode to handle gen.send() corner cases faster-cpython/ideas#38
Closed
gvanrossum
gvanrossum reviewed Apr 6, 2021
View reviewed changes
Comment thread Lib/importlib/_bootstrap_external.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
gvanrossum
gvanrossum reviewed Apr 6, 2021
View reviewed changes
Comment thread Python/compile.c
entryblock = b;
}

if (insert_generator_prefix(c, entryblock)) {

@gvanrossum gvanrossum Apr 6, 2021

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.

If we're compiling a generator (or coroutine etc.) the new function assumes (without asserting) that entryblock is not NULL. But evidently it could be, since below on line 6690 there's a check whether it could be. I don't know how it could be NULL - that would mean there's no code in the function at all, but it seems there's code above that always adds at least one instruction (RETURN_VALUE on line 6668). So maybe the check for entryblock == NULL below can be replaced by an assert? (I'd place that assert right underneath the for-loop above on lines 6683-6686.

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon Apr 6, 2021

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.

entryblock cannot be NULL. I'll add the assert.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@python python deleted a comment from fochoao Apr 6, 2021
@markshannon

markshannon commented Apr 6, 2021

Copy link
Copy Markdown
Member Author

Fixes are in #25224
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon
markshannon deleted the handle-generator-entry-in-bytecode branch April 14, 2021 13:27
@markshannon markshannon mentioned this pull request Sep 13, 2022
Handle generator (and coroutine) state in the bytecode. #87849
Closed
@tekknolagi tekknolagi mentioned this pull request Jun 12, 2023
Implement PEP 479 in bytecode tekknolagi/skybison#507
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@willingc willingc willingc left review comments
@gvanrossum gvanrossum gvanrossum 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 @willingc @gvanrossum @the-knights-who-say-ni @bedevere-bot
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.