bpo-38500: Add _PyInterpreterState_SetEvalFrameFunc()

[vstinner]

PyInterpreterState.eval_frame function now requires a tstate (Python
thread state) parameter.

Add private functions to the C API to get and set the frame
evaluation function:

• Add tstate parameter to _PyFrameEvalFunction function type.
• Add _PyInterpreterState_GetEvalFrameFunc() and
  _PyInterpreterState_SetEvalFrameFunc() functions.
• Add tstate parameter to _PyEval_EvalFrameDefault().

https://bugs.python.org/issue38500

[brettcannon]

I'm holding off on looking at this until the tstate question is resolved.

[ericsnowcurrently]

The change itself LGTM. However, I'm still not convinced that this is functionality we want to expose through the public API. So that question should be resolved before merging this. :)

(@brettcannon The question of passing tstate to the eval function seems unrelated.)

[ericsnowcurrently]

Also, it would probably make sense to have an explicit mention of this in whatsnew doc (porting section?).

[brettcannon]

@ericsnowcurrently I think the tstate discussion is related because @vstinner discussed changing the definition of PyFrameEvalFunction which is being explicitly dealt with here.

[vstinner]

Python 3.8.0 is now released. The internal C API can be used to access interp->eval_frame. I no longer want to backport this change to Python 3.8.

[vstinner]

I rebased my PR on master. I merged PR #17187 ("bpo-38818: PyInterpreterState.eval_frame now pass tstate #17187") changes into this PR.

[vstinner]

I plan to merge my PR 17340 at the end of week to not miss Python 3.9 feature freeze deadline, unless someone speaks up and find a good reason to not merge this PR. The PR adds a public C API PyInterpreterState_SetEvalFrameFunc() and now pass tstate to the frame evaluation function.

Please go to https://bugs.python.org/issue38500 for discussion, don't discuss the topic here.

[vstinner]

@DinoV @ericsnowcurrently @brettcannon: Would you mind to review the update PR?

[vstinner]

The Windows ARM job of Azure Pipelines PR failed on downloading dependencies ("urllib.error.HTTPError: HTTP Error 401: UNAUTHORIZED"). I will wait for reviews before restarting the job. Right now, I don't need a green CI, I don't plan to merge this PR before the end of the week.

[vstinner]

@rsumner868: Hi, I saw you approving a few PRs. Would you mind to elaborate what you reviewed and explain why/how you approve a PR? This PR is controversial, you cannot simply approve it with no message.

[markshannon]

As I've reiterated in https://bugs.python.org/issue38500, I am strongly opposed to this being merged.

[vstinner]

I took @markshannon's concerns in account and I updated my PR to only add private functions to the C API, not public functions.

[markshannon]

@vstinner Thank you for making these functions private.

However, if they are documented in the Doc/c-api/init.rst, then they aren't that private.
Could you remove the documentation?
These functions have the potential to hurt performance by a significant amount, so hiding them (or warning that might well be removed in 3.10) would be good.

[vstinner]

However, if they are documented in the Doc/c-api/init.rst, then they aren't that private. Could you remove the documentation?

I prefer to document them. The feature is part of the PEP 523 and we already had this discussion :-)