GeezerVison

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

GeezerVison is showing github.com — gh-148374: Fix a bug in `_Py_uop_sym_get_type` by Sacul0457 · Pull Request #148375 · python/cpython

Conversation

Sacul0457 commented Apr 11, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

@Fidget-Spinner gentle ping!

Issue: JIT: _Py_uop_sym_get_type returns PyGen_Type instead of NULL #148374

Sorry, something went wrong.

Uh oh!

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

All reactions


          Fix _Py_uop_sym_get_type and update test

cc75b3d

Sacul0457 requested review from Fidget-Spinner, markshannon, savannahostrowski and tomasr8 as code owners

April 11, 2026 11:35

bedevere-app Bot added the awaiting review label

Apr 11, 2026

bedevere-app Bot mentioned this pull request

Apr 11, 2026

JIT: _Py_uop_sym_get_type returns PyGen_Type instead of NULL #148374

Closed

Sacul0457 commented Apr 11, 2026

Copy link

Copy Markdown

Contributor Author

Also, would this need a news entry?

All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner reviewed

Apr 11, 2026

View reviewed changes

Comment thread

Python/optimizer_symbols.c Outdated

		_Py_uop_sym_set_recorded_gen_func(ctx, rg1, func);
		TEST_PREDICATE(_Py_uop_sym_matches_type(rg1, &PyGen_Type), "recorded gen func not treated as generator");
		TEST_PREDICATE(!_Py_uop_sym_matches_type(rg1, &PyGen_Type), "recorded gen func treated as generator");
		TEST_PREDICATE(_Py_uop_sym_get_probable_type(rg1) == NULL, "recorded gen func treated as generator");

Fidget-Spinner Apr 11, 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 be PyGen_Type, please edit the probable type function

Sorry, something went wrong.

Uh oh!

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

All reactions

Sacul0457 Apr 11, 2026

Copy link

Copy Markdown

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

Oh sorry, I just realised what you meant in #148372, one sec

Sorry, something went wrong.

Uh oh!

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

All reactions

Sacul0457 Apr 11, 2026

Copy link

Copy Markdown

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

fixed!

Sorry, something went wrong.

Uh oh!

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

All reactions

Fidget-Spinner commented Apr 11, 2026

Copy link

Copy Markdown

Member

I think this doesnt need one because the bug wont manifest on its own, as its always tied for FOR_ITER_GEN_FRAME which prior to your PR, was properly guarded

Sacul0457 reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

Sacul0457 added 2 commits

April 11, 2026 19:48


          edit _Py_uop_sym_get_probable_type and update test

c03a070


          forgot to add back the ifdef...

f9345a3

Fidget-Spinner reviewed

Apr 11, 2026

View reviewed changes

Comment thread

Python/optimizer_symbols.c Resolved

Uh oh!

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

Sacul0457 commented Apr 11, 2026

Copy link

Copy Markdown

Contributor Author

I just realised there's more tests I need to update

All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner reviewed

Apr 11, 2026

View reviewed changes

Comment thread

Python/optimizer_symbols.c

		_Py_uop_sym_set_recorded_gen_func(ctx, rg1, func);
		TEST_PREDICATE(_Py_uop_sym_matches_type(rg1, &PyGen_Type), "recorded gen func not treated as generator");
		TEST_PREDICATE(!_Py_uop_sym_matches_type(rg1, &PyGen_Type), "recorded gen func treated as generator");
		TEST_PREDICATE(_Py_uop_sym_get_probable_type(rg1) == &PyGen_Type, "recorded gen func not treated as generator");

Fidget-Spinner Apr 11, 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.

Seems like the test is failing. Not sure why. can you do some debugging please and check what type returns? You can do something like printf("TYPE: %s\n", rg1->tp_name); (assuming rg1 is not NULL).

Sorry, something went wrong.

Uh oh!

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

All reactions

Sacul0457 Apr 11, 2026

Copy link

Copy Markdown

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

I thought it was due to this test?

I have to go for a bit now, I'll upload the debug prints when i'm back

Sorry, something went wrong.

Uh oh!

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

All reactions

Sacul0457 Apr 11, 2026

Copy link

Copy Markdown

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

Okay, I think I understand what's going on.
Also, it seems like all the tests use rg1. Is that intended?

