Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Apr 8, 2022

@vstinner
Copy link
Member Author

vstinner commented Apr 8, 2022

@markshannon: I started to review #32413 before seeing that it was already merged 3 hours ago, aaaah :-) So I created a PR instead.

I don't think that it's useful to mention that the function argument must not be NULL. We don't do that in the doc of other C API functions. It's implicit.

@vstinner
Copy link
Member Author

vstinner commented Apr 8, 2022

In terms of API, PyFrame_GetLasti() returns an int. Would it make sense to return Py_ssize_t?

The code constructor makes sure that co_code cannot be larger than INT_MAX:

    /* Make sure that code is indexable with an int, this is
       a long running assumption in ceval.c and many parts of
       the interpreter. */
    if (PyBytes_GET_SIZE(con->code) > INT_MAX) {
        PyErr_SetString(PyExc_OverflowError,
                        "code: co_code larger than INT_MAX");
        return -1;
    }

@iritkatriel
Copy link
Member

https://bugs.python.org/issue40421 is closed. What is the status of this PR?

@markshannon markshannon reopened this Apr 18, 2022
@markshannon
Copy link
Member

Thanks @vstinner
Looks good to me.

@iritkatriel This is just a minor tidy up, https://bugs.python.org/issue40421 can remain closed.

@markshannon
Copy link
Member

Would it make sense to return Py_ssize_t?

I don't think so.

@vstinner vstinner merged commit aa5c0a9 into python:main Apr 19, 2022
@vstinner vstinner deleted the frame branch April 19, 2022 07:53
@vstinner
Copy link
Member Author

Merged, thanks for the review @markshannon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants