FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-148374: Fix a bug in `_Py_uop_sym_get_type` by Sacul0457 · Pull Request #148375 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

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
Conversation 13 Commits 5 Checks 71 Files changed
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 .c  (1) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
14 changes: 8 additions & 6 deletions Python/optimizer_symbols.c
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff 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 Down Expand 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-Spinner Apr 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Sacul0457 Apr 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Sacul0457 Apr 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Sacul0457 Apr 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Back | FazBrowse Home | New Git URL