bpo-24160: Fix breakpoints persistence across multiple pdb sessions

[iritkatriel]

The information about breakpoints is split between Breakpoint.bplist/Breakpoint.bpbynumber (at class scope) and the breaks dictionary that lives on the Bdb() instance. The problems described in issue24160 occurs when those two parts get out of sync.

In interactive mode, each pdb.run() call creates a new instance of Bdb(). In this case the user would like the breakpoints to be remembered between calls. This PR adds a function _load_breaks to Bdb, which is called in its constructor and populates the instance's breaks dict with the data from the Breakpoint class.

Tests were added in both test_bdb (to test it directly) and in test_pdb (to test it with the interactive usecase).

https://bugs.python.org/issue24160

[pppery]

I would have gone the other way with this, and made the breakpoints for different Bdb instances completely independent, rather than completely dependent, but this works too.

[iritkatriel]

That was my first thought as well, but then I realised it would be annoying to have to set the same breakpoints again and again while debugging something. (It’s easy to do ‘clear’ if that’s what you want).

[taleinat]

If possible, I think it would be god for the new tests to be slightly expanded to also include deleting breakpoints.

[taleinat]

Perhaps now Bdb.clear_all_breaks() should use the new clearBreakpoints internally?

[iritkatriel]

If possible, I think it would be god for the new tests to be slightly expanded to also include deleting breakpoints.

I don't see any tests for deleting breakpoint, and indeed coverage seems pretty grim:
https://codecov.io/gh/python/cpython/src/master/Lib/bdb.py

[iritkatriel]

Perhaps now Bdb.clear_all_breaks() should use the new clearBreakpoints internally?

I think this could break something if some Bdb subclass changes the way this data is represented.

[iritkatriel]

It's messy because this class level state is accessed through self.

[taleinat]

Perhaps now Bdb.clear_all_breaks() should use the new clearBreakpoints internally?

I think this could break something if some Bdb subclass changes the way this data is represented.

True, let's leave it as is for now.

[taleinat]

Random question: Maybe someone knows how "lineno" came to be shorthand for "line number"??

[taleinat]

@iritkatriel, this is looking good. I'll make a final review later and likely approve.

[iritkatriel]

Random question: Maybe someone knows how "lineno" came to be shorthand for "line number"??

I asked google and it said something about Lenin.

[gvanrossum]

This looks like a feature, let's not backport it.

[iritkatriel]

Shall we try to get this into 3.10?

[gvanrossum]

SGTM. Is it ready? I haven't reviewed it yet.

[iritkatriel]

I've just rebased it. I think it is ready to merge. @taleinat had reviewed it a while back and we made some improvements.

[gvanrossum]

Okay, from a cursory inspection looks good. I trust @taleinat.