Skip to content

gh-148829: Implement PEP 661#148831

Open
JelleZijlstra wants to merge 12 commits intopython:mainfrom
JelleZijlstra:pep661
Open

gh-148829: Implement PEP 661#148831
JelleZijlstra wants to merge 12 commits intopython:mainfrom
JelleZijlstra:pep661

Conversation

@JelleZijlstra
Copy link
Copy Markdown
Member

@JelleZijlstra JelleZijlstra commented Apr 21, 2026

Comment thread Objects/sentinelobject.c Outdated
Comment thread Lib/typing.py
Comment thread Objects/sentinelobject.c Outdated
Comment thread Objects/sentinelobject.c Outdated
Comment thread Objects/sentinelobject.c Outdated
Comment thread Objects/sentinelobject.c Outdated
Comment on lines +39 to +44
PyObject *r = PyFunction_GetModule(PyStackRef_AsPyObjectBorrow(f->f_funcobj));
if (!r) {
PyErr_Clear();
Py_RETURN_NONE;
}
return Py_NewRef(r);
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.

I don't think PyFunction_GetModule can fail here, because f_funcobj shouldn't ever be anything other than a function object. We can use _PyFrame_GetFunction to assert this:

Suggested change
PyObject *r = PyFunction_GetModule(PyStackRef_AsPyObjectBorrow(f->f_funcobj));
if (!r) {
PyErr_Clear();
Py_RETURN_NONE;
}
return Py_NewRef(r);
PyObject *r = PyFunction_GetModule(_PyFrame_GetFunction(f));
if (!r) {
Py_RETURN_NONE;
}
return Py_NewRef(r);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hm, do we even need the null check for funcobj on the line above? Not sure it can ever be NULL.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll keep that for paranoia. Also keeping an assertion that it really is a PyFunctionObject.

Comment thread Objects/sentinelobject.c
Comment thread Objects/sentinelobject.c
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.

3 participants