[BUG]: tracking issue for PyPy 7.3.6 support (PyPy 3.8)

[henryiii]

Required prerequisites

• [X] Make sure you've read the documentation. Your issue may be addressed there.
• [X] Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
• [X] Consider asking first in the Gitter chat room or in a Discussion.

Problem description

We are having issues with the PyPy update, so I'm going to avoid making it and just list the problems here, eventually opening up another PR (currently mixed into #3368). This will also focus only on supporting it, not on building binaries with it, which is a bad idea (on PyPy3.7, anyway) - users should wait till 7.3.7 to build binaries (and existing ones like possibly NumPy won't work).

For both 3.7 and 3.8:

• [ ] test_to_python returns 1 from cstats.alive()

For 3.8 Unix:

• [ ] test_int_convert does not produce a warning (was not included before)
• [ ] test_numpy_int_convert does not produce a warning (Also was not included before)
• [ ] PyLong_AsLong implementation does not contain the updates made in CPython to call __index__.
• [ ] test_eval_empty_globals does not contain __builtins__ (or anything).
• [ ] test_python_alreadyset_in_destructor returns False from triggered[0]

For 3.8 Windows:

• [ ] Dies with ImportError: initialization failed. Probably from the issue we've always had on 3.8 not being able to run the cross module tests on Windows.

For 2.7 macOS:

It seems the parallel setuptools_helpers test hangs. Could be intermittent, a CI issue, or something else, but could be related to PyPy 7.3.6. Edit: rerunning made it pass.

[henryiii]

The now-failing test links to https://foss.heptapod.net/pypy/pypy/-/issues/2444 (CC @mattip), but instead of working, it now seems to be leaking, perhaps? Since this runs on both CPython and PyPY, I'm assuming the test was working before, even though it has an issue at the top.

https://github.com/pybind/pybind11/blob/d45a88105c3dab1dc2a1da831f1c83a0f3b57f8b/tests/test_buffers.py#L38-L70

For the two tests not producing warnings for int conversion, I think that's a difference in implementation (but needs to be checked, CC @YannickJadoul.

https://github.com/pybind/pybind11/blob/d45a88105c3dab1dc2a1da831f1c83a0f3b57f8b/tests/test_builtin_casters.py#L335-L342

