GeezerVison

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

GeezerVison is showing github.com — gh-148874: add flag to `CALL` to skip periodic check in `with` by NekoAsakura · Pull Request #149019 · python/cpython

Conversation

NekoAsakura commented Apr 26, 2026 •

edited by github-actions Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

Issue: with lock: can skip __exit__ when a signal handler raises right after __enter__ #148874

📚 Documentation preview 📚: https://cpython-previews--149019.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

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

All reactions


          pythongh-148874: add flag to CALL to skip periodic check in with

34535ba

NekoAsakura requested review from Fidget-Spinner, ericsnowcurrently, iritkatriel, markshannon, savannahostrowski and tomasr8 as code owners

April 26, 2026 16:38

bedevere-app Bot added the awaiting review label

Apr 26, 2026

bedevere-app Bot mentioned this pull request

Apr 26, 2026

with lock: can skip __exit__ when a signal handler raises right after __enter__ #148874

Open

NekoAsakura added 2 commits

April 26, 2026 13:21


          fixup

cceb8db


          Merge branch 'main' into pythongh-148874/with-signal-exit-skip

6a07b77

markshannon reviewed

Apr 28, 2026

View reviewed changes

markshannon 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.

This looks good in general, but I've a few comments.

I'd prefer explicit oparg >> 1 and oparg & 1 so the tools can more easily understand offsets.

Sorry, something went wrong.

Uh oh!

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

NekoAsakura reacted with heart emoji

All reactions

❤️ 1 reaction

Comment thread

Doc/library/dis.rst Outdated

		Calls with keyword arguments are now handled by :opcode:`CALL_KW`.

		.. versionchanged:: next
		``argc`` is now the oparg shifted right by one.

markshannon Apr 28, 2026

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.

Also explain what the low bit is for.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Include/internal/pycore_opcode_utils.h Outdated

		#define RESUME_OPARG_LOCATION_MASK 0x7
		#define RESUME_OPARG_DEPTH1_MASK 0x8

		#define CALL_OPARG_SKIP_PENDING_MASK 0x1

markshannon Apr 28, 2026

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.

Can you avoid using macros (or inline functions) for this?
This is sp that our tools can understand the offsets in calls.
The tools can (with a bit of modification) understand oparg >> 1, but CALL_ARGCOUNT will require special casing CALL_ARGCOUNT making it more complex.

(The reason this matters is that we will want to convert fixes sized arrays to scalars in the code generator in the future)

Also, comments anywhere there is masking or shifting would be clearer, IMO, than opaque macros.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Lib/dis.py Outdated

		elif deop in (CALL, INSTRUMENTED_CALL):
		argval = arg >> 1
		if arg & 1:
		argrepr = f"{argval} + skip_pending"

markshannon Apr 28, 2026

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.

