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

[henryiii]

Required prerequisites

• Make sure you've read the documentation. Your issue may be addressed there.
• Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
• 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.

pybind11/tests/test_buffers.py

Lines 38 to 70 in d45a881

	# https://foss.heptapod.net/pypy/pypy/-/issues/2444
	def test_to_python():
	mat = m.Matrix(5, 4)
	assert memoryview(mat).shape == (5, 4)
	assert mat[2, 3] == 0
	mat[2, 3] = 4.0
	mat[3, 2] = 7.0
	assert mat[2, 3] == 4
	assert mat[3, 2] == 7
	assert struct.unpack_from("f", mat, (3 * 4 + 2) * 4) == (7,)
	assert struct.unpack_from("f", mat, (2 * 4 + 3) * 4) == (4,)
	mat2 = np.array(mat, copy=False)
	assert mat2.shape == (5, 4)
	assert abs(mat2).sum() == 11
	assert mat2[2, 3] == 4 and mat2[3, 2] == 7
	mat2[2, 3] = 5
	assert mat2[2, 3] == 5
	cstats = ConstructorStats.get(m.Matrix)
	assert cstats.alive() == 1
	del mat
	pytest.gc_collect()
	assert cstats.alive() == 1
	del mat2 # holds a mat reference
	pytest.gc_collect()
	assert cstats.alive() == 0
	assert cstats.values() == ["5x4 matrix"]
	assert cstats.copy_constructions == 0
	# assert cstats.move_constructions >= 0 # Don't invoke any
	assert cstats.copy_assignments == 0
	assert cstats.move_assignments == 0

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.

pybind11/tests/test_builtin_casters.py

Lines 335 to 342 in d45a881

	# The implicit conversion from np.float32 is undesirable but currently accepted.
	# TODO: Avoid DeprecationWarning in `PyLong_AsLong` (and similar)
	if (3, 8) <= env.PY < (3, 10):
	with env.deprecated_call():
	assert convert(np.float32(3.14159)) == 3
	else:
	assert convert(np.float32(3.14159)) == 3
	require_implicit(np.float32(3.14159))

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.

pybind11/include/pybind11/eval.h

Lines 21 to 32 in d45a881

	inline void ensure_builtins_in_globals(object &global) {
	#if PY_VERSION_HEX < 0x03080000
	// Running exec and eval on Python 2 and 3 adds `builtins` module under
	// `__builtins__` key to globals if not yet present.
	// Python 3.8 made PyRun_String behave similarly. Let's also do that for
	// older versions, for consistency.
	if (!global.contains("__builtins__"))
	global["__builtins__"] = module_::import(PYBIND11_BUILTINS_MODULE);
	#else
	(void) global;
	#endif
	}

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

pybind11/tests/test_exceptions.py

Lines 102 to 122 in d45a881

	hooked = False
	triggered = [False] # mutable, so Python 2.7 closure can modify it
	if hasattr(sys, "unraisablehook"): # Python 3.8+
	hooked = True
	# Don't take `sys.unraisablehook`, as that's overwritten by pytest
	default_hook = sys.__unraisablehook__
	def hook(unraisable_hook_args):
	exc_type, exc_value, exc_tb, err_msg, obj = unraisable_hook_args
	if obj == "already_set demo":
	triggered[0] = True
	default_hook(unraisable_hook_args)
	return
	# Use monkeypatch so pytest can apply and remove the patch as appropriate
	monkeypatch.setattr(sys, "unraisablehook", hook)
	assert m.python_alreadyset_in_destructor("already_set demo") is True
	if hooked:
	assert triggered[0] is True

[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?

#2616 points at python/cpython#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:

pybind11/include/pybind11/cast.h

Lines 135 to 141 in a80b223

	#if !defined(PYPY_VERSION)
	auto index_check = [](PyObject *o) { return PyIndex_Check(o); };
	#else
	// In PyPy 7.3.3, `PyIndex_Check` is implemented by calling `__index__`,
	// while CPython only considers the existence of `nb_index`/`__index__`.
	auto index_check = [](PyObject *o) { return hasattr(o, "__index__"); };
	#endif

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:

pybind11/include/pybind11/cast.h

Lines 153 to 174 in a80b223

	#if PY_VERSION_HEX < 0x03080000
	object index;
	if (!PYBIND11_LONG_CHECK(src.ptr())) { // So: index_check(src.ptr())
	index = reinterpret_steal<object>(PyNumber_Index(src.ptr()));
	if (!index) {
	PyErr_Clear();
	if (!convert)
	return false;
	}
	else {
	src_or_index = index;
	}
	}
	#endif
	if (std::is_unsigned<py_type>::value) {
	py_value = as_unsigned<py_type>(src_or_index.ptr());
	} else { // signed integer:
	py_value = sizeof(T) <= sizeof(long)
	? (py_type) PyLong_AsLong(src_or_index.ptr())
	: (py_type) PYBIND11_LONG_AS_LONGLONG(src_or_index.ptr());
	}
	}

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 pypy3@3.8 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:

