Skip to content

GH-149501: Fix compilation warning for _YIELD_VALUE#149502

Open
sergey-miryanov wants to merge 11 commits into
python:mainfrom
sergey-miryanov:bug/149501-fix-msvc-warning
Open

GH-149501: Fix compilation warning for _YIELD_VALUE#149502
sergey-miryanov wants to merge 11 commits into
python:mainfrom
sergey-miryanov:bug/149501-fix-msvc-warning

Conversation

@sergey-miryanov
Copy link
Copy Markdown
Contributor

@sergey-miryanov sergey-miryanov commented May 7, 2026

@sergey-miryanov
Copy link
Copy Markdown
Contributor Author

cc @vstinner as an author of the change

@sergey-miryanov sergey-miryanov force-pushed the bug/149501-fix-msvc-warning branch from f3eadda to 0cb3e7d Compare May 7, 2026 17:02
@vstinner
Copy link
Copy Markdown
Member

vstinner commented May 7, 2026

Would it be possible to change i type to size_t or Py_ssize_t instead?

Comment thread Include/internal/pycore_code.h
Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Sorry, but in fact I prefer the initial fix: int i = (int)(frame->instr_ptr - _PyFrame_GetBytecode(frame));.

I don't think that changing _Py_GetBaseCodeUnit() parameter type from int to Py_ssize_t is worth it, since PyCode_New() checks indirectly that the size fits into an int:

    if (PyBytes_GET_SIZE(con->code) > INT_MAX) {
        PyErr_SetString(PyExc_OverflowError,
                        "code: co_code larger than INT_MAX");
        return -1;
    }

@sergey-miryanov
Copy link
Copy Markdown
Contributor Author

@vstinner
I've updated the intermediate value to use Py_ssize_t and added an assertion to ensure it fits in an int before casting. What do you think? Does this approach look good?

Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Yes, I'm fine with the assert() approach.

Comment thread Python/bytecodes.c Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants