Get the correct computed tb lineno #158
Conversation
Fix for python/cpython#109181 introduced lazily computed lineno for traceback object in 3.11.7 and 3.12.1. Tested in a number of Python versions, the change seems to be safe.
There was a problem hiding this comment.
This does point out that we need to be careful when accessing c python structs that don't have proper / stable APIs. To be technically safe, we would need to be using using PyObject_GetAttrString to access attributes everywhere instead of assuming simple c struct access.
For example, while we are "ok" accessing tb->tb_next as a proxy for the tb_next attribute (https://docs.python.org/3/reference/datamodel.html#traceback.tb_next) a similar change could happen in the future where we'd need to go the official route to access the attribute. https://github.com/python/cpython/blob/v3.12.5/Python/traceback.c#L100-L108
| @@ -25,7 +25,7 @@ jobs: | |||
| distribution: 'temurin' | |||
| java-version: ${{ matrix.java }} | |||
|
|
|||
| - run: pip install setuptools | |||
| - run: pip install "setuptools < 72" | |||
There was a problem hiding this comment.
Why do we need to limit setuptools?
There was a problem hiding this comment.
We reply on setuptools.command.test to run the test suite. See pypa/setuptools#4519 and a solution is out of scope for now. will create a ticket.
| static int get_traceback_lineno(PyTracebackObject *tb) { | ||
| PyObject* po_lineno = PyObject_GetAttrString((PyObject*)tb, "tb_lineno"); | ||
| int lineno = (int)PyLong_AsLong(po_lineno); | ||
| JPy_DECREF(po_lineno); | ||
| return lineno; | ||
| } |
There was a problem hiding this comment.
Took me a little bit to figure out what actually changed. python/cpython#111548 is a useful reference. I'm a bit surprised there's not a more official stable API around PyTracebackObject. I understand the need to wash through PyObject_GetAttrString in this case, but I'm sad.
Does calling PyObject_GetAttrString actually set tb->tb_lineno? I can't tell from the linked PR if that's actually what python does in this case. From the description of the patch
Speed up :obj:
Tracebackobject creation by lazily compute the line number.
it's seems to me that "lazily" means that it will be cached after first access? (If that is not the case, it seems like tb->tb_lineno is completely useless and unused?)
Assuming "yes", we could probably check tb->tb_lineno == -1 before doing the more expensive call?
There was a problem hiding this comment.
No, and PyTracebackObject isn't part of the public stable API, so they are free to change the struct itself and how to use it.
Fix for python/cpython#109181 introduced lazily computed lineno for traceback object in 3.11.7 and 3.12.1. Tested in a number of Python versions, the change seems to be safe.
The problem was initially reported in deephaven/vscode-deephaven#9