[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

> Posted 
> https://mail.python.org/archives/list/python-...@python.org/thread/4UZJYAZL3NHRAGN5WAMJC4IHAHEXF3QF/
>  to see if anyone else wants to weigh in.

Mark Shannon listed flaws in the PEP 532 and suggest to withdraw this PEP. 
Honestly, I'm open to any solution. But this issue is a concrete regression of 
Python 3.8 with a concrete use case, whereas Mark only lists theoretical 
enhancements.

I would like to fix the regression first. There are users of the PEP 532, we 
cannot simply ignore them, withdraw the PEP and remove the feature immediately 
without warning users.

If someone wants to withdraw the PEP, it has to go through the regular 
deprecation process with a slow transition. Last time we pushed too many 
incompatible changes (Python 2 to Python 3 transition), it was a mess. 
Moreover, I would prefer to have at least one Python release (if not two or 
more) which provides two options, deprecated way and a new better way, to have 
a smooth transition.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

Mark Shannon is against the idea of providing a way to set the frame evaluation 
function (PEP 523), but Dino Viehland, Eric Snow, Brett Cannon and me want to 
provide a C API for that.

--

I propose to properly fix this issue in Python 3.9:

* Add a public C API to get and set the frame evaluation function
* Pass tstate to this frame evaluation function

Pass tstate is a backward incompatible change. But it's unclear to me if its 
API was part of the "public" C API in Python 3.7. In Python 3.8, 
PyInterpreterState structure moved to the internal C API which is clearly 
excluded from backward compatibility warranties of the public C API.

Anyway, I expect that they are less than 10 projects in the world which use the 
frame evaluation function, which it should be doable to update all of them to 
support the C API for Python 3.9 that I'm proposing.

--

> This is no longer possible because in 3.8 the PyInterpreterState is opaque, 
> so, Py_BUILD_CORE_MODULE needs to be defined defined and 
> "internal/pycore_pystate.h" must be included to set 
> PyInterpreterState.eval_frame.

The status quo is that Python 3.8.0, 3.8.1 and 3.8.2 have been released with 
that. If someone wants to support "Python 3.8", using Py_BUILD_CORE_MODULE to 
access the internal C API is the way to go. If we wanted to add a public C API 
in Python 3.8, IMHO we had to do it *before Python 3.8.0 release. Now it's way 
too late.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

I marked bpo-38818 "Modify PyInterpreterState.eval_frame to pass tstate 
(PyThreadState)" as a duplicate of this issue.

It has been said earlier that if we choose to add a public C API to set the 
frame evaluation function, it's better to design it properly from the start: so 
include tstate.

See https://vstinner.github.io/cpython-pass-tstate.html for the rationale 
behind passing tstate everywhere. See also multiple discussions on python-dev 
about passing explicitly tstate.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Paulo Henrique Silva]

Change by Paulo Henrique Silva :


--
nosy: +phsilva

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

Fabio: "I don't really see a problem in breaking the API in major releases (so, 
having access for it in the internal API but without a backwards-compatibility 
guarantee seems like a good fit for me)."

In Python, the internal C API now means that you have to define 
Py_BUILD_CORE_MODULE macro and include a special "internal/pycore_pystate.h" 
header file.

If you are fine with doing that in PyCharm, what is the purpose of this issue?

Do you mean in *internal* API or a *private* API? Private API is basically a 
public API with weaker backward compatibility warranties and with a "_" prefix 
(_Py, _PY prefix).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Ned Deily]

Change by Ned Deily :


--
keywords: +3.8regression
nosy: +lukasz.langa
priority: normal -> critical

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

> @victor if we are going to ask folks to start using a setter and getter I say 
> we might as well get it right the first time and start taking the tstate now.

Done: New PR 17352 combines my two other PRs.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +16836
pull_request: https://github.com/python/cpython/pull/17352

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Brett Cannon]

Brett Cannon  added the comment:

@greg I hope this goes into 3.8.1 since it was a breaking change.

@victor if we are going to ask folks to start using a setter and getter I say 
we might as well get it right the first time and start taking the tstate now.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

New question: if we add a public API to Python 3.8.1, would it make sense to 
directly change eval_frame type to add a new "tstate" parameter? See bpo-38818 
for the rationale.

I already proposed a change for the master branch: 
https://github.com/python/cpython/pull/17187

