[PATCH] Debuggers need a way to change the locals of a frame

[fabioz]

BPO	1654367
Nosy	@loewis, @arigo, @rhettinger, @fabioz
Files	test_frames.py test_save_locals.py: Some cases where changing frame.f_locals fails

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2007-02-07.17:12:28.000>
labels = ['interpreter-core', 'type-feature']
title = '[PATCH] Debuggers need a way to change the locals of a frame'
updated_at = <Date 2014-02-06.15:47:40.284>
user = 'https://github.com/fabioz'

bugs.python.org fields:

activity = <Date 2014-02-06.15:47:40.284>
actor = 'fabioz'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2007-02-07.17:12:28.000>
creator = 'fabioz'
dependencies = []
files = ['8288', '33917']
hgrepos = []
issue_num = 1654367
keywords = ['patch']
message_count = 14.0
messages = ['55003', '55004', '55005', '110520', '113625', '114415', '210233', '210265', '210266', '210275', '210374', '210400', '210402', '210403']
nosy_count = 4.0
nosy_names = ['loewis', 'arigo', 'rhettinger', 'fabioz']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue1654367'
versions = ['Python 3.3']

[fabioz]

Debuggers need a way to change the local variables in a given frame... currently, this works only if this change is done in the topmost frame (and under certain circumstances), but it should work for any frame.

Initial discussion at:
http://mail.python.org/pipermail/python-dev/2007-February/070884.html

Apparently, the problem is the order in which PyFrame_LocalsToFast / PyFrame_FastToLocals is called.

The proposed solution to this is having a savelocals() method in the frame object and have it reflect the changes in its returned dict into its locals. It will simply enable users to call PyFrame_LocalsToFast externally after a change, to be sure that it will not be changed (and it must be done before another call to PyFrame_LocalsToFast -- which I don't see as a large problem, because it is of restrict use -- mainly for debuggers).

--------- frameobject.c Patch part 1: -----------------

static PyObject *
PyFrame_SaveLocals(PyFrameObject *f)
{
    PyFrame_LocalsToFast(f, 0);
	Py_INCREF(Py_None);
	return Py_None;
}

static PyMethodDef frame_methodlist[] = {
{"savelocals", (PyCFunction)PyFrame_SaveLocals, METH_NOARGS,
"After a change in f_locals, this method should be called to save the changes internally."
},
{NULL} /* Sentinel */
};

---------- frameobject.c Patch part 2: ---------------
Add to PyTypeObject PyFrame_Type:

frame_methodlist,/* tp_methods */

------------------ end patch -------------------------

I'm sorry that this is not in an actual patch format... but as I didn't get it from the cvs, it is easier for me to explain it (as it is a rather small patch).

Attached is a test-case for this patch.

Thanks,

Fabio

[loewis]

Why don't you set the clear parameter to 1?

Please do submit a patch, you can use 'diff -ur' to create a recursive unified diff between source trees. Please also try to come up with a patch to the documentation.

[arigo]

A point of detail probably, but I suppose that instead of introducing a new method on frame objects, we could allow f_locals to be a writeable attribute. Setting it to a dictionary would update the value of the local variables. It's a bit of a hack, but so is the need for an explicit savelocals() method.

A cleaner solution is to have f_locals return a dict-like object instead of a real dict when appropriate, but it takes more efforts to implement.

[BreamoreBoy]

Fabio, could you please supply a patch as requested by Martin in msg55004? Also note Armin's comments in msg55005.

[rhettinger]

I think this falls under the language moratorium in that it requires core changes that make it more difficult for other implementations to catch-up.

[fabioz]

I agree that it'd be cleaner making the frame locals a dict-like object with write access, but I wouldn't be able to do that because of time constraints (and I'd have to research more how to do it and it'd be much more intrusive I guess).

So, if it's guaranteed that it'll be accepted I can provide a patch (changing the clear to 1) of the savelocals() version.

I guess I don't agree this falls in the language moratorium, but if that's the case I can wait 1 or 2 more years before submitting the patch :)

[fabioz]

Just a note for anyone interested: ctypes can be used to access that function:

http://pydev.blogspot.com.br/2014/02/changing-locals-of-frame-frameflocals.html

So, I think that the changes I proposed shouldn't be applied (it'd only be worth if someone provided an implementation that did frame.f_locals writable...).

[arigo]

Hi Fabio! This is admittedly a corner-case use case, but if you think it would be worth it, we can very easily add this as a feature to PyPy (by making frame.f_locals writeable). Then we can add the write inside pdb, and you could do the same for PyDev when running on PyPy. It's certainly something that goes beyond the standard of CPython 2.7 but we'd consider it because it's obviously debug-only.

[arigo]

Also, can you provide a concrete case? Trying around in CPython 2.7, it seems that locals of the current frame can always be modified from a pdb.set_trace().

[fabioz]

Hi Armin,

Sure, just attached the test case with tests failing (on the top, comment the save_locals(frame) which has a 'pass' to see it working).

Mostly, it happens when editing something not in the top-frame (but sometimes I think it could fail there too, depending whether it's a cellvar or freevar).

[arigo]

Sorry to hijack CPython's bug tracker for that, but can you check if this makes sense to you? I added a function 'locals_to_fast()' to the __pypy__ built-in module which just calls the PyPy equivalent to PyFrame_LocalsToFast(). Your tests are passing, so I guess there should be no problem. The diff is here: http://bpaste.net/show/176119/ (are you ok with having your name put into the test file?).