gh-141518: Add PyUnstable_InterpreterState_SetEvalFrameFunc()

[vstinner]

Add a PyUnstable API for PEP 523:

• PyUnstable_FrameEvalFunction type
• PyUnstable_InterpreterState_GetEvalFrameFunc()
• PyUnstable_InterpreterState_SetEvalFrameFunc()

Keep the old names as deprecated aliases to new names:

• _PyFrameEvalFunction
• _PyInterpreterState_GetEvalFrameFunc
• _PyInterpreterState_SetEvalFrameFunc

• Issue: [C API] Promote PEP 523 functions to public functions: add PyUnstable_InterpreterState_SetEvalFrameFunc() #141518

[efimov-mikhail]

Do we need tests on PyUnstable API?

[vstinner]

Do we need tests on PyUnstable API?

There are tests in test_optimizer: see _testinternalcapi.set_eval_frame_default().

[efimov-mikhail]

Do we need tests on PyUnstable API?

There are tests in test_optimizer: see _testinternalcapi.set_eval_frame_default().

Thanks!
But it's only on PyUnstable_InterpreterState_SetEvalFrameFunc.
Maybe we want to test PyUnstable_InterpreterState_GetEvalFrameFunc a little?

[vstinner]

Maybe we want to test PyUnstable_InterpreterState_GetEvalFrameFunc a little?

I added tests.

[vstinner]

cc @emmatyping

[emmatyping]

One question but otherwise looks good!

[vstinner]

I created a C API Working Group decision issue.

[encukou]

Doesn't _PyInterpreterFrame, need a public name too? Otherwise you can't quite use the functions without private API.

[vstinner]

To implement an eval function, you should likely use the internal C API (pycore_interpframe.h) to access frame members, you're right. I don't want to add _PyInterpreterFrame structure members to the PyUnstable API, it's too unstable :-D I prefer to leave it in the internal C API.

I don't think that it's worth it to expose struct _PyInterpreterFrame* type in the PyUnstable API. What do you think?

[encukou]

These functions are rather useless without making _PyInterpreterFrame public. At least should be an opaque type, with a function to upgrade it to the full PyFrameObject.

[vstinner]

The API changed in Python 3.11 to take a _PyInterpreterFrame* instead of a PyFrameObject*. It's basically only used by PyTorch Dynamo which already uses the internal C API:

https://github.com/pytorch/pytorch/blob/be33b7faf685560bb618561b44b751713a660337/torch/csrc/dynamo/eval_frame.c#L15-L17

I wouldn't say that this API is useless without a public API for _PyInterpreterFrame*. It's just "unusual" :-)

At least should be an opaque type, with a function to upgrade it to the full PyFrameObject.

It would make the code (way) slower, _PyInterpreterFrame* idea is to avoid creating a concrete Python object for a frame unless it's strictly needed. I suggest using the internal C API for _PyInterpreterFrame* instead.

[encukou]

I wouldn't say that this API is useless without a public API for _PyInterpreterFrame*.

What is the use, then?

[vstinner]

@emmatyping @efimov-mikhail: Do you have an opinion on these questions?

[efimov-mikhail]

IMO, the main idea of this change is not really to make those functions public. We mark them as PyUnstable_ for sending clear message to its users: "we remember about those function, we don't want to change or remove them in the near future". One can use them and rely on their presence in C API, but can't rely on stability of _PyInterpreterFrame structure. So, there's a need to use some private headers in this case.

I agree that it's not the perfect position for us, but current situation isn't any better.
We have functions that both private and public.
They're private, because they have _Py* names.
But they're "kinda public", since PEP 523 is guaranteed their presence.
PyUnstable_ suits better for those semi-public functions.
But making _PyInterpreterFrame and all its members PyUnstable_ too seems redundant to me.