Maybe I should combine PR 17340 (add PyInterpreterState_SetEvalFrameFunc()) 
with PR 17187 (add tstate parameter).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

I wrote PR 17340 to add Add PyInterpreterState_GetEvalFrameFunc() and 
PyInterpreterState_SetEvalFrameFunc() functions.

Since the PEP 523 has been approved, for me, these functions must be public, 
not private. My implementation adds these functions to 
Include/cpython/pystate.h: "non-portable" API specific to CPython (exclude from 
the limited C API).

PyInterpreterState_GetEvalFrameFunc() is needed if you want to inject a "hook" 
and not really replaced the whole function. For example, execute code before or 
after the call. I guess that PyCharm debugger works like that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +16824
pull_request: https://github.com/python/cpython/pull/17340

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Gregory P. Smith]

Gregory P. Smith  added the comment:

+1 to re-exposing a way to do PEP-523.

PEP-523 added a public API, we unintentionally hid it behind the mask of 
Py_BUILD_CORE_MODULE in 3.8.  We shouldn't remove PEP-523's abilities without a 
deprecation cycle.  But given the use cases tend to be "deep" knowledge places 
that can adapt to API change and version checks, I think re-exposing it without 
the need for Py_BUILD_CORE_MODULE is reasonable.

Are you proposing this for 3.8.1?

Or is 3.8 burned for this 3.6 & 3.7 feature :sadface: (but we know how to work 
around it - so if our RM says it is, we'll deal) and re-exposing in some manner 
it only happens in 3.9?

--
versions: +Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Gregory P. Smith]

Change by Gregory P. Smith :


--
nosy: +gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Brett Cannon]

Brett Cannon  added the comment:

Posted 
https://mail.python.org/archives/list/python-...@python.org/thread/4UZJYAZL3NHRAGN5WAMJC4IHAHEXF3QF/
 to see if anyone else wants to weigh in.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Brett Cannon]

Brett Cannon  added the comment:

@Mark

> PEP 523 makes no mention of adding a getter or setter.
> Adding them is a big change to Python semantics and shouldn't, IMO, be done 
> without a PEP that explicit states they are going to be added.

Adding getters or setters for something that was previously doable that we 
accidentally took away from users is not an expansion of semantics; it's fixing 
a backwards-compatibility break in a way that lets us keep the goal of making 
PyInterpreterState opaque.

I'm considering this issue at a stalemate and so I'm going to loop in 
python-dev to help settle this. If that still locks up then we can bring this 
to the steering council.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Fabio Zadrozny]

Fabio Zadrozny  added the comment:

@Mark

First you have to explain to me how you envision changing the method code 
reliably in the debugger... 

Import hooks don't work (they'd break with something as simple as the code 
below)

def method():
   a = 10

mod = reload(old_mod)
old_mod.method.__code__ = mod.method.__code__

using the tracing also doesn't work (it's too late to change the code)

Note: consider the reload just an example, not the only use case (say, the user 
could pickle code objects to remote execution and the debugger should still 
work).

Also, consider you have to change the bytecode of methods which are only 
internal to a function (and thus can't be accessed from the outside).

Then, if you have a reliable way to do it, how do you keep track of those code 
objects to reapply the patching when breakpoints change? What if it adds a 
breakpoint to a new method, how do you locate it? Creating strong references to 
methods isn't an option because it would prevent things from being garbage 
collected (and you'd have to track all objects containing code objects for it 
to be reliable).

As a note, pydevd does have some support for hot-reloading, but there are too 
many corner-cases and it doesn't work 100% for live coding (it's an unsolved 
problem is Python) -- and I can't really envision it working for regular 
breakpoints as there are too many corner cases to handle.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Mark Shannon]

Mark Shannon  added the comment:

Fabio,

Can you give me a specific example where changing the bytecode via the 
`__code__` attribute of a function does not work as expected?

I am assuming that adding a breakpoint on a line is equivalent to adding 
`breakpoint();` at the beginning of that line. If the bytecode of a function is 
modified dynamically, say by a decorator, then it is unclear what should be 
done, and it is hard to claim that any particular approach is more correct.
However, assuming that the change is well defined and keeps the mapping to the 
original code, then adding a breakpoint to the modified code should work just 
as well before or after modification.

As an example, consider 
http://code.activestate.com/recipes/277940-decorator-for-bindingconstants-at-compile-time/
 which provides a means of converting global variable reads to constants.  In 
