This fixes
SystemError: PyEval_EvalFrameEx returned a result with an error set
in test-storage.py on Python 3. I believe we should audit *all*
callsites of PyInt_AS_LONG and related macros to hunt for similar
bugs. :(
indygreg |
hg-reviewers |
This fixes
SystemError: PyEval_EvalFrameEx returned a result with an error set
in test-storage.py on Python 3. I believe we should audit *all*
callsites of PyInt_AS_LONG and related macros to hunt for similar
bugs. :(
Lint Skipped |
Unit Tests Skipped |
IIRC we #define PyInt and PyLong in the source for Python 3 compatibility. I remember doing something really hacky to unblock getting the C extensions to compile on Python 3 at the Paris sprint. It would definitely be a good idea to audit everything with PyInt and PyLong in it for sanity.
In this case, I suspect something really weird because PyInt_AS_LONG() doesn't exist on Python 3 and the docs for Python 2 say it performs no error checking. So the fact that it is setting an exception on Python 3 causes me to raise an eyebrow. Would you mind spending an extra few minutes investigating what's actually going on and explain it in the commit message?
In this case, I suspect something really weird because `PyInt_AS_LONG()` doesn't exist on Python 3 and the docs for Python 2 say it performs no error checking. So the fact that it is setting an exception on Python 3 causes me to raise an eyebrow.
It's #define PyLong_AS_LONG(op) PyLong_AsLong(op), sigh. And the reason
of the lack of PyLong_AS_LONG(op) would be that Python 3 has no bounded
integer type.
+ if (PyInt_Check(value)) { + long arg = PyInt_AS_LONG(value);
In this case, we'll probably have to replace PyInt_Check() + PyInt_AS_LONG()
with PyInt_AsLong() + PyErr_Occurred().
Ugh, good catch. How about this:
https://phab.mercurial-scm.org/D5235
I'm not super in love with it, but it feels somewhat better encapsulated this way. Seems to work right on both versions...