Repository navigation
PEP 553 built-in debug() function (bpo-31353) #3355
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.
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
Changes from 21 commits
9dc577a
bc4634d
f7e3c40
6696c23
4600bcf
0f46af7
a141f63
79d7ac8
1666925
8437033
6534542
2d75d7a
ed649fa
0bea882
eee151d
05b7cae
6b9a59c
5e0b93a
c57d0dc
eb17d0f
244bff5
c4bed66
6eb3470
af81484
f3200c7
1c60e11
ed41006
b84a018
f6db9e3
cf9e7ac
e46c59d
9bc36b8
b5c2393
2846d3b
50c3e40
6b23d5b
0d2fae5
1cf3e06
6217e90
28bd8e4
8808367
bd68b76
fea5d3d
599108d
a74f23e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| PEP 553 - Add a new built-in called ``breakpoint()`` which calls | ||
| ``sys.breakpointhook()``. By default this imports ``pdb`` and calls | ||
| ``pdb.set_trace()``, but users may override ``sys.breakpointhook()`` to call | ||
| whatever debugger they want. The original value of the hook is saved in | ||
| ``sys.__breakpointhook__``. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -415,6 +415,30 @@ builtin_callable(PyObject *module, PyObject *obj) | |
| return PyBool_FromLong((long)PyCallable_Check(obj)); | ||
| } | ||
|
|
||
| static PyObject * | ||
| builtin_breakpoint(PyObject *self, PyObject *args, PyObject *keywords) | ||
| { | ||
| PyObject *hook = PySys_GetObject("breakpointhook"); | ||
| PyObject *retval = NULL; | ||
|
|
||
| if (hook == NULL) { | ||
| PyErr_SetString(PyExc_RuntimeError, "lost sys.breakpointhook"); | ||
| return NULL; | ||
| } | ||
| Py_INCREF(hook); | ||
| retval = PyObject_Call(hook, args, keywords); | ||
| Py_DECREF(hook); | ||
| return retval; | ||
| } | ||
|
|
||
| PyDoc_STRVAR(breakpoint_doc, | ||
| "breakpoint(*args, **kws)\n\ | ||
| \n\ | ||
| Call the sys.breakpointhook() function, passing *args and **kws\n\ | ||
| through unchanged. sys.breakpointhook() must accept the arguments\n\ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not simply "Call sys.breakpointhook(*args, **kwds)."? It's just a suggestion. I don't know what is the best. |
||
| passed.\n\ | ||
| \n\ | ||
| By default, this drops you into the pdb debugger."); | ||
|
|
||
| typedef struct { | ||
| PyObject_HEAD | ||
|
|
@@ -2620,6 +2644,7 @@ static PyMethodDef builtin_methods[] = { | |
| BUILTIN_ANY_METHODDEF | ||
| BUILTIN_ASCII_METHODDEF | ||
| BUILTIN_BIN_METHODDEF | ||
| {"breakpoint", (PyCFunction)builtin_breakpoint, METH_VARARGS | METH_KEYWORDS, breakpoint_doc}, | ||
| BUILTIN_CALLABLE_METHODDEF | ||
| BUILTIN_CHR_METHODDEF | ||
| BUILTIN_COMPILE_METHODDEF | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,6 +93,32 @@ PySys_SetObject(const char *name, PyObject *v) | |
| return PyDict_SetItemString(sd, name, v); | ||
| } | ||
|
|
||
| static PyObject * | ||
| sys_breakpointhook(PyObject *self, PyObject *noargs) | ||
| { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add "assert(!PyErr_Occurred());" at the entry point since this function is called when something goes wrong, and the implementation don't expect that an exception is currently set. ... Maybe even raise an exception at runtime if it's called with an exception set? See _Py_CheckFunctionResult() which implements a similar check at exit point.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that true, i.e. that it's possible to call this with an exception set? I don't mind adding an assertion for that, but I'm not sure I see how it's possible.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In my experience, it's very common that a function returning a result and set an exception is not catched immediately, but somewhere "later". I had such issues when I ran my pyfailmalloc tool which injects MemoryError anywhere. But I made many changes in CPython internals to add assert(!PyErr_Occurred()) at many places. The main change is that all functions calling functions should now check that no exception is raised if the called function returned a result. This is _Py_CheckFunctionResult(). To come back to your breakpoint() implementation, "assert(!PyErr_Occurred());" should be enough.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to be clear, you're suggesting
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. |
||
| PyObject *pdb = PyImport_ImportModule("pdb"); | ||
| PyObject *set_trace = NULL; | ||
| PyObject *retval = NULL; | ||
|
|
||
| if (pdb == NULL) { | ||
| return NULL; | ||
| } | ||
| set_trace = PyObject_GetAttrString(pdb, "set_trace"); | ||
| Py_DECREF(pdb); | ||
| if (set_trace == NULL) { | ||
| return NULL; | ||
| } | ||
| retval = PyObject_CallObject(set_trace, NULL); | ||
| Py_DECREF(set_trace); | ||
| return retval; | ||
| } | ||
|
|
||
| PyDoc_STRVAR(breakpointhook_doc, | ||
| "breakpointhook()\n" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's unclear to me if the prototype is breakpointhook() or breakpointhook(*args, **kw)? |
||
| "\n" | ||
| "Called when the built-in breakpoint() function is called.\n" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest to repeat the first sentence of its doc: |
||
| ); | ||
|
|
||
| /* Write repr(o) to sys.stdout using sys.stdout.encoding and 'backslashreplace' | ||
| error handler. If sys.stdout has a buffer attribute, use | ||
| sys.stdout.buffer.write(encoded), otherwise redecode the string and use | ||
|
|
@@ -1360,6 +1386,7 @@ sys_getandroidapilevel(PyObject *self) | |
|
|
||
| static PyMethodDef sys_methods[] = { | ||
| /* Might as well keep this in alphabetic order */ | ||
| {"breakpointhook", sys_breakpointhook, METH_NOARGS, breakpointhook_doc}, | ||
| {"callstats", (PyCFunction)sys_callstats, METH_NOARGS, | ||
| callstats_doc}, | ||
| {"_clear_type_cache", sys_clear_type_cache, METH_NOARGS, | ||
|
|
@@ -1951,6 +1978,9 @@ _PySys_BeginInit(void) | |
| PyDict_GetItemString(sysdict, "displayhook")); | ||
| SET_SYS_FROM_STRING_BORROW("__excepthook__", | ||
| PyDict_GetItemString(sysdict, "excepthook")); | ||
| SET_SYS_FROM_STRING_BORROW( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your indentation here is different from what's already present here. While I personally prefer how you've done it, local inconsistency is ugly.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, but unfortunately that's the only way to not break 80 columns (or maybe "the least horrible way").
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing the indentation of the other entries should not be discarded as an option if that allows PEP 7 conformance. Not ideal, perhaps, but allows local consistency.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I want to touch lines outside those related to this PEP.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the interest of local consistency, that could be done after PEP code is landed (assuming acceptance, which sounds likely), as a separate commit. Otherwise, some way to conform would be valuable. |
||
| "__breakpointhook__", | ||
| PyDict_GetItemString(sysdict, "breakpointhook")); | ||
| SET_SYS_FROM_STRING("version", | ||
| PyUnicode_FromString(Py_GetVersion())); | ||
| SET_SYS_FROM_STRING("hexversion", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP 553 can be written:
to get a direct link to the PEP ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn’t docutils auto-linkify text that looks like
RFC 1234orPEP 123?