that case the order in which the decorator and breakpoint insertion are applied 
shouldn't matter.

I propose a new method on code objects `withCallAtLine(callable: Callable[], 
line:int)` which returns a new code object with calls to the given callable at 
the given line.
A breakpoint can then be inserted at line L in function f with `f.__code__ = 
f.__code__.withCallAtLine(sys.breakpoint, L)`.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Mark Shannon]

Mark Shannon  added the comment:

Brett,
PEP 523 makes no mention of adding a getter or setter.
Adding them is a big change to Python semantics and shouldn't, IMO, be done 
without a PEP that explicit states they are going to be added.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Brett Cannon]

Brett Cannon  added the comment:

OK, then my vote is to provide getter and setter methods that are 
underscore-prefixed to mark them as "internal" with clear comments specifying 
that they are part of PEP 523 and thus should not be needlessly broken in a 
bugfix release.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Fabio Zadrozny]

Fabio Zadrozny  added the comment:

@Bret

I don't really see a problem in breaking the API in major releases (so, having 
access for it in the internal API but without a backwards-compatibility 
guarantee seems like a good fit for me).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Brett Cannon]

Brett Cannon  added the comment:

I think the real question is whether this is part of the CPython public API or 
the CPython internal API.

@Fabio how burdensome would it be if we placed this in the internal API that 
you can get access to but we don't make backwards-compatibility guarantees 
about? For instance, Victor wants to start passing in thread state which will 
change the API for 3.9, but we wouldn't expect to change it in a bugfix release.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Fabio Zadrozny]

Fabio Zadrozny  added the comment:

@Mark @Brett

Well, PEP 523 still works (it's just more inconvenient to use now).

Still, if PEP 523 will still be supported I think that having the setter/getter 
makes sense.

If it is to be deprecated as @Mark is suggesting it doesn't really make sense 
to add it (but then, it should really be deprecated and ideally there'd be some 
replacement for the current debugger use... not sure about other use cases such 
as a jit, which was the initial target of PEP 523 -- @Mark, do you want to go 
that route/create a PEP to deprecate it so that this discussion takes place in 
a proper place?).

p.s.: as a note, bytecode modification on the actual object is not a usable 
approach for the debugger because users could break that in real-world use 
cases (i.e.: what happens if the user creates a **new** code and sets it to the 
code which had the breakpoint? What about breakpoint changes? Right now the 
debugger evaluates all assumptions just before the frame is executed, so, it's 
easier to get things right -- the case you posted currently does what's 
expected on pydevd). Still, a callback before the execution so that it could 
change the frame code before it's executed without the remainder of PEP 523 
would be enough (and maybe it could be adopted in other Python implementations 
too) -- actually, for the debugger it'd be awesome if the frame code could be 
changed from inside a trace call and then that stack would restart execution 
(close to what happens with setting the frame line to be executed), but I guess 
this would be a completely different request ;)

p.s.: please don't reply to my previous p.s. here (let's move the discussion to 
another place -- either by @Mark creating a PEP for discussion or acknowledging 
the issue is ok given the current status quo).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

Change by STINNER Victor :


--
keywords: +patch
pull_requests: +16739
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/17187

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Brett Cannon]

Brett Cannon  added the comment:

I would like to bring this issue back on topic as this about how to expose PEP 
523 support in Python 3.8, not whether Fabio should be using a different 
approach when he has been something sanctioned in an accepted PEP and was 
previously doable that was (accidentally) taken away from him.

If people want to do a separate PEP to roll back PEP 523 that's fine, but until 
that occurs I think we should move forward with the fact that PEP 523 is active.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Mark Shannon]

Mark Shannon  added the comment:

Fabio,

If the user changes the `__code__` attribute of a function then, AFAICT, your 
debugger does the wrong thing, but bytecode modification does the right thing.

Suppose we have two functions `spam` and `eggs`.
Set a break point in `eggs`, set `spam.__code__ = eggs.__code__`, then call 
`spam`.
With bytecode modification, we get the correct result. That is, execution 
breaks in the source code of `eggs` when `spam` is run.
I think your debugger will do the wrong thing as it will execute the original 
code of `spam`. Could you confirm what it does?

But that's not the main issue, IMO. The big problem is that changing out the 
interpreter is not composable, unlike bytecode modification.

