gh-121450: Make inline breakpoints use the most recent pdb instance #121451
Conversation
| @@ -0,0 +1 @@ | |||
| Inline breakpoints (:func:`breakpoint` and :func:`pdb.set_trace()`) will use the most recent ``Pdb`` instance, instead of creating a new one. | |||
There was a problem hiding this comment.
| Inline breakpoints (:func:`breakpoint` and :func:`pdb.set_trace()`) will use the most recent ``Pdb`` instance, instead of creating a new one. | |
| Inline breakpoints (:func:`breakpoint` and :func:`pdb.set_trace()`) now reuse the same ``Pdb`` instance, instead of creating a new one each time. |
|
Are you sure this can't change behaviour in some cases, where the pdb instance now has a different state? For example, what if the user added breakpoints at the prompt between two breakpoint() calls? |
|
This will change the behavior, in favor of the expectation, in some cases, and that's the reason of the change. Adding breakpoints is not impacted because The behavior changes when user is debugging between |
This could surprise someone, so best explain it in what's new. |
Lib/pdb.py
Outdated
|
|
||
| def __new__(self, *args, **kwargs): | ||
| Pdb._last_pdb_instance = super().__new__(self) | ||
| return Pdb._last_pdb_instance |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Maybe Pdb._last_pdb_instance should be set in set_trace().
Probably need a test for this scenario too.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
|
LGTM. Still needs a what's new entry, I think. |
iritkatriel
left a comment
iritkatriel
left a comment
There was a problem hiding this comment.
LGTM, add a what's new entry and it's good to go.
With 3.14, breakpoints use the most recently created Pdb instance, in order to maintain history, display commands etc. See gh-121450, python/cpython#121451
With 3.14, breakpoints use the most recently created Pdb instance, in order to maintain history, display commands etc. See gh-121450, python/cpython#121451
With 3.14, breakpoints use the most recently created Pdb instance, in order to maintain history, display commands etc. See gh-121450: python/cpython#121451 Also includes misc fixes tests: - test_break_with_inner_set_trace - test_set_trace_remembers_previous_state - set_trace/set_trace_via_module - set_trace_with_incomplete_pdb - test_commands_* - test_nested_completer - test_integration - fix fixtures (set_trace, ...)
With 3.14, breakpoints use the most recently created Pdb instance, in order to maintain history, display commands etc. See gh-121450: python/cpython#121451 Also includes misc fixes tests: - test_break_with_inner_set_trace - test_set_trace_remembers_previous_state - set_trace/set_trace_via_module - set_trace_with_incomplete_pdb - test_commands_* - test_nested_completer - test_integration - fix fixtures (set_trace, ...)
With 3.14, breakpoints use the most recently created Pdb instance, in order to maintain history, display commands etc. See gh-121450: python/cpython#121451 Also includes misc fixes tests: - fix fixtures (set_trace, ...) - test_break_with_inner_set_trace - test_set_trace_remembers_previous_state - set_trace/set_trace_via_module - fix set_trace_in_completion - set_trace_with_incomplete_pdb - test_commands_* - test_nested_completer - test_integration - test_locals - completions tests
With 3.14, breakpoints use the most recently created Pdb instance, in order to maintain history, display commands etc. See gh-121450: python/cpython#121451 Also includes misc fixes tests: - fix fixtures (set_trace, ...) - test_break_with_inner_set_trace - test_set_trace_remembers_previous_state - set_trace/set_trace_via_module - fix set_trace_in_completion - set_trace_with_incomplete_pdb - test_commands_* - test_nested_completer - test_integration - test_locals - completions tests
2 participants
pdb.set_trace()will use the most recent pdb instance created if there is one. A minor fix inbdbis introduced otherwise the debugger will stop inBdb.reset(). A small fix intest_pdbis also required because we need to make sure the instancepdb.set_traceis using the patched instance.The common debugger experience won't be much different to the users. We never explain the implementation details of
pdb.set_tracein our documentation so I don't even see a conflict with existing documentation. Not sure if any new documentation is needed at this point.