From fa27d2fd439cd84a749e2759c0dd21529fac50a4 Mon Sep 17 00:00:00 2001 From: Mattias Ellert Date: Tue, 24 Oct 2023 18:46:26 +0200 Subject: [PATCH 01/11] Adapt to changed function name in Python 3.13 (#4902) According to https://docs.python.org/3.13/whatsnew/3.13.html: Add PyThreadState_GetUnchecked() function: similar to PyThreadState_Get(), but don't kill the process with a fatal error if it is NULL. The caller is responsible to check if the result is NULL. Previously, the function was private and known as _PyThreadState_UncheckedGet(). --- include/pybind11/detail/type_caster_base.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index fc5f1c88b..68512b5bd 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -486,8 +486,10 @@ PYBIND11_NOINLINE handle get_object_handle(const void *ptr, const detail::type_i inline PyThreadState *get_thread_state_unchecked() { #if defined(PYPY_VERSION) return PyThreadState_GET(); -#else +#elif PY_VERSION_HEX < 0x030D0000 return _PyThreadState_UncheckedGet(); +#else + return PyThreadState_GetUnchecked(); #endif } From 1e28599e41ef5bcf436eb0ce61f7c1e8ebade054 Mon Sep 17 00:00:00 2001 From: Chun Yang Date: Thu, 26 Oct 2023 20:40:19 -0700 Subject: [PATCH 02/11] fix: Add missing spaces to error string (#4906) * [minor] Add a missing space Add a missing space to error message * Add space after period, always add newline. --- include/pybind11/pytypes.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index fa2d24984..e7c7c8124 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -305,19 +305,19 @@ private: "https://pybind11.readthedocs.io/en/stable/advanced/" "misc.html#common-sources-of-global-interpreter-lock-errors for debugging advice.\n" "If you are convinced there is no bug in your code, you can #define " - "PYBIND11_NO_ASSERT_GIL_HELD_INCREF_DECREF" + "PYBIND11_NO_ASSERT_GIL_HELD_INCREF_DECREF " "to disable this check. In that case you have to ensure this #define is consistently " "used for all translation units linked into a given pybind11 extension, otherwise " "there will be ODR violations.", function_name.c_str()); - fflush(stderr); if (Py_TYPE(m_ptr)->tp_name != nullptr) { fprintf(stderr, - "The failing %s call was triggered on a %s object.\n", + " The failing %s call was triggered on a %s object.", function_name.c_str(), Py_TYPE(m_ptr)->tp_name); - fflush(stderr); } + fprintf(stderr, "\n"); + fflush(stderr); throw std::runtime_error(function_name + " PyGILState_Check() failure."); } #endif From 3aece819fd0d0a9f40eed659a088eab60a555202 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 27 Oct 2023 01:26:28 -0400 Subject: [PATCH 03/11] chore: update hooks and Ruff config (#4904) Signed-off-by: Henry Schreiner --- .pre-commit-config.yaml | 14 +++++++------- pyproject.toml | 16 ++++++++-------- tests/test_numpy_dtypes.cpp | 2 +- tests/test_pytypes.cpp | 4 ++-- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e15902045..e40e4fe1c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,27 +25,27 @@ repos: # Clang format the codebase automatically - repo: https://github.com/pre-commit/mirrors-clang-format - rev: "v16.0.6" + rev: "v17.0.3" hooks: - id: clang-format types_or: [c++, c, cuda] # Black, the code formatter, natively supports pre-commit - repo: https://github.com/psf/black-pre-commit-mirror - rev: "23.9.1" # Keep in sync with blacken-docs + rev: "23.10.1" # Keep in sync with blacken-docs hooks: - id: black # Ruff, the Python auto-correcting linter written in Rust - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.0.292 + rev: v0.1.2 hooks: - id: ruff args: ["--fix", "--show-fixes"] # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v1.5.1" + rev: "v1.6.1" hooks: - id: mypy args: [] @@ -67,7 +67,7 @@ repos: # Standard hooks - repo: https://github.com/pre-commit/pre-commit-hooks - rev: "v4.4.0" + rev: "v4.5.0" hooks: - id: check-added-large-files - id: check-case-conflict @@ -98,7 +98,7 @@ repos: # Avoid directional quotes - repo: https://github.com/sirosen/texthooks - rev: "0.5.0" + rev: "0.6.2" hooks: - id: fix-ligatures - id: fix-smartquotes @@ -147,7 +147,7 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v3.0.0" + rev: "v3.0.1" hooks: - id: pylint files: ^pybind11 diff --git a/pyproject.toml b/pyproject.toml index ef2a8736b..7769860c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ ignore = [ [tool.mypy] files = ["pybind11"] -python_version = "3.6" +python_version = "3.7" strict = true show_error_codes = true enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"] @@ -57,10 +57,13 @@ messages_control.disable = [ "unused-argument", # covered by Ruff ARG ] - [tool.ruff] -select = [ - "E", "F", "W", # flake8 +target-version = "py37" +src = ["src"] +line-length = 120 + +[tool.ruff.lint] +extend-select = [ "B", # flake8-bugbear "I", # isort "N", # pep8-naming @@ -86,13 +89,10 @@ ignore = [ "PT004", # Fixture that doesn't return needs underscore (no, it is fine) "SIM118", # iter(x) is not always the same as iter(x.keys()) ] -target-version = "py37" -src = ["src"] unfixable = ["T20"] exclude = [] -line-length = 120 isort.known-first-party = ["env", "pybind11_cross_module_tests", "pybind11_tests"] -[tool.ruff.per-file-ignores] +[tool.ruff.lint.per-file-ignores] "tests/**" = ["EM", "N", "E721"] "tests/test_call_policies.py" = ["PLC1901"] diff --git a/tests/test_numpy_dtypes.cpp b/tests/test_numpy_dtypes.cpp index 6654f9ed8..053004a22 100644 --- a/tests/test_numpy_dtypes.cpp +++ b/tests/test_numpy_dtypes.cpp @@ -157,7 +157,7 @@ py::array mkarray_via_buffer(size_t n) { do { \ (s).bool_ = (i) % 2 != 0; \ (s).uint_ = (uint32_t) (i); \ - (s).float_ = (float) (i) *1.5f; \ + (s).float_ = (float) (i) * 1.5f; \ (s).ldbl_ = (long double) (i) * -2.5L; \ } while (0) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 0a587680f..0cd480ebb 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -662,8 +662,8 @@ TEST_SUBMODULE(pytypes, m) { // This is "most correct" and enforced on these platforms. # define PYBIND11_AUTO_IT auto it #else -// This works on many platforms and is (unfortunately) reflective of existing user code. -// NOLINTNEXTLINE(bugprone-macro-parentheses) + // This works on many platforms and is (unfortunately) reflective of existing user code. + // NOLINTNEXTLINE(bugprone-macro-parentheses) # define PYBIND11_AUTO_IT auto &it #endif From a18c10f6909dfb2c44be04e2a252c3388ddd6416 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 27 Oct 2023 11:02:05 -0400 Subject: [PATCH 04/11] fix(cmake): make library component optional (#4805) Signed-off-by: Henry Schreiner --- tools/pybind11NewTools.cmake | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/pybind11NewTools.cmake b/tools/pybind11NewTools.cmake index 8cee2d460..9106762fc 100644 --- a/tools/pybind11NewTools.cmake +++ b/tools/pybind11NewTools.cmake @@ -32,12 +32,22 @@ if(NOT Python_FOUND AND NOT Python3_FOUND) set(Python_ROOT_DIR "$ENV{pythonLocation}") endif() - find_package(Python 3.6 REQUIRED COMPONENTS Interpreter Development ${_pybind11_quiet}) + # Development.Module support (required for manylinux) started in 3.18 + if(CMAKE_VERSION VERSION_LESS 3.18) + set(_pybind11_dev_component Development) + else() + set(_pybind11_dev_component Development.Module OPTIONAL_COMPONENTS Development.Embed) + endif() + + find_package(Python 3.6 REQUIRED COMPONENTS Interpreter ${_pybind11_dev_component} + ${_pybind11_quiet}) # If we are in submodule mode, export the Python targets to global targets. # If this behavior is not desired, FindPython _before_ pybind11. if(NOT is_config) - set_property(TARGET Python::Python PROPERTY IMPORTED_GLOBAL TRUE) + if(TARGET Python::Python) + set_property(TARGET Python::Python PROPERTY IMPORTED_GLOBAL TRUE) + endif() set_property(TARGET Python::Interpreter PROPERTY IMPORTED_GLOBAL TRUE) if(TARGET Python::Module) set_property(TARGET Python::Module PROPERTY IMPORTED_GLOBAL TRUE) From 76b7f53649580b66b559752de6a39d98477ff1da Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Tue, 31 Oct 2023 19:56:16 -0700 Subject: [PATCH 05/11] Python_ADDITIONAL_VERSIONS: 3.12 (#4909) Add 3.12 to the default `Python_ADDITIONAL_VERSIONS`. --- tools/pybind11Tools.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pybind11Tools.cmake b/tools/pybind11Tools.cmake index 1b1774d09..045e5f1e7 100644 --- a/tools/pybind11Tools.cmake +++ b/tools/pybind11Tools.cmake @@ -43,7 +43,7 @@ endif() # A user can set versions manually too set(Python_ADDITIONAL_VERSIONS - "3.11;3.10;3.9;3.8;3.7;3.6" + "3.12;3.11;3.10;3.9;3.8;3.7;3.6" CACHE INTERNAL "") list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}") From 31b0a5d94f6006ede66b937d0a75d90a582c510f Mon Sep 17 00:00:00 2001 From: Social_Mean Date: Sat, 4 Nov 2023 10:53:15 +0800 Subject: [PATCH 06/11] fix doc typo --- tools/pybind11Config.cmake.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pybind11Config.cmake.in b/tools/pybind11Config.cmake.in index 5734f437b..304f1d907 100644 --- a/tools/pybind11Config.cmake.in +++ b/tools/pybind11Config.cmake.in @@ -149,7 +149,7 @@ default is ``MODULE``. There are several options: ``OPT_SIZE`` Optimize for size, even if the ``CMAKE_BUILD_TYPE`` is not ``MinSizeRel``. ``THIN_LTO`` - Use thin TLO instead of regular if there's a choice (pybind11's selection + Use thin LTO instead of regular if there's a choice (pybind11's selection is disabled if ``CMAKE_INTERPROCEDURAL_OPTIMIZATIONS`` is set). ``WITHOUT_SOABI`` Disable the SOABI component (``PYBIND11_NEWPYTHON`` mode only). From f2606930bf5d1140daecc6bc2aea2baf4b58f7ff Mon Sep 17 00:00:00 2001 From: cyyever Date: Mon, 6 Nov 2023 06:08:32 +0800 Subject: [PATCH 07/11] Use newer PyCode API and other fixes (#4916) * Use PyCode API * style: pre-commit fixes * Free locals * Fix PY_VERSION_HEX check --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/pybind11/pybind11.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 95f7fa2eb..3e1a057db 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2751,10 +2751,15 @@ get_type_override(const void *this_ptr, const type_info *this_type, const char * if ((std::string) str(f_code->co_name) == name && f_code->co_argcount > 0) { PyObject *locals = PyEval_GetLocals(); if (locals != nullptr) { +# if PY_VERSION_HEX >= 0x030b0000 + PyObject *co_varnames = PyCode_GetVarnames(f_code); +# else PyObject *co_varnames = PyObject_GetAttrString((PyObject *) f_code, "co_varnames"); +# endif PyObject *self_arg = PyTuple_GET_ITEM(co_varnames, 0); Py_DECREF(co_varnames); PyObject *self_caller = dict_getitem(locals, self_arg); + Py_DECREF(locals); if (self_caller == self.ptr()) { Py_DECREF(f_code); Py_DECREF(frame); From 0a974fed54b36b4d3df3651758a58edd04d6414f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 7 Nov 2023 22:59:52 -0800 Subject: [PATCH 08/11] chore(deps): update pre-commit hooks (#4923) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-clang-format: v17.0.3 → v17.0.4](https://github.com/pre-commit/mirrors-clang-format/compare/v17.0.3...v17.0.4) - [github.com/astral-sh/ruff-pre-commit: v0.1.2 → v0.1.4](https://github.com/astral-sh/ruff-pre-commit/compare/v0.1.2...v0.1.4) 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 e40e4fe1c..2c731f72d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,7 +25,7 @@ repos: # Clang format the codebase automatically - repo: https://github.com/pre-commit/mirrors-clang-format - rev: "v17.0.3" + rev: "v17.0.4" hooks: - id: clang-format types_or: [c++, c, cuda] @@ -38,7 +38,7 @@ repos: # Ruff, the Python auto-correcting linter written in Rust - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.1.2 + rev: v0.1.4 hooks: - id: ruff args: ["--fix", "--show-fixes"] From c758b81f3b67e64ffe6aa6fe467e02f6b7dde3e8 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 8 Nov 2023 02:42:54 -0500 Subject: [PATCH 09/11] chore: move to ruff-format (#4912) Signed-off-by: Henry Schreiner Co-authored-by: Ralf W. Grosse-Kunstleve --- .pre-commit-config.yaml | 11 +++------ pybind11/setup_helpers.py | 4 +++- pyproject.toml | 3 --- tests/test_enum.py | 4 +--- tests/test_methods_and_attributes.py | 34 ++++++++++++++++------------ 5 files changed, 26 insertions(+), 30 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2c731f72d..1c1531bb5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,18 +30,13 @@ repos: - id: clang-format types_or: [c++, c, cuda] -# Black, the code formatter, natively supports pre-commit -- repo: https://github.com/psf/black-pre-commit-mirror - rev: "23.10.1" # Keep in sync with blacken-docs - hooks: - - id: black - -# Ruff, the Python auto-correcting linter written in Rust +# Ruff, the Python auto-correcting linter/formatter written in Rust - repo: https://github.com/astral-sh/ruff-pre-commit rev: v0.1.4 hooks: - id: ruff args: ["--fix", "--show-fixes"] + - id: ruff-format # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy @@ -88,7 +83,7 @@ repos: hooks: - id: blacken-docs additional_dependencies: - - black==23.3.0 # keep in sync with black hook + - black==23.* # Changes tabs to spaces - repo: https://github.com/Lucas-C/pre-commit-hooks diff --git a/pybind11/setup_helpers.py b/pybind11/setup_helpers.py index aeeee9dcf..3b16dca88 100644 --- a/pybind11/setup_helpers.py +++ b/pybind11/setup_helpers.py @@ -66,7 +66,9 @@ try: from setuptools import Extension as _Extension from setuptools.command.build_ext import build_ext as _build_ext except ImportError: - from distutils.command.build_ext import build_ext as _build_ext # type: ignore[assignment] + from distutils.command.build_ext import ( # type: ignore[assignment] + build_ext as _build_ext, + ) from distutils.extension import Extension as _Extension # type: ignore[assignment] import distutils.ccompiler diff --git a/pyproject.toml b/pyproject.toml index 7769860c4..5af6015a6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,7 +60,6 @@ messages_control.disable = [ [tool.ruff] target-version = "py37" src = ["src"] -line-length = 120 [tool.ruff.lint] extend-select = [ @@ -71,7 +70,6 @@ extend-select = [ "C4", # flake8-comprehensions "EM", # flake8-errmsg "ICN", # flake8-import-conventions - "ISC", # flake8-implicit-str-concat "PGH", # pygrep-hooks "PIE", # flake8-pie "PL", # pylint @@ -90,7 +88,6 @@ ignore = [ "SIM118", # iter(x) is not always the same as iter(x.keys()) ] unfixable = ["T20"] -exclude = [] isort.known-first-party = ["env", "pybind11_cross_module_tests", "pybind11_tests"] [tool.ruff.lint.per-file-ignores] diff --git a/tests/test_enum.py b/tests/test_enum.py index b97b0fa56..6b75b7ae5 100644 --- a/tests/test_enum.py +++ b/tests/test_enum.py @@ -60,9 +60,7 @@ Members: ETwo : Docstring for ETwo - EThree : Docstring for EThree""".split( - "\n" - ): + EThree : Docstring for EThree""".split("\n"): assert docstring_line in m.UnscopedEnum.__doc__ # Unscoped enums will accept ==/!= int comparisons diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py index 955a85f67..7fdf4e3af 100644 --- a/tests/test_methods_and_attributes.py +++ b/tests/test_methods_and_attributes.py @@ -232,25 +232,29 @@ def test_no_mixed_overloads(): with pytest.raises(RuntimeError) as excinfo: m.ExampleMandA.add_mixed_overloads1() - assert str( - excinfo.value - ) == "overloading a method with both static and instance methods is not supported; " + ( - "#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for more details" - if not detailed_error_messages_enabled - else "error while attempting to bind static method ExampleMandA.overload_mixed1" - "(arg0: float) -> str" + assert ( + str(excinfo.value) + == "overloading a method with both static and instance methods is not supported; " + + ( + "#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for more details" + if not detailed_error_messages_enabled + else "error while attempting to bind static method ExampleMandA.overload_mixed1" + "(arg0: float) -> str" + ) ) with pytest.raises(RuntimeError) as excinfo: m.ExampleMandA.add_mixed_overloads2() - assert str( - excinfo.value - ) == "overloading a method with both static and instance methods is not supported; " + ( - "#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for more details" - if not detailed_error_messages_enabled - else "error while attempting to bind instance method ExampleMandA.overload_mixed2" - "(self: pybind11_tests.methods_and_attributes.ExampleMandA, arg0: int, arg1: int)" - " -> str" + assert ( + str(excinfo.value) + == "overloading a method with both static and instance methods is not supported; " + + ( + "#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for more details" + if not detailed_error_messages_enabled + else "error while attempting to bind instance method ExampleMandA.overload_mixed2" + "(self: pybind11_tests.methods_and_attributes.ExampleMandA, arg0: int, arg1: int)" + " -> str" + ) ) From 2c35fde389aceaad1b5390313ebf77d36afb97e5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 8 Nov 2023 11:16:10 -0800 Subject: [PATCH 10/11] Fix refcount bug introduced with PR #4916. (#4927) https://github.com/pybind/pybind11/pull/4916/files#r1387035547 --- include/pybind11/pybind11.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 3e1a057db..b87fe66b2 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2759,7 +2759,6 @@ get_type_override(const void *this_ptr, const type_info *this_type, const char * PyObject *self_arg = PyTuple_GET_ITEM(co_varnames, 0); Py_DECREF(co_varnames); PyObject *self_caller = dict_getitem(locals, self_arg); - Py_DECREF(locals); if (self_caller == self.ptr()) { Py_DECREF(f_code); Py_DECREF(frame); From e250155afadde7100e627e6aa4a541137a863243 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 8 Nov 2023 12:44:04 -0800 Subject: [PATCH 11/11] Fix a long-standing bug in the handling of Python multiple inheritance (#4762) * Equivalent of https://github.com/google/clif/commit/5718e4d0807fd3b6a8187dde140069120b81ecef * Resolve clang-tidy errors. * Moving test_PPCCInit() first changes the behavior! * Resolve new Clang dev C++11 errors: ``` The CXX compiler identification is Clang 17.0.0 ``` ``` pytypes.h:1615:23: error: identifier '_s' preceded by whitespace in a literal operator declaration is deprecated [-Werror,-Wdeprecated-literal-operator] ``` ``` cast.h:1380:26: error: identifier '_a' preceded by whitespace in a literal operator declaration is deprecated [-Werror,-Wdeprecated-literal-operator] ``` * Resolve gcc 4.8.5 error: ``` pytypes.h:1615:12: error: missing space between '""' and suffix identifier ``` * Specifically exclude `__clang__` * Snapshot of debugging code (does NOT pass pre-commit checks). * Revert "Snapshot of debugging code (does NOT pass pre-commit checks)." This reverts commit 1d4f9ff2632b32ddcb0dc7ecd0ab7a4ce4c15a4e. * [ci skip] Order Dependence Demo * Revert "[ci skip] Order Dependence Demo" This reverts commit d37b5409d4e5b835620ccbb321a4e1ba89af315c. * One way to deal with the order dependency issue. This is not the best way, more like a proof of concept. * Move test_PC() first again. * Add `all_type_info_add_base_most_derived_first()`, use in `all_type_info_populate()` * Revert "One way to deal with the order dependency issue. This is not the best way, more like a proof of concept." This reverts commit eb09c6c1b978208ceee40f05bbe75491b6ff8ad6. * clang-tidy fixes (automatic) * Add `is_redundant_value_and_holder()` and use to avoid forcing `__init__` overrides when they are not needed. * Streamline implementation and avoid unsafe `reinterpret_cast()` introduced with PR #2152 The `reinterpret_cast(self)` is unsafe if `__new__` is mocked, which was actually found in the wild: the mock returned `None` for `self`. This was inconsequential because `inst` is currently cast straight back to `PyObject *` to compute `all_type_info()`, which is empty if `self` is not a pybind11 `instance`, and then `inst` is never dereferenced. However, the unsafe detour through `instance *` is easily avoided and the updated implementation is less prone to accidents while debugging or refactoring. * Fix actual undefined behavior exposed by previous changes. It turns out the previous commit message is incorrect, the `inst` pointer is actually dereferenced, in the `value_and_holder` ctor here: https://github.com/pybind/pybind11/blob/f3e0602802c7840992c97f4960515777cad6a5c7/include/pybind11/detail/type_caster_base.h#L262-L263 ``` 259 // Main constructor for a found value/holder: 260 value_and_holder(instance *i, const detail::type_info *type, size_t vpos, size_t index) 261 : inst{i}, index{index}, type{type}, 262 vh{inst->simple_layout ? inst->simple_value_holder 263 : &inst->nonsimple.values_and_holders[vpos]} {} ``` * Add test_mock_new() * Experiment: specify indirect bases * Revert "Experiment: specify indirect bases" This reverts commit 4f90d85f9fc15290d6be54d5ae9417bd131b84d9. * Add `all_type_info_check_for_divergence()` and some tests. * Call `all_type_info_check_for_divergence()` also from `type_caster_generic::load_impl<>` * Resolve clang-tidy error: ``` include/pybind11/detail/type_caster_base.h:795:21: error: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty,-warnings-as-errors] if (matching_bases.size() != 0) { ^~~~~~~~~~~~~~~~~~~~~~~~~~ !matching_bases.empty() ``` * Revert "Resolve clang-tidy error:" This reverts commit df27188dc6d6cf333145755543f56b2f6657aa5e. * Revert "Call `all_type_info_check_for_divergence()` also from `type_caster_generic::load_impl<>`" This reverts commit 5f5fd6a68e3cff1726628f6dea8e1c0754636a23. * Revert "Add `all_type_info_check_for_divergence()` and some tests." This reverts commit 0a9599f775bfd3ca196c5e23a3fcf2890cbf6e82. --- include/pybind11/detail/class.h | 8 ++-- include/pybind11/detail/type_caster_base.h | 49 ++++++++++++++++++---- tests/CMakeLists.txt | 1 + tests/test_class.py | 14 +++++++ tests/test_python_multiple_inheritance.cpp | 45 ++++++++++++++++++++ tests/test_python_multiple_inheritance.py | 35 ++++++++++++++++ 6 files changed, 140 insertions(+), 12 deletions(-) create mode 100644 tests/test_python_multiple_inheritance.cpp create mode 100644 tests/test_python_multiple_inheritance.py diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 3ece0643b..b31727167 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -189,12 +189,10 @@ extern "C" inline PyObject *pybind11_meta_call(PyObject *type, PyObject *args, P return nullptr; } - // This must be a pybind11 instance - auto *instance = reinterpret_cast(self); - // Ensure that the base __init__ function(s) were called - for (const auto &vh : values_and_holders(instance)) { - if (!vh.holder_constructed()) { + values_and_holders vhs(self); + for (const auto &vh : vhs) { + if (!vh.holder_constructed() && !vhs.is_redundant_value_and_holder(vh)) { PyErr_Format(PyExc_TypeError, "%.200s.__init__() must be called when overriding __init__", get_fully_qualified_tp_name(vh.type->type).c_str()); diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 68512b5bd..476646ee8 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -102,8 +102,22 @@ public: inline std::pair all_type_info_get_cache(PyTypeObject *type); +// Band-aid workaround to fix a subtle but serious bug in a minimalistic fashion. See PR #4762. +inline void all_type_info_add_base_most_derived_first(std::vector &bases, + type_info *addl_base) { + for (auto it = bases.begin(); it != bases.end(); it++) { + type_info *existing_base = *it; + if (PyType_IsSubtype(addl_base->type, existing_base->type) != 0) { + bases.insert(it, addl_base); + return; + } + } + bases.push_back(addl_base); +} + // Populates a just-created cache entry. PYBIND11_NOINLINE void all_type_info_populate(PyTypeObject *t, std::vector &bases) { + assert(bases.empty()); std::vector check; for (handle parent : reinterpret_borrow(t->tp_bases)) { check.push_back((PyTypeObject *) parent.ptr()); @@ -136,7 +150,7 @@ PYBIND11_NOINLINE void all_type_info_populate(PyTypeObject *t, std::vectortp_bases) { @@ -322,18 +336,29 @@ public: explicit values_and_holders(instance *inst) : inst{inst}, tinfo(all_type_info(Py_TYPE(inst))) {} + explicit values_and_holders(PyObject *obj) + : inst{nullptr}, tinfo(all_type_info(Py_TYPE(obj))) { + if (!tinfo.empty()) { + inst = reinterpret_cast(obj); + } + } + struct iterator { private: instance *inst = nullptr; const type_vec *types = nullptr; value_and_holder curr; friend struct values_and_holders; - iterator(instance *inst, const type_vec *tinfo) - : inst{inst}, types{tinfo}, - curr(inst /* instance */, - types->empty() ? nullptr : (*types)[0] /* type info */, - 0, /* vpos: (non-simple types only): the first vptr comes first */ - 0 /* index */) {} + iterator(instance *inst, const type_vec *tinfo) : inst{inst}, types{tinfo} { + if (inst != nullptr) { + assert(!types->empty()); + curr = value_and_holder( + inst /* instance */, + (*types)[0] /* type info */, + 0, /* vpos: (non-simple types only): the first vptr comes first */ + 0 /* index */); + } + } // Past-the-end iterator: explicit iterator(size_t end) : curr(end) {} @@ -364,6 +389,16 @@ public: } size_t size() { return tinfo.size(); } + + // Band-aid workaround to fix a subtle but serious bug in a minimalistic fashion. See PR #4762. + bool is_redundant_value_and_holder(const value_and_holder &vh) { + for (size_t i = 0; i < vh.index; i++) { + if (PyType_IsSubtype(tinfo[i]->type, tinfo[vh.index]->type) != 0) { + return true; + } + } + return false; + } }; /** diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 80ee9c1f1..d68067df6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -144,6 +144,7 @@ set(PYBIND11_TEST_FILES test_opaque_types test_operator_overloading test_pickling + test_python_multiple_inheritance test_pytypes test_sequences_and_iterators test_smart_ptr diff --git a/tests/test_class.py b/tests/test_class.py index ee7467cf8..73a48309e 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -1,3 +1,5 @@ +from unittest import mock + import pytest import env @@ -203,6 +205,18 @@ def test_inheritance_init(msg): assert msg(exc_info.value) == expected +@pytest.mark.parametrize( + "mock_return_value", [None, (1, 2, 3), m.Pet("Polly", "parrot"), m.Dog("Molly")] +) +def test_mock_new(mock_return_value): + with mock.patch.object( + m.Pet, "__new__", return_value=mock_return_value + ) as mock_new: + obj = m.Pet("Noname", "Nospecies") + assert obj is mock_return_value + mock_new.assert_called_once_with(m.Pet, "Noname", "Nospecies") + + def test_automatic_upcasting(): assert type(m.return_class_1()).__name__ == "DerivedClass1" assert type(m.return_class_2()).__name__ == "DerivedClass2" diff --git a/tests/test_python_multiple_inheritance.cpp b/tests/test_python_multiple_inheritance.cpp new file mode 100644 index 000000000..689917158 --- /dev/null +++ b/tests/test_python_multiple_inheritance.cpp @@ -0,0 +1,45 @@ +#include "pybind11_tests.h" + +namespace test_python_multiple_inheritance { + +// Copied from: +// https://github.com/google/clif/blob/5718e4d0807fd3b6a8187dde140069120b81ecef/clif/testing/python_multiple_inheritance.h + +struct CppBase { + explicit CppBase(int value) : base_value(value) {} + int get_base_value() const { return base_value; } + void reset_base_value(int new_value) { base_value = new_value; } + +private: + int base_value; +}; + +struct CppDrvd : CppBase { + explicit CppDrvd(int value) : CppBase(value), drvd_value(value * 3) {} + int get_drvd_value() const { return drvd_value; } + void reset_drvd_value(int new_value) { drvd_value = new_value; } + + int get_base_value_from_drvd() const { return get_base_value(); } + void reset_base_value_from_drvd(int new_value) { reset_base_value(new_value); } + +private: + int drvd_value; +}; + +} // namespace test_python_multiple_inheritance + +TEST_SUBMODULE(python_multiple_inheritance, m) { + using namespace test_python_multiple_inheritance; + + py::class_(m, "CppBase") + .def(py::init()) + .def("get_base_value", &CppBase::get_base_value) + .def("reset_base_value", &CppBase::reset_base_value); + + py::class_(m, "CppDrvd") + .def(py::init()) + .def("get_drvd_value", &CppDrvd::get_drvd_value) + .def("reset_drvd_value", &CppDrvd::reset_drvd_value) + .def("get_base_value_from_drvd", &CppDrvd::get_base_value_from_drvd) + .def("reset_base_value_from_drvd", &CppDrvd::reset_base_value_from_drvd); +} diff --git a/tests/test_python_multiple_inheritance.py b/tests/test_python_multiple_inheritance.py new file mode 100644 index 000000000..3bddd67df --- /dev/null +++ b/tests/test_python_multiple_inheritance.py @@ -0,0 +1,35 @@ +# Adapted from: +# https://github.com/google/clif/blob/5718e4d0807fd3b6a8187dde140069120b81ecef/clif/testing/python/python_multiple_inheritance_test.py + +from pybind11_tests import python_multiple_inheritance as m + + +class PC(m.CppBase): + pass + + +class PPCC(PC, m.CppDrvd): + pass + + +def test_PC(): + d = PC(11) + assert d.get_base_value() == 11 + d.reset_base_value(13) + assert d.get_base_value() == 13 + + +def test_PPCC(): + d = PPCC(11) + assert d.get_drvd_value() == 33 + d.reset_drvd_value(55) + assert d.get_drvd_value() == 55 + + assert d.get_base_value() == 11 + assert d.get_base_value_from_drvd() == 11 + d.reset_base_value(20) + assert d.get_base_value() == 20 + assert d.get_base_value_from_drvd() == 20 + d.reset_base_value_from_drvd(30) + assert d.get_base_value() == 30 + assert d.get_base_value_from_drvd() == 30