Suppose we have MyProfiler and YourDebugger.
MyProfiler wants to record calls and YourDebugger wants to support breakpoints.

With bytecode modification, and some care, we can do both.
Swapping out the interpreter is likely to cause all sorts of errors and 
confusion.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Fabio Zadrozny]

Fabio Zadrozny  added the comment:

@Mark

I can think of many use-cases which may break if the function code is changed 
(users can change the code in real-use cases and when they do that they'd loose 
debugging).

So, as long as the function code is part of the public API of Python, the 
debugger can't really change it for breakpoints (which is a bit different from 
the frame code, which the user can't really swap and it's not so common to 
change).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Mark Shannon]

Mark Shannon  added the comment:

It sounds to me like `PyInterpreterState.eval_frame` is being used to lazily 
modify the bytecode to support breakpoints.

I can see no reason why changing the bytecode can't be done via 
`function.__code__`.
Suppose the code-object with the breakpoint add is `bcode`, then to turn on the 
breakpoint:
original_code = f.__code__
f.__code__ = bcode

and to turn it off
f.__code__ = original_code


The JVM supports bytecode instrumentation (via class loaders). It works well, 
as it provides a clear way for third party tools to modify the behaviour of a 
particular piece of code without violating any of the invariants of the 
interpreter. 
We don't really advertise setting `function.__code__` as a way to add 
low-impact breakpoints or profiling, but it is there.

If this use case is important, and it sounds like it is, then a better option 
would be to offer library support for adding and removing 
breakpoints/instrumentation.
This would have the advantage of being composable in a way that changing 
`PyInterpreterState.eval_frame` is not; in other words, it would be possible 
for one tool to add profiling and another to add breakpoints and have both work 
correctly.

I can write up a PEP if necessary.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

I heard that for debuggers, the PEP 523 is really useful. I recall a huge 
speedup thanks to this PEP in Jetbrain:
https://blog.jetbrains.com/pycharm/2017/03/inside-the-debugger-interview-with-elizaveta-shashkova/

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Brett Cannon]

Brett Cannon  added the comment:

PEP 523 was to give user code the ability to change the eval function. While 
the work was motivated by our JIT work, supporting debugging was another 
motivating factor: https://www.python.org/dev/peps/pep-0523/#debugging. There's 
no C API because at the time there was no need as PyInterpreterState was 
publicly exposed.

I don't think anyone is suggesting to add something to the stable ABI, so this 
comes down to whether this should be exposed as part of the CPython API or the 
publicly accessible internal API. Since there is no distinction of "you 
probably don't want to use this but we won't yank it out from underneath" you I 
think this belongs in the CPython API somehow. Otherwise someone should propose 
withdrawing PEP 523 as I think that shifts what the PEP was aiming for. You can 
also ask on python-dev or the steering council (which I will abstain myself 
from due to being a co-author of PEP 523).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

By the way, it's still possible to access directly ts->interp if you include 
"pycore_pystate.h" header, which requires to define Py_BUILD_CORE_MODULE. This 
header is an internal header.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Fabio Zadrozny]

Fabio Zadrozny  added the comment:

@Mark I don't want to change the original function code, I just want to change 
the code to be executed in the frame (i.e.: as breakpoints change things may be 
different).

Changing the actual function code is a no-go since changing the real function 
code can break valid user code.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Mark Shannon]

Mark Shannon  added the comment:

Victor,
I don't think this is a regression.
`PyThreadState` is an internal opaque data structure, which means we are free 
to change it.
That the `eval_frame` is hard to access is a feature not a bug, as it 
discourages misuse and enables us to remove it easily, when a better approach 
becomes available.

PEP 523 is quite vague, but the rationale indicates that exposing `eval_frame` 
is for "a method-level JIT". PEP 523 did not suggest adding an API. If it had 
(and I had had the time) I would have opposed it more vigorously.

IMO, the correct way to change the code object is to set `function.__code__` 
which can be done easily from either Python or C code.

@Fabioz, is there anything preventing you from doing that?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

STINNER Victor  added the comment:

> IMO this punches a big hole in the Python execution model, but provides no 
> benefit.

This PEP is about fixing a Python 3.8 regression. In Python 3.7, it was 
possible to get and set frame_eval. In Python 3.8, it's no longer possible.

One option to fix the regression would be to again expose PyInterpreterState 
structure... but we are trying to do the opposite: hide more structures, not 
expose more structures :-/