For test_eval_empty_globals, this seems to be a difference between CPython and PyPy; starting in 3.8, CPython injects __builtins__ with PyRun_String, while PyPy doesn't. :'( We can make our workaround include PyPy on 3.8. include/pybind11/eval.h contains the workaround, and it's pretty safe, just costs an extra Python containership check.

https://github.com/pybind/pybind11/blob/d45a88105c3dab1dc2a1da831f1c83a0f3b57f8b/include/pybind11/eval.h#L21-L32

Looks like the block patching in the hook in 3.8 doesn't work in PyPy 3.8.

https://github.com/pybind/pybind11/blob/d45a88105c3dab1dc2a1da831f1c83a0f3b57f8b/tests/test_exceptions.py#L102-L122

[mattip]

The now-failing test links to https://foss.heptapod.net/pypy/pypy/-/issues/2444, but instead of working, it now seems to be leaking, perhaps?

It would be good to track this down, since that issue is closed. We yet again modified buffer handling for PyPy3.8 and maybe missed something. With that, the implementation of pytest.gc_collect() calls gc.collect() twice. In NumPy's implementation, I needed 3 calls.

starting in 3.8, CPython injects builtins with PyRun_String, while PyPy doesn't.

Do you know where/how this change happened? I don't see it documented for PyRun_String nor in the 3.8 changelog. Maybe this is not 3.8-specific or is it more general to all invocations of eval?

For sys.unraisablehook, I opened https://foss.heptapod.net/pypy/pypy/-/issues/3583

[henryiii]

Do you know where/how this change happened?

https://github.com/pybind/pybind11/pull/2616 points at https://github.com/python/cpython/pull/13362, the implementation of PEP 587.

[mattip]

For __builtins__, I opened https://foss.heptapod.net/pypy/pypy/-/issues/3584

[mattip]

For sys.unraisablehook, I opened https://foss.heptapod.net/pypy/pypy/-/issues/3583

I think that might have been on PyPy3.7, sys.unraisablehook is there on PyPy3.8.

[henryiii]

I think it's there, but it doesn't work. The code path does trigger to set it, but it doesn't do anything.

[mattip]

Strange. sys.unraisablehook = ... is used in Lib/test/support for a catch_unraisable_exceptions context manager, and, for instance, we are passing test_coroutines::test_fatal_coro_warning which uses it, so I am not sure what is going on.

[henryiii]

I'm rerunning it (as part of something else) in #3419 - I'll see if I can see what part is failing.

[henryiii]

Interesting, for failure 1, that happens on PyPy3.7 7.3.7 as well (wow that's a lot of threes and sevens). So it's a change with all PyPy, not just the 3.8 branch. It used to behave more like CPython, and now isn't - though like anything related to the garbage collector, I'm not sure it's so critical. I'll try multiple calls, which I think is what was suggested above.

[henryiii]

Another PyPy 3.8 issue: Before Python 3.8, PyLong_AsLong does not pick up on obj.__index__. PyPy 3.8 seems to trigger the old 3.7 and before behavior, rather than using __index__ (at least that's what it looks like at the moment).

Also, there's a missing DeprecationWarning in PyLong_AsLong. I think that's from CPython and not us.

I see difference between PyPy and CPython's PyIndex_Check. Pretty sure that's unrelated.

[henryiii]

We can (and do) backport that behavior for 3.7 and before, but it should be supported by PyPy3.8 directly, I think.

[mattip]

It is a bit hard to manage all this in a github issue conversation. Can you point to the failing tests?

[henryiii]

Sure, ignore the failing Python 3.11 tests, I'm enabling 3.11 and the new PyPy in the same PR for testing, #3419. If you scroll to the bottom of https://github.com/pybind/pybind11/pull/3419/files, you can see the failures annotated in place.

PyPy 3.7: https://github.com/pybind/pybind11/runs/4027080761?check_suite_focus=true The garbage collector test is broken. I can try adding three calls.

PyPy 3.8: https://github.com/pybind/pybind11/runs/4027080807?check_suite_focus=true

The test_int_convert test is broken, IntAndIndex() returns the value for __int__() instead of __index__() from PyLong_AsLong.

test_python_alreadyset_in_destructor is broken, as we've discussed above. I think I can make a MWE if I can throw an unraisable exception in pure Python.

I'm ignoring the float conversion lack-of-warnings for now.

[henryiii]

Here's an old workaround for the difference in PyIndex_Check:

https://github.com/pybind/pybind11/blob/a80b22374ae9398962fe9746f84edc7e7982f9f7/include/pybind11/cast.h#L135-L141

But I don't think that affects anything we are seeing here. This is the block implementing the Python 3.8 behavior for all versions of Python for long conversion:

https://github.com/pybind/pybind11/blob/a80b22374ae9398962fe9746f84edc7e7982f9f7/include/pybind11/cast.h#L153-L174

And I think this is also where CPython 3.8-3.9 produce a DeprecationWarning, while PyPy does not, if you call PyLong_AsLong on a float.

[mattip]

Ahh, so the difference is PyIndex_Check not PyLong_AsLong. That makes more sense, since __int__ has a higher priority than __index__.

[henryiii]

PyLong_AsLong calls __index__ first, and falls back to __int__ if that's missing and produces a warning that the fallback is deprecated behavior in Python 3.8 & 3.9. In Python 3.10, it does not call __int__ at all. https://docs.python.org/3/c-api/long.html#c.PyLong_AsLong

__int__ has a higher priority than __index__

If you call int(x), yes. Which makes sense, that's what int should call - it's a conversion function. __int__ is a (potentially lossy) conversion to an int. But this is not int(), this is PyLong_*, which tries to get its "lossless representation" as an int (which is exactly what __index__ represents).

[mattip]

Thanks. This part was not done for PyPy.

Changed in version 3.8: Use __index__() if available.

[henryiii]

I've downloaded PyPy 3.8 via the web to test locally and can't really use it because I'm getting spammed with "Cannot be opened because the developer cannot be verified" for every .so. Any suggestions for how to fix all those warnings at once?

[mattip]

I assume you are on macOS. There are guides somewhere about marking the file as "allowed". Is it up on homebrew? One trick that used to work is to download it via curl or wget and not via a browser.

[henryiii]

I can try wget. Homebrew is just now updating to 7.3.7 (as in it's available now but not when I sent the last message), but [email protected] hasn't made it in yet (there's a PR for it, but it's failing ATM). This is kind of a weird usage of the "@" version for brew, but... Okay.

[henryiii]

So the current status:

PyLong_AsLong: I've added the backported behavior for 2.7-3.7 to pypy for index. Shouldn't be harmful even if PyPy fixes this, just a bit wasteful. This fixes three items.

__builtins__: Again, old behavior also added to PyPy. It's just a wasted check if PyPy fixes this.

Garbage collector buffer test: Adding one or two more calls to the garbage collector fixes everything except PyPy3.7 on Ubuntu. That still is not cleaning up.

test_python_alreadyset_in_destructor: The problem here is PyPy is setting obj to None, rather than a string. If I try to make a MWE, I know how to make obj a function, but not a string. It works when it's a function (like Unraisable.__del__). Maybe an issue with PyErr_WriteUnraisable? Definition looks okay.

[mattip]

The problem here is PyPy is setting obj to None, rather than a string

Where is the value already_set demo coming from?

[henryiii]

It's from this call: https://github.com/pybind/pybind11/blob/c8ce4b8df854a630a5f02192305c183299778b84/tests/test_exceptions.py#L120

Which triggers this call with the given string object as s:

https://github.com/pybind/pybind11/blob/a80b22374ae9398962fe9746f84edc7e7982f9f7/tests/test_exceptions.cpp#L101

Which triggers this CAPI:

https://github.com/pybind/pybind11/blob/a80b22374ae9398962fe9746f84edc7e7982f9f7/include/pybind11/pytypes.h#L361-L364

[henryiii]

I'd guess it triggers this in PyPy, which looks fine to me:

https://foss.heptapod.net/pypy/pypy/-/blob/cc9dd0b02ff856eadb38ea55ca1b7e7b5290c852/pypy/module/cpyext/pyerrors.py#L339-353

[henryiii]

And this is my first attempt at a MWE, but it seems to work identically in python and pypy3.8:

import sys
import gc

triggered = [False]  # mutable

default_hook = sys.__unraisablehook__


class Unraisable:
    def __del__(self):
        raise Exception("demo")


def hook(unraisable_hook_args):
    exc_type, exc_value, exc_tb, err_msg, obj = unraisable_hook_args
    if obj == Unraisable.__del__:
        triggered[0] = True
    default_hook(unraisable_hook_args)
    return


sys.unraisablehook = hook


def demo():
    x = Unraisable()
    del x
    gc.collect()


demo()
assert triggered[0], "This printout should not be shown"

Might need CAPI to go further, unless there's a way to get at this more easily.

[mattip]

How critical is this problem?

[henryiii]

Not very. It's just nice to have as much parity as possible between CPython and PyPy, and the less we have to work around, the better it is for other users too. But I can ignore the remaining tests on PyPy, ping the author of the unraisable code to see if it's worth a caveat in the docs, and go forward. The danger is that makes it easy to forget. :/

[mattip]

There are so many things to work on, I don't want to get bogged down in this corner case.

The danger is that makes it easy to forget. :/

Sure, an issue on the pypy tracker would prevent that.

[mattip]

Here is a new failure. I think it can be described as: calling std::cout << value.cast<py::list>() << std::endl succeeds in CPython and fails to compile in PyPy. This case is hit when value is

class a:
        pass

value = [a()]

~Edit: apparently that is a regression somewhere between pybind11 2.5 to 2.8~ Nope, not a regression.

[mattip]

The test_int_convert test is broken, IntAndIndex() returns the value for __int__() instead of __index__() from PyLong_AsLong.

This is fixed on latest pypy3.8 nightlies.