Skip to content
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.

Sign up for GitHub

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

Allow function definition locals() to be modified in the shell #571

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Allow function definition locals() to be modified in the shell #571

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b9d94f
Allow function definition locals() to be modified in the shell
asmeurer Nov 16, 2022
c5d17fd
Revert change to IPython shell to make locals editable
asmeurer Nov 17, 2022
95ef370
Make the var view update when executing a command in the internal shell
asmeurer Nov 17, 2022
82b1f15
Add a comment explaining the curframe_locals behavior
asmeurer Dec 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions pudb/debugger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ def set_frame_index(self, index):
return

self.curframe, lineno = self.stack[index]
self.curframe_locals = self.curframe.f_locals
Copy link
Owner

@inducer inducer Nov 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment explaining what it's good for? TBH, I'm not sure I understand why the proposed changes have the described effect. (at least with just looking at the diff)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might try to dig deeper to figure out exactly what is going on. From what I can tell, as long as you reuse the same locals object, any modifications you make to it are reflected when the frame is run. But if you access frame.f_locals again, it recomputes the locals and resets them. I'm unclear exactly how it works, but it does.

I'm actually unclear why your example at #26 (comment) didn't work for you. I tested it and even in Python 2, it has the desired effect when you set i on the given line.

Copy link
Owner

@inducer inducer Nov 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you've nerd-sniped me. Grr. 🙂

Here's what I've found so far:

Some notes:

  • Interestingly, it seems that all this code was significantly rewritten for 3.12.
  • There also seem to have been some changes here for 3.11. Search for PyFrame_LocalsToFast here.

Whatever we do here, we should make sure to preserve a summary of it in comments, so that we don't have to rediscover it some other time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found https://peps.python.org/pep-0558/, which mentions this behavior (footnote 3), although the link there is broken.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That PEP is very technical, but I think what happens is:

  • locals is designed to be snapshotted. That's why when you do something like 
    def f():
    x = 1
    locals()['x'] = 2
    print(x)

f()
    it prints 1 instead of 2.
  • frame.f_locals is designed to be a write-through proxy (probably exactly for this feature).
  • What I think happens is that the descriptor for f_locals computes this write-through proxy from a snapshot. That means that when you access it, it recomputes it from what it was, and throws away whatever was there before. Since it's a write-through proxy, this effectively resets the locals in the frame.

But I'm not completely sure about this. Honestly I think a core dev would need to confirm the details. I think I'll just cross-reference this PEP in the comments since that should be a good jumping off point for anyone who is interested in what is going on.


filename = self.curframe.f_code.co_filename

Expand Down Expand Up @@ -1706,8 +1707,8 @@ def cmdline_get_namespace():

from pudb.shell import SetPropagatingDict
return SetPropagatingDict(
[curframe.f_locals, curframe.f_globals],
curframe.f_locals)
[self.debugger.curframe_locals, curframe.f_globals],
self.debugger.curframe_locals)

def cmdline_tab_complete(w, size, key):
try:
Expand Down Expand Up @@ -1837,6 +1838,8 @@ def cmdline_exec(w, size, key):
sys.stdout = prev_sys_stdout
sys.stderr = prev_sys_stderr

self.update_var_view()

def cmdline_history_browse(direction):
if self.cmdline_history_position == -1:
self.cmdline_history_position = len(self.cmdline_history)
Expand Down Expand Up @@ -2060,7 +2063,7 @@ def fallback():
else:
runner = shell.custom_shell_dict["pudb_shell"]

runner(curframe.f_globals, curframe.f_locals)
runner(curframe.f_globals, self.debugger.curframe_locals)

self.update_var_view()

Expand Down Expand Up @@ -2768,7 +2771,7 @@ def set_current_line(self, line, source_code_provider):

def update_var_view(self, locals=None, globals=None, focus_index=None):
if locals is None:
locals = self.debugger.curframe.f_locals
locals = self.debugger.curframe_locals
if globals is None:
globals = self.debugger.curframe.f_globals

Expand Down