IMHO private getter and setter functions are perfectly fine. Please ensure that 
the setter can report an issue. We have too many setters which cannot report an 
error which is very painful :-(

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Brett Cannon]

Brett Cannon  added the comment:

@Mark are you strongly opposed because we're providing an API for changing the 
eval function in the CPython API and you think it should be in the private API? 
Or you objecting to PEP 523 all-up (based on your list of objections)? Either 
way the PEP was accepted and implemented a while ago and so I'm not quite sure 
what you are expecting as an outcome short of a repeal of PEP 523 which would 
require a separate PEP.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Fabio Zadrozny]

Fabio Zadrozny  added the comment:

@Mark Shannon what I do is change the code object of the frame about to be 
evaluated to add a programmatic breakpoint, to avoid the need to have the trace 
function set at contexts that would need to be traced (after changing the 
frame.f_code it goes on to call the regular _PyEval_EvalFrameDefault), so, the 
user-code runs at full speed on all contexts (there's still added overhead on a 
function call to decide if the code object needs to be changed, but that'd 
happen on the regular tracing code too).

Note that this does not change the semantics of anything as it calls the 
regular _PyEval_EvalFrameDefault, so, the worries you're listing shouldn't be a 
concern in this particular use-case.

Also note that until Python 3.7 this was easy to change, and that's still 
possible in Python 3.8 (the only thing is that now it's less straightforward).

Note that my use is much simpler that the original intent of the frame 
evaluator -- my use case could be solved by having a callback to change the 
code object before the frame execution -- but as far as I know, right now, the 
way to do that is through the frame evaluation API.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Mark Shannon]

Mark Shannon  added the comment:

Fabio,

OOI what are you trying achieve by changing the frame evaluator?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Mark Shannon]

Mark Shannon  added the comment:

I'm strongly opposed to this change.

PEP 523 does not specify what the semantics of changing the interpreter frame 
evaluator actually is:
Is the VM obliged to call the new interpreter?
What happens if the custom evaluator leaves the VM in a inconsistent state?
Does the VM have to roll back any speculative optimisations it has made? What 
happens if it the evaluator is changed multiple times by different modules?
What if the evaluator is changed when a coroutine or generator is suspended, or 
in another thread?
I could go on...

IMO this punches a big hole in the Python execution model, but provides no 
benefit.

--
nosy: +Mark.Shannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Dino Viehland]

Dino Viehland  added the comment:

Adding the getter/setters seems perfectly reasonable to me, and I agree they 
should be underscore prefixed as well.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Eric Snow]

Eric Snow  added the comment:

It depends on how you look at the degree to which you are interacting with the 
runtime.  This is a fairly low-level hook into the runtime.  So arguably if you 
are using this API then you should specify being a "core" extension.  That 
said, getting that clever about it is a bit too much.  The authors or PEP 523 
can correct me if I'm wrong, but it seems like there isn't a good reason to 
restrict access.

So basically, I agree with you. :)

How about one of the following?

* _PyInterpreterState_SetEvalFrame(_PyFrameEvalFunction eval_frame)
* _PyInterpreterState_SetFrameEval(_PyFrameEvalFunction eval_frame)

The underscore basically says "don't use this unless you know what you are 
doing".  Or perhaps that is overkill too?  "_PyFrameEvalFunction" has an 
underscore, so perhaps not.

Also, it would make sense to have a matching getter.

--
nosy: +brett.cannon, dino.viehland

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +eric.snow

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38500] Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals

[Fabio Zadrozny]

New submission from Fabio Zadrozny :

In CPython 3.7 it was possible to do:

#include "pystate.h"
...
PyThreadState *ts = PyThreadState_Get();
PyInterpreterState *interp = ts->interp;
interp->eval_frame = my_frame_eval_func;

This is no longer possible because in 3.8 the PyInterpreterState is opaque, so, 
Py_BUILD_CORE_MODULE needs to be defined defined and 
"internal/pycore_pystate.h" must be included to set 
PyInterpreterState.eval_frame.

This works but isn't ideal -- maybe there could be a function to set 
PyInterpreterState.eval_frame?

--
components: Interpreter Core
messages: 354803
nosy: fabioz, vstinner
priority: normal
severity: normal
status: open
title: Provide a way to get/set PyInterpreterState.frame_eval without needing 
to access interpreter internals
type: enhancement
versions: Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com