We use the term "no interrupt" for jumps, so could you use that here for consistency?

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Python/bytecodes.c Outdated

		ERROR_IF(err != 0);
		}

		replaced op(_CHECK_PERIODIC_AT_END_PENDING, (--)) {

markshannon Apr 28, 2026

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.

Suggested change

	replaced op(_CHECK_PERIODIC_AT_END_PENDING, (--)) {
	replaced op(_CHECK_PERIODIC_IF_INTERRUPTIBLE, (--)) {

The meaning of _CHECK_PERIODIC_AT_END_PENDING isn't clear.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Python/optimizer.c Outdated

		[_FOR_ITER_VIRTUAL] = _FOR_ITER_VIRTUAL_TIER_TWO,
		[_ITER_NEXT_LIST] = _ITER_NEXT_LIST_TIER_TWO,
		[_CHECK_PERIODIC_AT_END] = _TIER2_RESUME_CHECK,
		[_CHECK_PERIODIC_AT_END_PENDING] = _TIER2_RESUME_CHECK,

markshannon Apr 28, 2026

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.

This should either be replaced by _NOP if oparg & 1 is 0 or _CHECK_PERIODIC_AT_END if oparg & 1 is 1 in the front-end, so I wouldn't expect it to appear here.

We do much the same with _PUSH_NULL_CONDITIONAL, converting it to _NOP or _PUSH_NULL

Sorry, something went wrong.

Uh oh!

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

All reactions


          address review feedback

658c05f

read-the-docs-community Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #32893794 | 📁 Comparing 62bd5bb against main (5d41632)

🔍 Preview build

126 files changed · + 4 added · ± 122 modified

+ Added

± Modified

All reactions

Sorry, something went wrong.

Uh oh!

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


          Merge branch 'main' into pythongh-148874/with-signal-exit-skip

0237f8e

NekoAsakura commented Apr 28, 2026

Copy link

Copy Markdown

Member Author

Thanks for the review <3

All reactions

Sorry, something went wrong.

Uh oh!

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

NekoAsakura commented Apr 29, 2026

Copy link

Copy Markdown

Member Author

A bit of clash with #148831, having a look.

All reactions

Sorry, something went wrong.

Uh oh!

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

markshannon commented Apr 29, 2026

Copy link

Copy Markdown

Member

A bit of clash with #148831, having a look.

Probably the magic number. You'll need to merge in main then increase it again.

All reactions

Sorry, something went wrong.

Uh oh!

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


          Merge branch 'main' into pythongh-148874/with-signal-exit-skip

7262c62

NekoAsakura commented Apr 29, 2026

Copy link

Copy Markdown

Member Author

Yep was fixed in #149121

All reactions

Sorry, something went wrong.

Uh oh!

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

markshannon self-requested a review

April 30, 2026 10:36

markshannon requested changes

Apr 30, 2026

View reviewed changes

markshannon 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.

Generally looks good.

I don't we need a new uop, changing _CHECK_PERIODIC_AT_END should be sufficient.

Sorry, something went wrong.

Uh oh!

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

NekoAsakura reacted with heart emoji

All reactions

❤️ 1 reaction

Comment thread

Doc/library/dis.rst Resolved

Uh oh!

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

Comment thread

Python/bytecodes.c Resolved

Uh oh!

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

Comment thread

Include/internal/pycore_magic_number.h Resolved

Uh oh!

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

bedevere-app Bot removed the awaiting review label

Apr 30, 2026

bedevere-app Bot commented Apr 30, 2026

Copy link

Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-app Bot added the awaiting changes label

Apr 30, 2026

NekoAsakura and others added 4 commits

April 30, 2026 07:20


          Update Doc/library/dis.rst

40fc50c

Co-authored-by: Mark Shannon <[email protected]>


          Merge branch 'main' into pythongh-148874/with-signal-exit-skip

8fc75f0


          Merge branch 'main' into pythongh-148874/with-signal-exit-skip

e8e3d32


          Merge branch 'main' into pythongh-148874/with-signal-exit-skip

55f1981

markshannon self-requested a review

May 28, 2026 14:03

NekoAsakura added 2 commits

May 28, 2026 12:42


          Merge branch 'main' into pythongh-148874/with-signal-exit-skip

236d271


          Merge branch 'main' into pythongh-148874/with-signal-exit-skip

62bd5bb

NekoAsakura commented Jul 7, 2026

Copy link

Copy Markdown

Member Author

Closing as #150911 has landed.

All reactions

Sorry, something went wrong.

Uh oh!

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

NekoAsakura closed this

Jul 7, 2026

NekoAsakura deleted the gh-148874/with-signal-exit-skip branch

July 7, 2026 15:42

Reviewers

tomasr8

Awaiting requested review from tomasr8

tomasr8 is a code owner

Fidget-Spinner

Awaiting requested review from Fidget-Spinner

Fidget-Spinner is a code owner

savannahostrowski

Awaiting requested review from savannahostrowski

ericsnowcurrently

Awaiting requested review from ericsnowcurrently

ericsnowcurrently is a code owner

iritkatriel

Awaiting requested review from iritkatriel

iritkatriel is a code owner

markshannon

Awaiting requested review from markshannon

markshannon is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

awaiting changes

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.

2 participants

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 01402520 — 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.

GeezerVison

Conversation

NekoAsakura commented Apr 26, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

markshannon Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

markshannon Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

markshannon Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

markshannon Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

markshannon Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

read-the-docs-community Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation build overview

Uh oh!

NekoAsakura commented Apr 28, 2026

Uh oh!

NekoAsakura commented Apr 29, 2026

Uh oh!

markshannon commented Apr 29, 2026

Uh oh!

NekoAsakura commented Apr 29, 2026

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-app Bot commented Apr 30, 2026

Uh oh!

NekoAsakura commented Jul 7, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NekoAsakura commented Apr 26, 2026 •

edited by github-actions Bot

Loading

Uh oh!

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

read-the-docs-community Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

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