Skip to content

gh-148374: Fix a bug in _Py_uop_sym_get_type#148375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Fidget-Spinner merged 5 commits into from
Apr 11, 2026
Merged

gh-148374: Fix a bug in _Py_uop_sym_get_type#148375

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc75b3d
Fix _Py_uop_sym_get_type and update test
Sacul0457 Apr 11, 2026
c03a070
edit _Py_uop_sym_get_probable_type and update test
Sacul0457 Apr 11, 2026
f9345a3
forgot to add back the ifdef...
Sacul0457 Apr 11, 2026
d848083
use rg2 and rg3 instead of rg1 where appropriate
Sacul0457 Apr 11, 2026
c76db65
add more tests
Sacul0457 Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions Python/optimizer_symbols.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -783,6 +783,7 @@ _Py_uop_sym_get_type(JitOptRef ref)
case JIT_SYM_NON_NULL_TAG:
case JIT_SYM_UNKNOWN_TAG:
case JIT_SYM_RECORDED_TYPE_TAG:
case JIT_SYM_RECORDED_GEN_FUNC_TAG:
return NULL;
case JIT_SYM_RECORDED_VALUE_TAG:
if (sym->recorded_value.known_type) {
Expand All@@ -804,8 +805,6 @@ _Py_uop_sym_get_type(JitOptRef ref)
return &PyBool_Type;
case JIT_SYM_COMPACT_INT:
return &PyLong_Type;
case JIT_SYM_RECORDED_GEN_FUNC_TAG:
return &PyGen_Type;
}
Py_UNREACHABLE();
}
Expand All@@ -830,7 +829,7 @@ _Py_uop_sym_get_probable_type(JitOptRef ref)
case JIT_SYM_KNOWN_VALUE_TAG:
return _Py_uop_sym_get_type(ref);
case JIT_SYM_RECORDED_GEN_FUNC_TAG:
return NULL;
return &PyGen_Type;
case JIT_SYM_RECORDED_VALUE_TAG:
return Py_TYPE(sym->recorded_value.value);
case JIT_SYM_RECORDED_TYPE_TAG:
Expand DownExpand Up@@ -2211,21 +2210,24 @@ _Py_uop_symbols_test(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(ignored))
/* Test that recorded type aren't treated as known values*/
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 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-SpinnerFidget-SpinnerApr 11, 2026

Copy link
Copy Markdown
Member

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

@Sacul0457Sacul0457Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

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

@Sacul0457Sacul0457Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

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:

JitOptRefrg1=_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 */JitOptRefrg2=_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");
JitOptRefrg3=_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:

RunningDebug|x64interpreter...
F.....
======================================================================FAIL: test_optimizer_symbols (test.test_optimizer.TestOptimizerSymbols.test_optimizer_symbols)
----------------------------------------------------------------------Traceback (mostrecentcalllast):
File"C:\Sacul Dev Stuff\Cpython - Contributing\cpython\Lib\test\test_optimizer.py", line86, intest_optimizer_symbols_testinternalcapi.uop_symbols_test()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^AssertionError: rg2: recordedgenfuncnottreatedasgeneratorRunningDebug|x64interpreter...
F.....
======================================================================FAIL: test_optimizer_symbols (test.test_optimizer.TestOptimizerSymbols.test_optimizer_symbols)
----------------------------------------------------------------------Traceback (mostrecentcalllast):
File"C:\Sacul Dev Stuff\Cpython - Contributing\cpython\Lib\test\test_optimizer.py", line86, intest_optimizer_symbols_testinternalcapi.uop_symbols_test()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^AssertionError: rg3: recordedgenfuncnottreatedasgenerator

@Fidget-SpinnerFidget-SpinnerApr 11, 2026

Copy link
Copy Markdown
Member

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.

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(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-SpinnerFidget-SpinnerApr 11, 2026

Copy link
Copy Markdown
Member

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");

@Sacul0457Sacul0457Apr 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added 👍

TEST_PREDICATE(_Py_uop_sym_get_probable_type(rg2) == &PyGen_Type, "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(rg1, &PyGen_Type), "recorded gen func not treated as generator");
TEST_PREDICATE(!_Py_uop_sym_matches_type(rg3, &PyGen_Type), "recorded gen func treated as generator");
TEST_PREDICATE(_Py_uop_sym_get_probable_type(rg3) == &PyGen_Type, "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");

/* Test contradictions */
Expand Down
Loading