8000 gh-121450: Make inline breakpoints use the most recent pdb instance by gaogaotiantian · Pull Request #121451 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-121450: Make inline breakpoints use the most recent pdb instance #121451

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

Merged
merged 9 commits into from
Jul 11, 2024
Merged

gh-121450: Make inline breakpoints use the most recent pdb instance #121451

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
634725c
Make inline breakpoints use the most recent pdb instance
gaogaotiantian Jul 6, 2024
c15480c
📜🤖 Added by blurb_it.
blurb-it[bot] Jul 6, 2024
a72864d
Fix docs/comments
gaogaotiantian Jul 10, 2024
dbe8be4
Merge branch 'main' into pdb-singleton
gaogaotiantian Jul 10, 2024
9233cc7
Set _last_pdb_instance in Pdb.set_trace()
gaogaotiantian Jul 10, 2024
6d438a3
Update news
gaogaotiantian Jul 10, 2024
fa7574b
Does not need the explicit instantiation anymore
gaogaotiantian Jul 10, 2024
b15c3b2
Fix lint
gaogaotiantian Jul 10, 2024
73b0e9f
Add what's new entry
gaogaotiantian Jul 10, 2024
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
Next Next commit
Make inline breakpoints use the most recent pdb instance
  • Loading branch information
@gaogaotiantian
gaogaotiantian committed Jul 6, 2024
commit 634725c0b60b2d8abaaa113852980be520d4e6da
1 change: 1 addition & 0 deletions Lib/bdb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ def set_trace(self, frame=None):

If frame is not specified, debugging starts from caller's frame.
"""
sys.settrace(None)
if frame is None:
frame = sys._getframe().f_back
self.reset()
Expand Down
11 changes: 10 additions & 1 deletion Lib/pdb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,12 @@ class Pdb(bdb.Bdb, cmd.Cmd):

_file_mtime_table = {}

_last_pdb_instance = None

def __new__(self, *args, **kwargs):
Pdb._last_pdb_instance = super().__new__(self)
return Pdb._last_pdb_instance
Copy link
Member

Choose a reason for hiding this comment

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

what if we're creating this instance while debugging? It would overwrite the instance from the hard coded breakpoint, and this one will be reused at the next hard coded breakpoint. This is not what's intended right?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe Pdb._last_pdb_instance should be set in set_trace().

Probably need a test for this scenario too.

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean in Pdb.set_trace()? That would make some sense, only enlist the instance when it explicitly takes over the tracing.

However, if the user somehow creates and uses a pdb instance before a breakpoint(), we really can't determine the purpose. It could be nested debugging (debug command in pdb which I think we should patch to make sure the _last_pdb_instance is not modified). Or the user could want a new instance, for example, in the test cases.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand. If you set _last_pdb_instance in Pdb.set_trace() then you get clear semantics, and no other breakpoint interrupts with this. Is that not a good thing?

8000
Copy link
Member Author

Choose a reason for hiding this comment

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

No I agree that setting _last_pdb_instance in Pdb.set_trace() is a good approach. And I just realized the nested debugging (debug command in pdb) uses sys.call_tracing so it probably won't affect anything if we do it in Pdb.set_trace(). I'll do the change and see how it works out.

Copy link
Member Author

Choose a reason for hiding this comment

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

And this change actually made the test change unnecessary so it's good. Do you think we need to test more scenarios? I think the case where users explicitly create a Pdb instance in the pdb prompt is really rare - what's the point?


def __init__(self, completekey='tab', stdin=None, stdout=None, skip=None,
nosigint=False, readrc=True):
bdb.Bdb.__init__(self, skip=skip)
Expand Down Expand Up @@ -2350,7 +2356,10 @@ def set_trace(*, header=None):
an assertion fails). If given, *header* is printed to the console
just before debugging begins.
"""
pdb = Pdb()
if Pdb._last_pdb_instance is not None:
pdb = Pdb._last_pdb_instance
else:
pdb = Pdb()
if header is not None:
pdb.message(header)
pdb.set_trace(sys._getframe().f_back)
Expand Down
45 changes: 45 additions & 0 deletions Lib/test/test_pdb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2448,6 +2448,49 @@ def test_pdb_show_attribute_and_item():
(Pdb) c
"""

# doctest will override pdb.set_trace during the test, so we need to backup
# the original function to use it in the test
original_pdb_settrace = pdb.set_trace

def test_pdb_with_inline_breakpoint():
"""Inline breakpoint() calls should invoke the same debugger instance

>>> def test_function():
... x = 1
... import pdb; pdb.Pdb().set_trace()
... original_pdb_settrace()
... x = 2

>>> with PdbTestInput(['display x',
... 'n',
... 'n',
... 'n',
... 'n',
... 'undisplay',
... 'c']):
... test_function()
> <doctest test.test_pdb.test_pdb_with_inline_breakpoint[0]>(3)test_function()
-> import pdb; pdb.Pdb().set_trace()
(Pdb) display x
display x: 1
(Pdb) n
> <doctest test.test_pdb.test_pdb_with_inline_breakpoint[0]>(4)test_function()
-> original_pdb_settrace()
(Pdb) n
> <doctest test.test_pdb.test_pdb_with_inline_breakpoint[0]>(4)test_function()
-> original_pdb_settrace()
(Pdb) n
> <doctest test.test_pdb.test_pdb_with_inline_breakpoint[0]>(5)test_function()
-> x = 2
(Pdb) n
--Return--
> <doctest test.test_pdb.test_pdb_with_inline_breakpoint[0]>(5)test_function()->None
-> x = 2
display x: 2 [old: 1]
(Pdb) undisplay
(Pdb) c
"""

def test_pdb_issue_20766():
"""Test for reference leaks when the SIGINT handler is set.

Expand Down Expand Up @@ -3347,6 +3390,8 @@ def test_header(self):
with ExitStack() as resources:
resources.enter_context(patch('sys.stdout', stdout))
resources.enter_context(patch.object(pdb.Pdb, 'set_trace'))
# Make sure pdb.set_trace() is using a patched pdb instance
_ = pdb.Pdb()
pdb.set_trace(header=header)
self.assertEqual(stdout.getvalue(), header + '\n')

Expand Down
Loading
0