-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
bpo-38818: PyInterpreterState.eval_frame now pass tstate #17187
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.
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
Conversation
PyInterpreterState.eval_frame (PEP 523) now requires a new mandatory tstate parameter (PyThreadState*).
This change is backward incompatible and changes the PEP 523. I would prefer to have it carefully reviewed ;-) |
In order to keep backward compatibility, can we use a new function to do this action? |
PyInterpreterState.eval_frame is a structure field. What do you mean? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can happen from the contributors who are not familiar with the codebase.
@shihai1991 I think that the snippet I attach links below will help your future contribution.
Line 25 in 2bc43cd
typedef struct _is PyInterpreterState; |
cpython/Include/internal/pycore_pystate.h
Line 109 in 9def81a
_PyFrameEvalFunction eval_frame; |
@vstinner
Looks good to me.
Sorry, i didn't express cleary. I just worried about whether we have external user use |
frame_eval is specified by https://www.python.org/dev/peps/pep-0523/ The question if it should be public or not is discussed at https://bugs.python.org/issue38500 At least, the API (frame_eval) is used by PyCharm to implement their debugger ;-) |
I don't think this should go in until bpo-38500 is resolved. |
I included this change into PR #17340. I close this PR. |
PyInterpreterState.eval_frame (PEP 523) now requires a new mandatory
tstate parameter (PyThreadState*).
https://bugs.python.org/issue38818