JitOptRef doesn't seem to have a tp_name member I'm guessing you meant something else?
In the meantime, I did this instead:

    JitOptRef rg1 = _Py_uop_sym_new_unknown(ctx);
    _Py_uop_sym_set_recorded_gen_func(ctx, rg1, func);
    TEST_PREDICATE(!_Py_uop_sym_matches_type(rg1, &PyGen_Type), "recorded gen func treated as generator");
    TEST_PREDICATE(_Py_uop_sym_get_probable_type(rg1) == &PyGen_Type, "rg1: recorded gen func not treated as generator");
    TEST_PREDICATE(_Py_uop_sym_get_const(ctx, rg1) == NULL, "recorded gen func is treated as known value");

    /* Test that setting type narrows correctly */

    JitOptRef rg2 = _Py_uop_sym_new_unknown(ctx);
    _Py_uop_sym_set_recorded_gen_func(ctx, rg2, func);
    _Py_uop_sym_set_type(ctx, rg2, &PyGen_Type);
    TEST_PREDICATE(!_Py_uop_sym_matches_type(rg2, &PyGen_Type), "rg2: recorded gen func not treated as generator");
    TEST_PREDICATE(_Py_uop_sym_get_const(ctx, rg2) == NULL, "known type is treated as known value");

    JitOptRef rg3 = _Py_uop_sym_new_unknown(ctx);
    _Py_uop_sym_set_recorded_gen_func(ctx, rg3, func);
    _Py_uop_sym_set_type_version(ctx, rg3, PyGen_Type.tp_version_tag);
    TEST_PREDICATE(!_Py_uop_sym_matches_type(rg3, &PyGen_Type), "rg3: recorded gen func not treated as generator");
    TEST_PREDICATE(_Py_uop_sym_get_const(ctx, rg3) == NULL, "recorded value with type is treated as known");

Output:

Running Debug|x64 interpreter...
F.....
======================================================================
FAIL: test_optimizer_symbols (test.test_optimizer.TestOptimizerSymbols.test_optimizer_symbols)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Sacul Dev Stuff\Cpython - Contributing\cpython\Lib\test\test_optimizer.py", line 86, in test_optimizer_symbols
    _testinternalcapi.uop_symbols_test()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
AssertionError: rg2: recorded gen func not treated as generator


Running Debug|x64 interpreter...
F.....
======================================================================
FAIL: test_optimizer_symbols (test.test_optimizer.TestOptimizerSymbols.test_optimizer_symbols)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Sacul Dev Stuff\Cpython - Contributing\cpython\Lib\test\test_optimizer.py", line 86, in test_optimizer_symbols
    _testinternalcapi.uop_symbols_test()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
AssertionError: rg3: recorded gen func not treated as generator

Sorry, something went wrong.

Uh oh!

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

All reactions

Fidget-Spinner Apr 11, 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.

Oh yeah you're right. Please amend the other tests.

Sorry, something went wrong.

Uh oh!

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

Sacul0457 reacted with thumbs up emoji

All reactions

👍 1 reaction


          use rg2 and rg3 instead of rg1 where appropriate

d848083

Fidget-Spinner added the skip news label

Apr 11, 2026

Fidget-Spinner reviewed

Apr 11, 2026

View reviewed changes

Comment thread

Python/optimizer_symbols.c

		_Py_uop_sym_set_recorded_gen_func(ctx, rg2, func);
		_Py_uop_sym_set_type(ctx, rg2, &PyGen_Type);
		TEST_PREDICATE(_Py_uop_sym_matches_type(rg1, &PyGen_Type), "recorded gen func not treated as generator");
		TEST_PREDICATE(!_Py_uop_sym_matches_type(rg2, &PyGen_Type), "recorded gen func treated as generator");

Fidget-Spinner Apr 11, 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.

Please add for this test and below the converse as well

TEST_PREDICATE(_Py_uop_sym_get_probable_type(rg2) == &PyGen_Type, "recorded gen func not treated as generator");

Sorry, something went wrong.

Uh oh!

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

All reactions

Sacul0457 Apr 11, 2026

Copy link

Copy Markdown

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

added 👍

Sorry, something went wrong.

Uh oh!

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

All reactions


          add more tests

c76db65

Fidget-Spinner approved these changes

Apr 11, 2026

View reviewed changes

Fidget-Spinner 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.

Thanks

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-app Bot added awaiting merge and removed awaiting review labels

Apr 11, 2026

Fidget-Spinner merged commit 83f33dc into python:main

Apr 11, 2026

73 checks passed

Uh oh!

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

bedevere-app Bot removed the awaiting merge label

Apr 11, 2026

Sacul0457 deleted the Fix-_Py_uop_sym_get_type branch

April 11, 2026 17:41

ljfp pushed a commit to ljfp/cpython that referenced this pull request

Apr 25, 2026


          pythongh-148374: Fix a bug in _Py_uop_sym_get_type (pythonGH-148375)

e4aac3e

Reviewers

Fidget-Spinner

Fidget-Spinner approved these changes

markshannon

Awaiting requested review from markshannon

markshannon is a code owner

tomasr8

Awaiting requested review from tomasr8

tomasr8 is a code owner

savannahostrowski

Awaiting requested review from savannahostrowski

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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

Conversation

Sacul0457 commented Apr 11, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sacul0457 commented Apr 11, 2026

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!

Fidget-Spinner commented Apr 11, 2026

Uh oh!

Uh oh!

Sacul0457 commented Apr 11, 2026

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!

Choose a reason for hiding this comment

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Sacul0457 commented Apr 11, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

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