From a48ec3e8820064511aac5fab71706166bdd590d3 Mon Sep 17 00:00:00 2001 From: Brad Messer Date: Wed, 24 Aug 2022 10:34:31 -0400 Subject: [PATCH 01/25] Words matter updates (#4155) * Remove sanity check from code base. * Use main over master. * Better alternative that doesn't collide with language keywords/frequent usage words. --- include/pybind11/numpy.h | 2 +- tests/test_eigen.py | 68 ++++++++++++++++++++-------------------- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 56e5edc5a..369e18649 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1401,7 +1401,7 @@ PYBIND11_NOINLINE void register_structured_dtype(any_container oss << '}'; auto format_str = oss.str(); - // Sanity check: verify that NumPy properly parses our buffer format string + // Smoke test: verify that NumPy properly parses our buffer format string auto &api = npy_api::get(); auto arr = array(buffer_info(nullptr, itemsize, format_str, 1)); if (!api.PyArray_EquivTypes_(dtype_ptr, arr.dtype().ptr())) { diff --git a/tests/test_eigen.py b/tests/test_eigen.py index a1c114aed..9c6485de3 100644 --- a/tests/test_eigen.py +++ b/tests/test_eigen.py @@ -251,14 +251,14 @@ def array_copy_but_one(a, r, c, v): def test_eigen_return_references(): """Tests various ways of returning references and non-referencing copies""" - master = np.ones((10, 10)) + primary = np.ones((10, 10)) a = m.ReturnTester() a_get1 = a.get() assert not a_get1.flags.owndata and a_get1.flags.writeable - assign_both(a_get1, master, 3, 3, 5) + assign_both(a_get1, primary, 3, 3, 5) a_get2 = a.get_ptr() assert not a_get2.flags.owndata and a_get2.flags.writeable - assign_both(a_get1, master, 2, 3, 6) + assign_both(a_get1, primary, 2, 3, 6) a_view1 = a.view() assert not a_view1.flags.owndata and not a_view1.flags.writeable @@ -271,25 +271,25 @@ def test_eigen_return_references(): a_copy1 = a.copy_get() assert a_copy1.flags.owndata and a_copy1.flags.writeable - np.testing.assert_array_equal(a_copy1, master) + np.testing.assert_array_equal(a_copy1, primary) a_copy1[7, 7] = -44 # Shouldn't affect anything else - c1want = array_copy_but_one(master, 7, 7, -44) + c1want = array_copy_but_one(primary, 7, 7, -44) a_copy2 = a.copy_view() assert a_copy2.flags.owndata and a_copy2.flags.writeable - np.testing.assert_array_equal(a_copy2, master) + np.testing.assert_array_equal(a_copy2, primary) a_copy2[4, 4] = -22 # Shouldn't affect anything else - c2want = array_copy_but_one(master, 4, 4, -22) + c2want = array_copy_but_one(primary, 4, 4, -22) a_ref1 = a.ref() assert not a_ref1.flags.owndata and a_ref1.flags.writeable - assign_both(a_ref1, master, 1, 1, 15) + assign_both(a_ref1, primary, 1, 1, 15) a_ref2 = a.ref_const() assert not a_ref2.flags.owndata and not a_ref2.flags.writeable with pytest.raises(ValueError): a_ref2[5, 5] = 33 a_ref3 = a.ref_safe() assert not a_ref3.flags.owndata and a_ref3.flags.writeable - assign_both(a_ref3, master, 0, 7, 99) + assign_both(a_ref3, primary, 0, 7, 99) a_ref4 = a.ref_const_safe() assert not a_ref4.flags.owndata and not a_ref4.flags.writeable with pytest.raises(ValueError): @@ -297,23 +297,23 @@ def test_eigen_return_references(): a_copy3 = a.copy_ref() assert a_copy3.flags.owndata and a_copy3.flags.writeable - np.testing.assert_array_equal(a_copy3, master) + np.testing.assert_array_equal(a_copy3, primary) a_copy3[8, 1] = 11 - c3want = array_copy_but_one(master, 8, 1, 11) + c3want = array_copy_but_one(primary, 8, 1, 11) a_copy4 = a.copy_ref_const() assert a_copy4.flags.owndata and a_copy4.flags.writeable - np.testing.assert_array_equal(a_copy4, master) + np.testing.assert_array_equal(a_copy4, primary) a_copy4[8, 4] = 88 - c4want = array_copy_but_one(master, 8, 4, 88) + c4want = array_copy_but_one(primary, 8, 4, 88) a_block1 = a.block(3, 3, 2, 2) assert not a_block1.flags.owndata and a_block1.flags.writeable a_block1[0, 0] = 55 - master[3, 3] = 55 + primary[3, 3] = 55 a_block2 = a.block_safe(2, 2, 3, 2) assert not a_block2.flags.owndata and a_block2.flags.writeable a_block2[2, 1] = -123 - master[4, 3] = -123 + primary[4, 3] = -123 a_block3 = a.block_const(6, 7, 4, 3) assert not a_block3.flags.owndata and not a_block3.flags.writeable with pytest.raises(ValueError): @@ -321,18 +321,18 @@ def test_eigen_return_references(): a_copy5 = a.copy_block(2, 2, 2, 3) assert a_copy5.flags.owndata and a_copy5.flags.writeable - np.testing.assert_array_equal(a_copy5, master[2:4, 2:5]) + np.testing.assert_array_equal(a_copy5, primary[2:4, 2:5]) a_copy5[1, 1] = 777 - c5want = array_copy_but_one(master[2:4, 2:5], 1, 1, 777) + c5want = array_copy_but_one(primary[2:4, 2:5], 1, 1, 777) a_corn1 = a.corners() assert not a_corn1.flags.owndata and a_corn1.flags.writeable a_corn1 *= 50 a_corn1[1, 1] = 999 - master[0, 0] = 50 - master[0, 9] = 50 - master[9, 0] = 50 - master[9, 9] = 999 + primary[0, 0] = 50 + primary[0, 9] = 50 + primary[9, 0] = 50 + primary[9, 9] = 999 a_corn2 = a.corners_const() assert not a_corn2.flags.owndata and not a_corn2.flags.writeable with pytest.raises(ValueError): @@ -340,22 +340,22 @@ def test_eigen_return_references(): # All of the changes made all the way along should be visible everywhere # now (except for the copies, of course) - np.testing.assert_array_equal(a_get1, master) - np.testing.assert_array_equal(a_get2, master) - np.testing.assert_array_equal(a_view1, master) - np.testing.assert_array_equal(a_view2, master) - np.testing.assert_array_equal(a_ref1, master) - np.testing.assert_array_equal(a_ref2, master) - np.testing.assert_array_equal(a_ref3, master) - np.testing.assert_array_equal(a_ref4, master) - np.testing.assert_array_equal(a_block1, master[3:5, 3:5]) - np.testing.assert_array_equal(a_block2, master[2:5, 2:4]) - np.testing.assert_array_equal(a_block3, master[6:10, 7:10]) + np.testing.assert_array_equal(a_get1, primary) + np.testing.assert_array_equal(a_get2, primary) + np.testing.assert_array_equal(a_view1, primary) + np.testing.assert_array_equal(a_view2, primary) + np.testing.assert_array_equal(a_ref1, primary) + np.testing.assert_array_equal(a_ref2, primary) + np.testing.assert_array_equal(a_ref3, primary) + np.testing.assert_array_equal(a_ref4, primary) + np.testing.assert_array_equal(a_block1, primary[3:5, 3:5]) + np.testing.assert_array_equal(a_block2, primary[2:5, 2:4]) + np.testing.assert_array_equal(a_block3, primary[6:10, 7:10]) np.testing.assert_array_equal( - a_corn1, master[0 :: master.shape[0] - 1, 0 :: master.shape[1] - 1] + a_corn1, primary[0 :: primary.shape[0] - 1, 0 :: primary.shape[1] - 1] ) np.testing.assert_array_equal( - a_corn2, master[0 :: master.shape[0] - 1, 0 :: master.shape[1] - 1] + a_corn2, primary[0 :: primary.shape[0] - 1, 0 :: primary.shape[1] - 1] ) np.testing.assert_array_equal(a_copy1, c1want) From fac23b6f65e6d0b9aa8525712936b036e372eae1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 24 Aug 2022 13:08:24 -0700 Subject: [PATCH 02/25] `error_fetch_and_normalize`: PyPy 7.3.10+ does not need the PR #4079 workaround anymore. (#4154) --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 6ba1f5f20..f4ba459f2 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -473,7 +473,7 @@ struct error_fetch_and_normalize { + " failed to obtain the name " "of the normalized active exception type."); } -#if defined(PYPY_VERSION) +#if defined(PYPY_VERSION_NUM) && PYPY_VERSION_NUM < 0x07030a00 // This behavior runs the risk of masking errors in the error handling, but avoids a // conflict with PyPy, which relies on the normalization here to change OSError to // FileNotFoundError (https://github.com/pybind/pybind11/issues/4075). From 0b4c1bc2865cf89abc751809e72a184bde671678 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Mon, 29 Aug 2022 20:25:01 -0700 Subject: [PATCH 03/25] test: ConstructorStats newline (PyPy) (#4167) This looks like it lacks a newline. --- tests/constructor_stats.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/constructor_stats.h b/tests/constructor_stats.h index a3835c21e..937f6c233 100644 --- a/tests/constructor_stats.h +++ b/tests/constructor_stats.h @@ -115,7 +115,7 @@ public: #if defined(PYPY_VERSION) PyObject *globals = PyEval_GetGlobals(); PyObject *result = PyRun_String("import gc\n" - "for i in range(2):" + "for i in range(2):\n" " gc.collect()\n", Py_file_input, globals, From 283f10dc55636e495bc2114c6b5baf7cd7064a0f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 29 Aug 2022 23:26:53 -0400 Subject: [PATCH 04/25] chore(deps): bump ilammy/msvc-dev-cmd from 1.10.0 to 1.11.0 (#4161) Bumps [ilammy/msvc-dev-cmd](https://github.com/ilammy/msvc-dev-cmd) from 1.10.0 to 1.11.0. - [Release notes](https://github.com/ilammy/msvc-dev-cmd/releases) - [Commits](https://github.com/ilammy/msvc-dev-cmd/compare/v1.10.0...v1.11.0) --- updated-dependencies: - dependency-name: ilammy/msvc-dev-cmd dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac6c536ce..213c69005 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -754,7 +754,7 @@ jobs: uses: jwlawson/actions-setup-cmake@v1.12 - name: Prepare MSVC - uses: ilammy/msvc-dev-cmd@v1.10.0 + uses: ilammy/msvc-dev-cmd@v1.11.0 with: arch: x86 @@ -807,7 +807,7 @@ jobs: uses: jwlawson/actions-setup-cmake@v1.12 - name: Prepare MSVC - uses: ilammy/msvc-dev-cmd@v1.10.0 + uses: ilammy/msvc-dev-cmd@v1.11.0 with: arch: x86 From 8756f16ed842e40406018df901f3219b231e2105 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 29 Aug 2022 21:59:48 -0700 Subject: [PATCH 05/25] [pre-commit.ci] pre-commit autoupdate (#4151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [pre-commit.ci] pre-commit autoupdate updates: - [github.com/Lucas-C/pre-commit-hooks: v1.3.0 → v1.3.1](https://github.com/Lucas-C/pre-commit-hooks/compare/v1.3.0...v1.3.1) - [github.com/sirosen/texthooks: 0.3.1 → 0.4.0](https://github.com/sirosen/texthooks/compare/0.3.1...0.4.0) - [github.com/PyCQA/pylint: v2.14.5 → v2.15.0](https://github.com/PyCQA/pylint/compare/v2.14.5...v2.15.0) - [github.com/codespell-project/codespell: v2.1.0 → v2.2.1](https://github.com/codespell-project/codespell/compare/v2.1.0...v2.2.1) * Introduce .codespell-ignore-lines for safer (line-based instead of word-based) suppressions. * Fix two issues: 1. ensure sort order; 2. remove duplicates Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Ralf W. Grosse-Kunstleve --- .codespell-ignore-lines | 24 ++++++++++++++ .pre-commit-config.yaml | 12 ++++--- tools/codespell_ignore_lines_from_errors.py | 35 +++++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 .codespell-ignore-lines create mode 100644 tools/codespell_ignore_lines_from_errors.py diff --git a/.codespell-ignore-lines b/.codespell-ignore-lines new file mode 100644 index 000000000..2a01d63eb --- /dev/null +++ b/.codespell-ignore-lines @@ -0,0 +1,24 @@ +template + template + auto &this_ = static_cast(*this); + if (load_impl(temp, false)) { + ssize_t nd = 0; + auto trivial = broadcast(buffers, nd, shape); + auto ndim = (size_t) nd; + int nd; + ssize_t ndim() const { return detail::array_proxy(m_ptr)->nd; } + using op = op_impl; +template + template + class_ &def(const detail::op_ &op, const Extra &...extra) { + class_ &def_cast(const detail::op_ &op, const Extra &...extra) { +@pytest.mark.parametrize("access", ["ro", "rw", "static_ro", "static_rw"]) +struct IntStruct { + explicit IntStruct(int v) : value(v){}; + ~IntStruct() { value = -value; } + IntStruct(const IntStruct &) = default; + IntStruct &operator=(const IntStruct &) = default; + py::class_(m, "IntStruct").def(py::init([](const int i) { return IntStruct(i); })); + py::implicitly_convertible(); + m.def("test", [](int expected, const IntStruct &in) { + [](int expected, const IntStruct &in) { diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a962f8b79..bced50caf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -62,12 +62,12 @@ repos: # Changes tabs to spaces - repo: https://github.com/Lucas-C/pre-commit-hooks - rev: "v1.3.0" + rev: "v1.3.1" hooks: - id: remove-tabs - repo: https://github.com/sirosen/texthooks - rev: "0.3.1" + rev: "0.4.0" hooks: - id: fix-ligatures - id: fix-smartquotes @@ -110,7 +110,7 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v2.14.5" + rev: "v2.15.0" hooks: - id: pylint files: ^pybind11 @@ -143,12 +143,14 @@ repos: additional_dependencies: [cmake, ninja] # Check for spelling +# Use tools/codespell_ignore_lines_from_errors.py +# to rebuild .codespell-ignore-lines - repo: https://github.com/codespell-project/codespell - rev: "v2.1.0" + rev: "v2.2.1" hooks: - id: codespell exclude: ".supp$" - args: ["-L", "nd,ot,thist"] + args: ["-x", ".codespell-ignore-lines"] # Check for common shell mistakes - repo: https://github.com/shellcheck-py/shellcheck-py diff --git a/tools/codespell_ignore_lines_from_errors.py b/tools/codespell_ignore_lines_from_errors.py new file mode 100644 index 000000000..5403ec3ad --- /dev/null +++ b/tools/codespell_ignore_lines_from_errors.py @@ -0,0 +1,35 @@ +"""Simple script for rebuilding .codespell-ignore-lines + +Usage: + +cat < /dev/null > .codespell-ignore-lines +pre-commit run --all-files codespell >& /tmp/codespell_errors.txt +python3 tools/codespell_ignore_lines_from_errors.py /tmp/codespell_errors.txt > .codespell-ignore-lines + +git diff to review changes, then commit, push. +""" + +import sys +from typing import List + + +def run(args: List[str]) -> None: + assert len(args) == 1, "codespell_errors.txt" + cache = {} + done = set() + for line in sorted(open(args[0]).read().splitlines()): + i = line.find(" ==> ") + if i > 0: + flds = line[:i].split(":") + if len(flds) >= 2: + filename, line_num = flds[:2] + if filename not in cache: + cache[filename] = open(filename).read().splitlines() + supp = cache[filename][int(line_num) - 1] + if supp not in done: + print(supp) + done.add(supp) + + +if __name__ == "__main__": + run(args=sys.argv[1:]) From aa8f8baa4e402885e5c89f5dc42a579932c33790 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 7 Sep 2022 09:19:02 -0400 Subject: [PATCH 06/25] [pre-commit.ci] pre-commit autoupdate (#4171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [pre-commit.ci] pre-commit autoupdate updates: - [github.com/psf/black: 22.6.0 → 22.8.0](https://github.com/psf/black/compare/22.6.0...22.8.0) * Update .pre-commit-config.yaml Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Henry Schreiner --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bced50caf..b23030647 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -48,7 +48,7 @@ repos: # Black, the code formatter, natively supports pre-commit - repo: https://github.com/psf/black - rev: "22.6.0" # Keep in sync with blacken-docs + rev: "22.8.0" # Keep in sync with blacken-docs hooks: - id: black @@ -58,7 +58,7 @@ repos: hooks: - id: blacken-docs additional_dependencies: - - black==22.6.0 # keep in sync with black hook + - black==22.8.0 # keep in sync with black hook # Changes tabs to spaces - repo: https://github.com/Lucas-C/pre-commit-hooks From 64f7281874c3b439dc0102b6ad6cfc7586f17651 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 12 Sep 2022 20:04:44 -0400 Subject: [PATCH 07/25] [pre-commit.ci] pre-commit autoupdate (#4178) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/PyCQA/pylint: v2.15.0 → v2.15.2](https://github.com/PyCQA/pylint/compare/v2.15.0...v2.15.2) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b23030647..5d794a749 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -110,7 +110,7 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v2.15.0" + rev: "v2.15.2" hooks: - id: pylint files: ^pybind11 From 8524b20c3caf8a5e868dbbf76b39927c45b41f72 Mon Sep 17 00:00:00 2001 From: Sergei Izmailov Date: Thu, 15 Sep 2022 05:56:40 +0900 Subject: [PATCH 08/25] fix: Python-3.12 compatibility (#4168) * fix: Python-3.12 compatibility Enable dynamic attributes for `pybind11_static_property` * Add future-notice comment --- include/pybind11/detail/class.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index a98e5e541..528e716f7 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -55,6 +55,9 @@ extern "C" inline int pybind11_static_set(PyObject *self, PyObject *obj, PyObjec return PyProperty_Type.tp_descr_set(self, cls, value); } +// Forward declaration to use in `make_static_property_type()` +inline void enable_dynamic_attributes(PyHeapTypeObject *heap_type); + /** A `static_property` is the same as a `property` but the `__get__()` and `__set__()` methods are modified to always use the object type instead of a concrete instance. Return value: New reference. */ @@ -87,6 +90,13 @@ inline PyTypeObject *make_static_property_type() { pybind11_fail("make_static_property_type(): failure in PyType_Ready()!"); } +# if PY_VERSION_HEX >= 0x030C0000 + // PRE 3.12 FEATURE FREEZE. PLEASE REVIEW AFTER FREEZE. + // Since Python-3.12 property-derived types are required to + // have dynamic attributes (to set `__doc__`) + enable_dynamic_attributes(heap_type); +# endif + setattr((PyObject *) type, "__module__", str("pybind11_builtins")); PYBIND11_SET_OLDPY_QUALNAME(type, name_obj); From 1874f8fa8767179ab8f8c645a6e8c8c4e4a7c486 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Wed, 14 Sep 2022 17:00:27 -0400 Subject: [PATCH 09/25] Clarify GIL documentation (#4057) --- docs/advanced/misc.rst | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index edab15fcb..71960b803 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -39,15 +39,42 @@ The ``PYBIND11_MAKE_OPAQUE`` macro does *not* require the above workarounds. Global Interpreter Lock (GIL) ============================= -When calling a C++ function from Python, the GIL is always held. +The Python C API dictates that the Global Interpreter Lock (GIL) must always +be held by the current thread to safely access Python objects. As a result, +when Python calls into C++ via pybind11 the GIL must be held, and pybind11 +will never implicitly release the GIL. + +.. code-block:: cpp + + void my_function() { + /* GIL is held when this function is called from Python */ + } + + PYBIND11_MODULE(example, m) { + m.def("my_function", &my_function); + } + +pybind11 will ensure that the GIL is held when it knows that it is calling +Python code. For example, if a Python callback is passed to C++ code via +``std::function``, when C++ code calls the function the built-in wrapper +will acquire the GIL before calling the Python callback. Similarly, the +``PYBIND11_OVERRIDE`` family of macros will acquire the GIL before calling +back into Python. + +When writing C++ code that is called from other C++ code, if that code accesses +Python state, it must explicitly acquire and release the GIL. + The classes :class:`gil_scoped_release` and :class:`gil_scoped_acquire` can be used to acquire and release the global interpreter lock in the body of a C++ function call. In this way, long-running C++ code can be parallelized using -multiple Python threads. Taking :ref:`overriding_virtuals` as an example, this +multiple Python threads, **but great care must be taken** when any +:class:`gil_scoped_release` appear: if there is any way that the C++ code +can access Python objects, :class:`gil_scoped_acquire` should be used to +reacquire the GIL. Taking :ref:`overriding_virtuals` as an example, this could be realized as follows (important changes highlighted): .. code-block:: cpp - :emphasize-lines: 8,9,31,32 + :emphasize-lines: 8,30,31 class PyAnimal : public Animal { public: @@ -56,9 +83,7 @@ could be realized as follows (important changes highlighted): /* Trampoline (need one for each virtual function) */ std::string go(int n_times) { - /* Acquire GIL before calling Python code */ - py::gil_scoped_acquire acquire; - + /* PYBIND11_OVERRIDE_PURE will acquire the GIL before accessing Python state */ PYBIND11_OVERRIDE_PURE( std::string, /* Return type */ Animal, /* Parent class */ @@ -78,7 +103,8 @@ could be realized as follows (important changes highlighted): .def(py::init<>()); m.def("call_go", [](Animal *animal) -> std::string { - /* Release GIL before calling into (potentially long-running) C++ code */ + // GIL is held when called from Python code. Release GIL before + // calling into (potentially long-running) C++ code py::gil_scoped_release release; return call_go(animal); }); From 9c04c7b0f1210a5f6ca7cc2e975606a1eaf21316 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 19 Sep 2022 12:56:31 -0400 Subject: [PATCH 10/25] chore: Delete copy ctor/assign for GIL RAIIs (#4183) * chore: Delete copy ctor/assign for GIL RAIIs * Fix typo * Delete copy ops for local gil scoped acquire --- include/pybind11/detail/internals.h | 2 ++ include/pybind11/gil.h | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 6ca5e1482..86991955b 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -412,6 +412,8 @@ PYBIND11_NOINLINE internals &get_internals() { // Cannot use py::gil_scoped_acquire here since that constructor calls get_internals. struct gil_scoped_acquire_local { gil_scoped_acquire_local() : state(PyGILState_Ensure()) {} + gil_scoped_acquire_local(const gil_scoped_acquire_local &) = delete; + gil_scoped_acquire_local &operator=(const gil_scoped_acquire_local &) = delete; ~gil_scoped_acquire_local() { PyGILState_Release(state); } const PyGILState_STATE state; } gil; diff --git a/include/pybind11/gil.h b/include/pybind11/gil.h index a0b5de151..1ef5f0a8c 100644 --- a/include/pybind11/gil.h +++ b/include/pybind11/gil.h @@ -80,6 +80,9 @@ public: inc_ref(); } + gil_scoped_acquire(const gil_scoped_acquire &) = delete; + gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete; + void inc_ref() { ++tstate->gilstate_counter; } PYBIND11_NOINLINE void dec_ref() { @@ -144,6 +147,9 @@ public: } } + gil_scoped_release(const gil_scoped_acquire &) = delete; + gil_scoped_release &operator=(const gil_scoped_acquire &) = delete; + /// This method will disable the PyThreadState_DeleteCurrent call and the /// GIL won't be acquired. This method should be used if the interpreter /// could be shutting down when this is called, as thread deletion is not @@ -178,6 +184,8 @@ class gil_scoped_acquire { public: gil_scoped_acquire() { state = PyGILState_Ensure(); } + gil_scoped_acquire(const gil_scoped_acquire &) = delete; + gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete; ~gil_scoped_acquire() { PyGILState_Release(state); } void disarm() {} }; @@ -187,6 +195,8 @@ class gil_scoped_release { public: gil_scoped_release() { state = PyEval_SaveThread(); } + gil_scoped_release(const gil_scoped_release &) = delete; + gil_scoped_release &operator=(const gil_scoped_acquire &) = delete; ~gil_scoped_release() { PyEval_RestoreThread(state); } void disarm() {} }; From d02f219fb9b9012ce7b27f34c5ee9feb55da08ec Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 19 Sep 2022 21:11:57 -0400 Subject: [PATCH 11/25] [pre-commit.ci] pre-commit autoupdate (#4189) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/asottile/pyupgrade: v2.37.3 → v2.38.0](https://github.com/asottile/pyupgrade/compare/v2.37.3...v2.38.0) - [github.com/PyCQA/pylint: v2.15.2 → v2.15.3](https://github.com/PyCQA/pylint/compare/v2.15.2...v2.15.3) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5d794a749..508b3ea6e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -35,7 +35,7 @@ repos: # Upgrade old Python syntax - repo: https://github.com/asottile/pyupgrade - rev: "v2.37.3" + rev: "v2.38.0" hooks: - id: pyupgrade args: [--py36-plus] @@ -110,7 +110,7 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v2.15.2" + rev: "v2.15.3" hooks: - id: pylint files: ^pybind11 From 424ac4fe1bde46894a56775e78a702199a03137b Mon Sep 17 00:00:00 2001 From: Jan Iwaszkiewicz Date: Tue, 20 Sep 2022 19:03:57 +0200 Subject: [PATCH 12/25] fix: Windows compiler, missing object initializer (#4188) * Fix for windows compiler, missing object initializer * Removal of if-else macro for MSVC --- include/pybind11/detail/common.h | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 9e6947daa..6da7ef859 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -1033,12 +1033,7 @@ PYBIND11_NAMESPACE_END(detail) /// - regular: static_cast(&Class::func) /// - sweet: overload_cast(&Class::func) template -# if (defined(_MSC_VER) && _MSC_VER < 1920) /* MSVC 2017 */ \ - || (defined(__clang__) && __clang_major__ == 5) -static constexpr detail::overload_cast_impl overload_cast = {}; -# else -static constexpr detail::overload_cast_impl overload_cast; -# endif +static constexpr detail::overload_cast_impl overload_cast{}; #endif /// Const member function selector for overload_cast From 95d0e71a652992bcdc0c54844c4c6e5282247c6b Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 21 Sep 2022 11:20:07 -0400 Subject: [PATCH 13/25] test C++14 on MSVC (#4191) --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 213c69005..fe57e7ce9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -737,6 +737,9 @@ jobs: args: -DCMAKE_CXX_STANDARD=20 - python: 3.8 args: -DCMAKE_CXX_STANDARD=17 + - python: 3.7 + args: -DCMAKE_CXX_STANDARD=14 + name: "🐍 ${{ matrix.python }} • MSVC 2019 • x86 ${{ matrix.args }}" runs-on: windows-2019 From f743bdf8e61b2dc4a8995a0485111bedd9c2cbb1 Mon Sep 17 00:00:00 2001 From: bogdan-lab <60753036+bogdan-lab@users.noreply.github.com> Date: Wed, 21 Sep 2022 21:50:31 +0300 Subject: [PATCH 14/25] Avoid local_internals destruction (#4192) * Avoid local_internals destruction It allows to avoid possible static deinitialization fiasco. * Add link to relevant google style guide discussion --- include/pybind11/detail/internals.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 86991955b..d47084e26 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -514,8 +514,13 @@ struct local_internals { /// Works like `get_internals`, but for things which are locally registered. inline local_internals &get_local_internals() { - static local_internals locals; - return locals; + // Current static can be created in the interpreter finalization routine. If the later will be + // destroyed in another static variable destructor, creation of this static there will cause + // static deinitialization fiasco. In order to avoid it we avoid destruction of the + // local_internals static. One can read more about the problem and current solution here: + // https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables + static auto *locals = new local_internals(); + return *locals; } /// Constructs a std::string with the given arguments, stores it in `internals`, and returns its From 5aa0fad5def3fc4d48f29998b6f65e74bacafd19 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 25 Sep 2022 16:10:57 -0400 Subject: [PATCH 15/25] perf: call reserve method in set and map casters (#4194) * Call reserve method in set and map casters too * Refactor template logic into has_reserve_method * Adjust comment for reviews * Rearrange reserve_maybe to not be underneath macro --- include/pybind11/stl.h | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 48031c2a6..8f243502e 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -49,17 +49,31 @@ constexpr forwarded_type forward_like(U &&u) { return std::forward>(std::forward(u)); } +// Checks if a container has a STL style reserve method. +// This will only return true for a `reserve()` with a `void` return. +template +using has_reserve_method = std::is_same().reserve(0)), void>; + template struct set_caster { using type = Type; using key_conv = make_caster; +private: + template ::value, int> = 0> + void reserve_maybe(const anyset &s, Type *) { + value.reserve(s.size()); + } + void reserve_maybe(const anyset &, void *) {} + +public: bool load(handle src, bool convert) { if (!isinstance(src)) { return false; } auto s = reinterpret_borrow(src); value.clear(); + reserve_maybe(s, &value); for (auto entry : s) { key_conv conv; if (!conv.load(entry, convert)) { @@ -94,12 +108,21 @@ struct map_caster { using key_conv = make_caster; using value_conv = make_caster; +private: + template ::value, int> = 0> + void reserve_maybe(const dict &d, Type *) { + value.reserve(d.size()); + } + void reserve_maybe(const dict &, void *) {} + +public: bool load(handle src, bool convert) { if (!isinstance(src)) { return false; } auto d = reinterpret_borrow(src); value.clear(); + reserve_maybe(d, &value); for (auto it : d) { key_conv kconv; value_conv vconv; @@ -160,9 +183,7 @@ struct list_caster { } private: - template < - typename T = Type, - enable_if_t().reserve(0)), void>::value, int> = 0> + template ::value, int> = 0> void reserve_maybe(const sequence &s, Type *) { value.reserve(s.size()); } From da8c730a62a7f9654433f5e2206bd1135a8b57d8 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 26 Sep 2022 20:44:19 -0400 Subject: [PATCH 16/25] [pre-commit.ci] pre-commit autoupdate (#4197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/asottile/pyupgrade: v2.38.0 → v2.38.2](https://github.com/asottile/pyupgrade/compare/v2.38.0...v2.38.2) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 508b3ea6e..2c317ea7a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -35,7 +35,7 @@ repos: # Upgrade old Python syntax - repo: https://github.com/asottile/pyupgrade - rev: "v2.38.0" + rev: "v2.38.2" hooks: - id: pyupgrade args: [--py36-plus] From c78dfe6964579c69692eb8b5e57cbed26e409f6e Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 3 Oct 2022 13:44:09 -0400 Subject: [PATCH 17/25] bugfix: Add error checking to list append and insert (#4208) --- include/pybind11/pytypes.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index f4ba459f2..565df4375 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -2022,14 +2022,20 @@ public: detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } template void append(T &&val) /* py-non-const */ { - PyList_Append(m_ptr, detail::object_or_cast(std::forward(val)).ptr()); + if (PyList_Append(m_ptr, detail::object_or_cast(std::forward(val)).ptr()) != 0) { + throw error_already_set(); + } } template ::value, int> = 0> void insert(const IdxType &index, ValType &&val) /* py-non-const */ { - PyList_Insert( - m_ptr, ssize_t_cast(index), detail::object_or_cast(std::forward(val)).ptr()); + if (PyList_Insert(m_ptr, + ssize_t_cast(index), + detail::object_or_cast(std::forward(val)).ptr()) + != 0) { + throw error_already_set(); + } } }; From 600d697648f4eda8c88a6884472bd1d2ec0a68ad Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 3 Oct 2022 21:25:46 -0400 Subject: [PATCH 18/25] [pre-commit.ci] pre-commit autoupdate (#4210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-mypy: v0.971 → v0.981](https://github.com/pre-commit/mirrors-mypy/compare/v0.971...v0.981) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2c317ea7a..ad07577eb 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -126,7 +126,7 @@ repos: # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v0.971" + rev: "v0.981" hooks: - id: mypy args: [] From 8275b769129816f97c7665f64c88a0e05d8987df Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 4 Oct 2022 14:01:26 -0400 Subject: [PATCH 19/25] ci: update pre-commit schedule (#4212) --- .pre-commit-config.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ad07577eb..742e7a30a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,6 +12,12 @@ # # See https://github.com/pre-commit/pre-commit + +ci: + autoupdate_commit_msg: "chore(deps): update pre-commit hooks" + autofix_commit_msg: "style: pre-commit fixes" + autoupdate_schedule: monthly + # third-party content exclude: ^tools/JoinPaths.cmake$ From 864ed1120c704bf27d6ad5ab40645262d0d1e955 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 6 Oct 2022 16:11:34 -0400 Subject: [PATCH 20/25] chore: steal arg_v.value from copied arg in unpacking_collector (#4219) --- include/pybind11/cast.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index a0e32281b..b89e4946a 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1545,7 +1545,7 @@ private: throw cast_error_unable_to_convert_call_arg(a.name, a.type); #endif } - m_kwargs[a.name] = a.value; + m_kwargs[a.name] = std::move(a.value); } void process(list & /*args_list*/, detail::kwargs_proxy kp) { From 6cb214748d6ec3a61fdc35ee27f9a90d170dd242 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Thu, 6 Oct 2022 21:02:57 -0700 Subject: [PATCH 21/25] fix: NVCC 11.4.0 - 11.8.0 host bug workaround (#4220) * Work-Around: NVCC 11.4.0 - 11.8.0 Adds a targeted NVCC work around for limited number of CUDA releases. Fixed in NVCC development. * style: pre-commit fixes * CI: Bump CTK Version 11.2 -> 11.7 Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 4 ++-- include/pybind11/pybind11.h | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe57e7ce9..99caabf09 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -324,8 +324,8 @@ jobs: # Testing NVCC; forces sources to behave like .cu files cuda: runs-on: ubuntu-latest - name: "🐍 3.8 • CUDA 11.2 • Ubuntu 20.04" - container: nvidia/cuda:11.2.2-devel-ubuntu20.04 + name: "🐍 3.10 • CUDA 11.7 • Ubuntu 22.04" + container: nvidia/cuda:11.7.0-devel-ubuntu22.04 steps: - uses: actions/checkout@v3 diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index c889dc416..3f6e27b75 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1578,6 +1578,22 @@ public: return *this; } +// Nvidia's NVCC is broken between 11.4.0 and 11.8.0 +// https://github.com/pybind/pybind11/issues/4193 +#if defined(__CUDACC__) && (__CUDACC_VER_MAJOR__ == 11) && (__CUDACC_VER_MINOR__ >= 4) \ + && (__CUDACC_VER_MINOR__ <= 8) + template + class_ &def(const T &op, const Extra &...extra) { + op.execute(*this, extra...); + return *this; + } + + template + class_ &def_cast(const T &op, const Extra &...extra) { + op.execute_cast(*this, extra...); + return *this; + } +#else template class_ &def(const detail::op_ &op, const Extra &...extra) { op.execute(*this, extra...); @@ -1589,6 +1605,7 @@ public: op.execute_cast(*this, extra...); return *this; } +#endif template class_ &def(const detail::initimpl::constructor &init, const Extra &...extra) { From 4a4215620920ee659145cd34597e14f0553a73f1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 7 Oct 2022 09:20:38 -0700 Subject: [PATCH 22/25] test_eigen.py test_nonunit_stride_to_python bug fix (ASAN failure) (#4217) * Disable test triggering ASAN failure (to pin-point where the problem is). * Fix unsafe "block" implementation in test_eigen.cpp * Undo changes (i.e. revert back to master). * Detect "type_caster for Eigen::Ref made a copy." This is achieved without * reaching into internals, * making test_eigen.cpp depend on pybind11/numpy.h. * Add comment pointing to PR, for easy reference. --- tests/test_eigen.cpp | 39 ++++++++++++++++++++++++++++++++++----- tests/test_eigen.py | 15 ++++++++++++--- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index 591dacc62..b0c7bdb48 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -197,11 +197,40 @@ TEST_SUBMODULE(eigen, m) { // Return a block of a matrix (gives non-standard strides) m.def("block", - [](const Eigen::Ref &x, - int start_row, - int start_col, - int block_rows, - int block_cols) { return x.block(start_row, start_col, block_rows, block_cols); }); + [m](const py::object &x_obj, + int start_row, + int start_col, + int block_rows, + int block_cols) { + return m.attr("_block")(x_obj, x_obj, start_row, start_col, block_rows, block_cols); + }); + + m.def( + "_block", + [](const py::object &x_obj, + const Eigen::Ref &x, + int start_row, + int start_col, + int block_rows, + int block_cols) { + // See PR #4217 for background. This test is a bit over the top, but might be useful + // as a concrete example to point to when explaining the dangling reference trap. + auto i0 = py::make_tuple(0, 0); + auto x0_orig = x_obj[*i0].cast(); + if (x(0, 0) != x0_orig) { + throw std::runtime_error( + "Something in the type_caster for Eigen::Ref is terribly wrong."); + } + double x0_mod = x0_orig + 1; + x_obj[*i0] = x0_mod; + auto copy_detected = (x(0, 0) != x0_mod); + x_obj[*i0] = x0_orig; + if (copy_detected) { + throw std::runtime_error("type_caster for Eigen::Ref made a copy."); + } + return x.block(start_row, start_col, block_rows, block_cols); + }, + py::keep_alive<0, 1>()); // test_eigen_return_references, test_eigen_keepalive // return value referencing/copying tests: diff --git a/tests/test_eigen.py b/tests/test_eigen.py index 9c6485de3..713432a81 100644 --- a/tests/test_eigen.py +++ b/tests/test_eigen.py @@ -217,15 +217,24 @@ def test_negative_stride_from_python(msg): ) +def test_block_runtime_error_type_caster_eigen_ref_made_a_copy(): + with pytest.raises(RuntimeError) as excinfo: + m.block(ref, 0, 0, 0, 0) + assert str(excinfo.value) == "type_caster for Eigen::Ref made a copy." + + def test_nonunit_stride_to_python(): assert np.all(m.diagonal(ref) == ref.diagonal()) assert np.all(m.diagonal_1(ref) == ref.diagonal(1)) for i in range(-5, 7): assert np.all(m.diagonal_n(ref, i) == ref.diagonal(i)), f"m.diagonal_n({i})" - assert np.all(m.block(ref, 2, 1, 3, 3) == ref[2:5, 1:4]) - assert np.all(m.block(ref, 1, 4, 4, 2) == ref[1:, 4:]) - assert np.all(m.block(ref, 1, 4, 3, 2) == ref[1:4, 4:]) + # Must be order="F", otherwise the type_caster will make a copy and + # m.block() will return a dangling reference (heap-use-after-free). + rof = np.asarray(ref, order="F") + assert np.all(m.block(rof, 2, 1, 3, 3) == rof[2:5, 1:4]) + assert np.all(m.block(rof, 1, 4, 4, 2) == rof[1:, 4:]) + assert np.all(m.block(rof, 1, 4, 3, 2) == rof[1:4, 4:]) def test_eigen_ref_to_python(): From 7c6f2f80a73e330642226138ae3ddaf448a4f672 Mon Sep 17 00:00:00 2001 From: Daniel Galvez Date: Fri, 7 Oct 2022 12:27:54 -0700 Subject: [PATCH 23/25] fix: PyCapsule_GetDestructor is allowed to return a nullptr destructor (#4221) * fix: PyCapsule_GetDestructor is allowed to return a nullptr destructor Previously, this code would error out if the destructor happened to be a nullptr. This is incorrect. nullptrs are allowed for capsule destructors. "It is legal for a capsule to have a NULL destructor. This makes a NULL return code somewhat ambiguous; use PyCapsule_IsValid() or PyErr_Occurred() to disambiguate." See: https://docs.python.org/3/c-api/capsule.html#c.PyCapsule_GetDestructor I noticed this while working on a type caster related to #3858 DLPack happens to allow the destructor not to be defined on a capsule, and I encountered such a case. See: https://github.com/dmlc/dlpack/blob/e2bdd3bee8cb6501558042633fa59144cc8b7f5f/include/dlpack/dlpack.h#L219 * Add test for the fix. * Update tests/test_pytypes.cpp I tried this locally and it works! I never knew that there are cases where `reinterpret_cast` does not work but `static_cast` does. Let's see if all compilers are happy with this. Co-authored-by: Aaron Gokaslan * style: pre-commit fixes Co-authored-by: Ralf W. Grosse-Kunstleve Co-authored-by: Ralf W. Grosse-Kunstleve Co-authored-by: Aaron Gokaslan Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/pybind11/pytypes.h | 12 ++++++------ tests/test_pytypes.cpp | 6 ++++++ tests/test_pytypes.py | 11 +++++++++++ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 565df4375..29506b7ea 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1829,18 +1829,18 @@ public: // guard if destructor called while err indicator is set error_scope error_guard; auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); - if (destructor == nullptr) { - if (PyErr_Occurred()) { - throw error_already_set(); - } - pybind11_fail("Unable to get capsule context"); + if (PyErr_Occurred()) { + throw error_already_set(); } const char *name = get_name_in_error_scope(o); void *ptr = PyCapsule_GetPointer(o, name); if (ptr == nullptr) { throw error_already_set(); } - destructor(ptr); + + if (destructor != nullptr) { + destructor(ptr); + } }); if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index da0dc8f6b..c95ff8230 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -289,6 +289,12 @@ TEST_SUBMODULE(pytypes, m) { return capsule; }); + m.def("return_capsule_with_explicit_nullptr_dtor", []() { + py::print("creating capsule with explicit nullptr dtor"); + return py::capsule(reinterpret_cast(1234), + static_cast(nullptr)); // PR #4221 + }); + // test_accessors m.def("accessor_api", [](const py::object &o) { auto d = py::dict(); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index a34eaa59e..070849fc5 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -299,6 +299,17 @@ def test_capsule(capture): """ ) + with capture: + a = m.return_capsule_with_explicit_nullptr_dtor() + del a + pytest.gc_collect() + assert ( + capture.unordered + == """ + creating capsule with explicit nullptr dtor + """ + ) + def test_accessors(): class SubTestObject: From da104a9efd0357b5c144c67eb641ae0b9e23012d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 9 Oct 2022 21:50:35 -0700 Subject: [PATCH 24/25] Reproducer and fix for issue encountered in smart_holder update. (#4228) * Reproducer for issue encountered in smart_holder update. * clang-tidy compatibility (untested). * Add `enable_if_t` to workaround. * Bug fix: Move `PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8` determination to detail/common.h So that it actually is defined in pybind11.h * Try using the workaround (which is nicer than the original code) universally. * Reduce reproducer for CUDA 11.7 issue encountered in smart_holder update. This commit tested in isolation on top of current master + first version of reproducer (62311eb431849d135a5db84f6c75ec390f2ede7c). Succeeds with Debian Clang 14.0.6 C++17 (and probably all other compilers). Fails for CUDA 11.7: ``` cd /build/tests && /usr/local/cuda/bin/nvcc -forward-unknown-to-host-compiler -Dpybind11_tests_EXPORTS -I/mounted_pybind11/include -isystem=/usr/include/python3.10 -g --generate-code=arch=compute_52,code=[compute_52,sm_52] -Xcompiler=-fPIC -Xcompiler=-fvisibility=hidden -Werror all-warnings -std=c++17 -MD -MT tests/CMakeFiles/pybind11_tests.dir/test_class.cpp.o -MF CMakeFiles/pybind11_tests.dir/test_class.cpp.o.d -x cu -c /mounted_pybind11/tests/test_class.cpp -o CMakeFiles/pybind11_tests.dir/test_class.cpp.o /mounted_pybind11/tests/test_class.cpp(53): error: more than one instance of overloaded function "pybind11::class_::def [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]" matches the argument list: function template "pybind11::class_ &pybind11::class_::def(const char *, Func &&, const Extra &...) [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]" /mounted_pybind11/include/pybind11/pybind11.h(1557): here function template "pybind11::class_ &pybind11::class_::def(const T &, const Extra &...) [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]" /mounted_pybind11/include/pybind11/pybind11.h(1586): here argument types are: (const char [8], ) object type is: pybind11::class_ 1 error detected in the compilation of "/mounted_pybind11/tests/test_class.cpp". ``` --- include/pybind11/operators.h | 1 + include/pybind11/pybind11.h | 21 ++------------------- tests/test_class.cpp | 22 ++++++++++++++++++++++ tests/test_class.py | 7 +++++++ 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/include/pybind11/operators.h b/include/pybind11/operators.h index a0c3b78d6..16a88ae17 100644 --- a/include/pybind11/operators.h +++ b/include/pybind11/operators.h @@ -84,6 +84,7 @@ struct op_impl {}; /// Operator implementation generator template struct op_ { + static constexpr bool op_enable_if_hook = true; template void execute(Class &cl, const Extra &...extra) const { using Base = typename Class::type; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 3f6e27b75..fb4b7578d 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1578,34 +1578,17 @@ public: return *this; } -// Nvidia's NVCC is broken between 11.4.0 and 11.8.0 -// https://github.com/pybind/pybind11/issues/4193 -#if defined(__CUDACC__) && (__CUDACC_VER_MAJOR__ == 11) && (__CUDACC_VER_MINOR__ >= 4) \ - && (__CUDACC_VER_MINOR__ <= 8) - template + template = 0> class_ &def(const T &op, const Extra &...extra) { op.execute(*this, extra...); return *this; } - template + template = 0> class_ &def_cast(const T &op, const Extra &...extra) { op.execute_cast(*this, extra...); return *this; } -#else - template - class_ &def(const detail::op_ &op, const Extra &...extra) { - op.execute(*this, extra...); - return *this; - } - - template - class_ &def_cast(const detail::op_ &op, const Extra &...extra) { - op.execute_cast(*this, extra...); - return *this; - } -#endif template class_ &def(const detail::initimpl::constructor &init, const Extra &...extra) { diff --git a/tests/test_class.cpp b/tests/test_class.cpp index c8b8071dc..18c8d358b 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -36,6 +36,26 @@ struct NoBraceInitialization { std::vector vec; }; +namespace test_class { +namespace pr4220_tripped_over_this { // PR #4227 + +template +struct SoEmpty {}; + +template +std::string get_msg(const T &) { + return "This is really only meant to exercise successful compilation."; +} + +using Empty0 = SoEmpty<0x0>; + +void bind_empty0(py::module_ &m) { + py::class_(m, "Empty0").def(py::init<>()).def("get_msg", get_msg); +} + +} // namespace pr4220_tripped_over_this +} // namespace test_class + TEST_SUBMODULE(class_, m) { // test_instance struct NoConstructor { @@ -517,6 +537,8 @@ TEST_SUBMODULE(class_, m) { py::class_(gt, "OtherDuplicateNested"); py::class_(gt, "YetAnotherDuplicateNested"); }); + + test_class::pr4220_tripped_over_this::bind_empty0(m); } template diff --git a/tests/test_class.py b/tests/test_class.py index ff9196f0f..47ba450fc 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -469,3 +469,10 @@ def test_register_duplicate_class(): m.register_duplicate_nested_class_type(ClassScope) expected = 'generic_type: type "YetAnotherDuplicateNested" is already registered!' assert str(exc_info.value) == expected + + +def test_pr4220_tripped_over_this(): + assert ( + m.Empty0().get_msg() + == "This is really only meant to exercise successful compilation." + ) From ff7b69714d76c312b0bcc45e4a8b854783f6dbc5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 11 Oct 2022 00:50:40 -0400 Subject: [PATCH 25/25] chore(deps): bump jwlawson/actions-setup-cmake from 1.12 to 1.13 (#4233) Bumps [jwlawson/actions-setup-cmake](https://github.com/jwlawson/actions-setup-cmake) from 1.12 to 1.13. - [Release notes](https://github.com/jwlawson/actions-setup-cmake/releases) - [Commits](https://github.com/jwlawson/actions-setup-cmake/compare/v1.12...v1.13) --- updated-dependencies: - dependency-name: jwlawson/actions-setup-cmake dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 12 ++++++------ .github/workflows/configure.yml | 2 +- .github/workflows/upstream.yml | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99caabf09..38694bcfa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,7 +80,7 @@ jobs: run: brew install boost - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.12 + uses: jwlawson/actions-setup-cmake@v1.13 - name: Cache wheels if: runner.os == 'macOS' @@ -202,7 +202,7 @@ jobs: debug: ${{ matrix.python-debug }} - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.12 + uses: jwlawson/actions-setup-cmake@v1.13 - name: Valgrind cache if: matrix.valgrind @@ -465,7 +465,7 @@ jobs: run: python3 -m pip install --upgrade pip - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.12 + uses: jwlawson/actions-setup-cmake@v1.13 - name: Configure shell: bash @@ -754,7 +754,7 @@ jobs: architecture: x86 - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.12 + uses: jwlawson/actions-setup-cmake@v1.13 - name: Prepare MSVC uses: ilammy/msvc-dev-cmd@v1.11.0 @@ -807,7 +807,7 @@ jobs: architecture: x86 - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.12 + uses: jwlawson/actions-setup-cmake@v1.13 - name: Prepare MSVC uses: ilammy/msvc-dev-cmd@v1.11.0 @@ -858,7 +858,7 @@ jobs: python3 -m pip install -r tests/requirements.txt - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.12 + uses: jwlawson/actions-setup-cmake@v1.13 - name: Configure C++20 run: > diff --git a/.github/workflows/configure.yml b/.github/workflows/configure.yml index edcad4198..5ec0dd462 100644 --- a/.github/workflows/configure.yml +++ b/.github/workflows/configure.yml @@ -51,7 +51,7 @@ jobs: # An action for adding a specific version of CMake: # https://github.com/jwlawson/actions-setup-cmake - name: Setup CMake ${{ matrix.cmake }} - uses: jwlawson/actions-setup-cmake@v1.12 + uses: jwlawson/actions-setup-cmake@v1.13 with: cmake-version: ${{ matrix.cmake }} diff --git a/.github/workflows/upstream.yml b/.github/workflows/upstream.yml index 95ff4cb83..366284acf 100644 --- a/.github/workflows/upstream.yml +++ b/.github/workflows/upstream.yml @@ -31,7 +31,7 @@ jobs: run: sudo apt-get install libboost-dev - name: Update CMake - uses: jwlawson/actions-setup-cmake@v1.12 + uses: jwlawson/actions-setup-cmake@v1.13 - name: Prepare env run: |