Define all Pdb's instance variable in `__init__`

[Carreau]

Pdb currently have a few instances variables that are not defined in __init__.

Mainly as far as I can tell, :

self.lineno
self.stack
self.curindex
self.curframe
self.currentbp
self._user_requested_quit

(not set in Bdb, or Cmd superclass either)

This means that accessing these requires a hasattr/getattr, which we can see in the Pdb's implementation in a few places. This is a footgun for a subclasses, and in particular for IPython, in some circumstances, the gettattr start to have a non-negligible perf impact (We support things like having __debuggerskip__ = True|False in a frame, so the getattr(self, 'currentframe',...) start to get expensive.

https://github.com/ipython/ipython/blob/f74b500b0f3061452898ed51cb44e05ebda01cd6/IPython/core/debugger.py#L595-L615

It would be great if all those were initialized in __init__; Yes I can initialize in my __init__ subclass, but having it in CPython should benefit everybody, plus it would allow the removal of a few getattr() in CPython source directly.

Personally I care mostly about curframe, but while at it maybe do the other ones.

I am not aware of anything that uses the fact the attribute is missing.

Linked PRs

• gh-140790: pdb: Initialize instance variables in Pdb.__init__ #140791

[picnixz]

• curframe is conditionally checked on its existence depending in forget() but I don't think it's relevant. One can just have it as None for a sentinel.
• currentbp looks special, there is a comment: "# self.currentbp is set in bdb in Bdb.break_here if a breakpoint was hit" and I think it should remains as is.
• For the rest it looks fine to change them.

I think curframe can be changed but currentbp seems to be an attribute of Bdb instead of Pdb.

cc @gaogaotiantian

[gaogaotiantian]

I need some time to take a deeper look into this. It might be a breaking change for other debuggers besides IPython. In general I agree that it would be much better if we can just define everyting in __init__. In practice, pdb has been there for many years and any issue could be a feature people rely on.

I still think we can probably move forward on this, just need some careful review and thought.

[lysnikolaou]

I did a semi-thorough look around GitHub to see whether there's any code that depends on curframe not existing. I couldn't find anything. Also, the attached PR does not change currentbp.

@gaogaotiantian Does this maybe help push the attached PR forward? Is there anything else that we can do that would make the decision to merge easier?

[gaogaotiantian]

@lysnikolaou yes that is very helpful. The other attributes are safer. self.curframe was there for too long and I'm a bit hesitated, glad to know that there's no code that we can find to rely on it.