pybind11/tests/test_exceptions.py

Line 120 in c8ce4b8

assert m.python_alreadyset_in_destructor("already_set demo") is True

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

pybind11/tests/test_exceptions.cpp

Line 101 in a80b223

ex.discard_as_unraisable(s);

Which triggers this CAPI:

pybind11/include/pybind11/pytypes.h

Lines 361 to 364 in a80b223

	void discard_as_unraisable(object err_context) {
	restore();
	PyErr_WriteUnraisable(err_context.ptr());
	}

[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.

[henryiii]

std::cout << py::cast<py::list>(value) << std::endl

I tried a little test and this seemed to be fine.

diff --git a/noxfile.py b/noxfile.py
index 4adffac2..3eedd455 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -2,7 +2,7 @@ import nox

 nox.options.sessions = ["lint", "tests", "tests_packaging"]

-PYTHON_VERISONS = ["2.7", "3.5", "3.6", "3.7", "3.8", "3.9", "3.10", "3.11"]
+PYTHON_VERISONS = ["2.7", "3.5", "3.6", "3.7", "3.8", "3.9", "3.10", "3.11", "pypy3.7"]


 @nox.session(reuse_venv=True)
diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index bc5c6553..ce2953fd 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -522,4 +522,7 @@ TEST_SUBMODULE(stl, m) {
         []() { return new std::vector<bool>(4513); },
         // Without explicitly specifying `take_ownership`, this function leaks.
         py::return_value_policy::take_ownership);
+
+    // Test list printing
+    m.def("print_as_list", [](const py::object &any){std::cout << py::cast<py::list>(any);});
 }
diff --git a/tests/test_stl.py b/tests/test_stl.py
index e2179759..d939ce4f 100644
--- a/tests/test_stl.py
+++ b/tests/test_stl.py
@@ -356,3 +356,14 @@ def test_return_vector_bool_raw_ptr():
     v = m.return_vector_bool_raw_ptr()
     assert isinstance(v, list)
     assert len(v) == 4513
+
+
+def test_print_list(capsys):
+    m.print_as_list([])
+
+    class A:
+        pass
+
+    m.print_as_list([A()])
+
+    assert False

This spits out [][<test_stl.test_print_list.<locals>.A object at 0x00007fa2a8324020>] on both CPython and PyPy.

[mattip]

Maybe there is some subtle difference between value.cast<py::list>() and py::cast<py::list>(value)?

[henryiii]

There shouldn't be, but just I tried that anyway, same results. Does the same thing happen if you do throw py::type_error(py::str("not a valid type for conversion {}").format(it));?

[henryiii]

Unrelated: @mattip could PyPy binaries for macOS please be built with 10.9+ instead of 10.7+ compatibility like the CPython binaries? Setting 10.7 means that these are broken out-of-the box when trying to build, since 10.9 was the first macOS version to use libc++, and libstdc++ was removed a few years ago, meaning targeting anything less than 10.9 is broken on modern macOS's. And the extension default is to match what the Python binaries target, and they target 10.7.

[mattip]

please be built with 10.9+ instead of 10.7+ compatibility like the CPython binaries

Do you know which PR bumped it?

[henryiii]

It used to be 10.6 when they provided FAT binaries, before they went to 64 bit only, that's when they became 10.9+ only. It's never been 10.7, IIRC pypy needed 10.7 instead of 10.6 for some reason. But you really, really want to be 10.9, due to the change to libc++. I can't get pytomlpp to build locally because it insists on 10.7 and then can't find libstdc++. Edit: because I'm misspelling the MACOS(X)_DEPLOYMENT_TARGET.

See https://www.python.org/downloads/release/python-365/ -> macOS users, and this was also backported to 2.7 (but not 3.5, which had already finished binary releases). Python 3.8.0 was the first to remove the old 32/64 binary format entirely.

[henryiii]

Also, it seems like PyPy is burning in 10.7 regardless of the OS it was built on when nothing is set. While Python defaults to the current OS version, and only the distributed binaries have older versions set. This means homebrew python, conda-forge python, etc. all have a much higher OS version than the official binaries, which is why we only use the official binaries for building wheels in cibuildwheel. But it also means that users trying to build code locally aren't having to worry about old compatibility issues like limited C++17 support (unless they download the official binaries, which many macOS users do not do).

[mattip]

Does PyPy from conda-forge/homebrew also pin to 10.7?

[mattip]

Also, it seems like PyPy is burning in 10.7 regardless of the OS it was built on when nothing is set

Sorry, I am not really fluent in macOS. What is wrong and what would the fix look like? I didn't understand the rest of the comment.

[mattip]

d={}; eval("1", d); assert "__builtins__" in d now works on PyPy nightly.

[mattip]

@henryiii PyPy has somewhat redone its macOS builds to accomodate arm64 builds. Do the nightlies (py3.8, py3.9) work any better now?