gh-148874: add flag to `CALL` to skip periodic check in `with` by NekoAsakura · Pull Request #149019 · python/cpython

NekoAsakura · 2026-04-26T16:38:21Z

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

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

markshannon

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.

markshannon · 2026-04-28T14:51:11Z

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

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


Also explain what the low bit is for.

markshannon · 2026-04-28T14:56:13Z

 #define RESUME_OPARG_LOCATION_MASK 0x7
 #define RESUME_OPARG_DEPTH1_MASK 0x8

+#define CALL_OPARG_SKIP_PENDING_MASK 0x1


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.

markshannon · 2026-04-28T14:57:31Z

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


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

markshannon · 2026-04-28T15:02:26Z

+            ERROR_IF(err != 0);
+        }
+
+        replaced op(_CHECK_PERIODIC_AT_END_PENDING, (--)) {


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.

markshannon · 2026-04-28T15:06:04Z

    [_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,


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

read-the-docs-community · 2026-04-28T20:01:12Z

Documentation build overview

📚 cpython-previews | 🛠️ Build #32483173 | 📁 Comparing 40fc50c against main (5d41632)

🔍 Preview build

33 files changed · + 1 added · ± 32 modified

+ Added

c-api/sentinel.html

± Modified

NekoAsakura · 2026-04-28T20:14:02Z

Thanks for the review <3

NekoAsakura · 2026-04-29T01:38:21Z

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

markshannon · 2026-04-29T09:57:34Z

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

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

NekoAsakura · 2026-04-29T10:03:17Z

Yep was fixed in #149121

markshannon

Generally looks good.

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

markshannon · 2026-04-30T10:42:36Z

            ERROR_IF(err != 0);
        }

+        op(_CHECK_PERIODIC_IF_INTERRUPTIBLE, (--)) {


I don't think you need a new uop. The if (oparg & 1) { can just be inserted in _CHECK_PERIODIC_AT_END

Just to make sure, _CHECK_PERIODIC_AT_END is also used by CALL_FUNCTION_EX, CALL_EX_NON_PY_GENERAL, INSTRUMENTED_CALL_FUNCTION_EX, CALL_KW_NON_PY.

markshannon · 2026-04-30T10:47:29Z

 */

-#define PYC_MAGIC_NUMBER 3665
+#define PYC_MAGIC_NUMBER 3666


This might need bumping again

I think I already bumped it. Current upstream HEAD:

cpython/Include/internal/pycore_magic_number.h

Line 312 in 4599335

#define PYC_MAGIC_NUMBER 3665

bedevere-app · 2026-04-30T10:47:39Z

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.

Co-authored-by: Mark Shannon <Mark.Shannon@arm.com>

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

address review feedback

658c05f

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

0237f8e

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

7262c62

markshannon self-requested a review April 30, 2026 10:36

markshannon requested changes Apr 30, 2026

View reviewed changes

bedevere-app Bot removed the awaiting review label Apr 30, 2026

bedevere-app Bot added the awaiting changes label Apr 30, 2026

Update Doc/library/dis.rst

40fc50c

Co-authored-by: Mark Shannon <Mark.Shannon@arm.com>

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

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NekoAsakura commented Apr 26, 2026 •

edited by github-actions Bot

Loading

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

edited

Loading