From b21b049029798d5dc2ae0db048ffcafb67f690e5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 1 Jul 2024 23:03:52 -0700 Subject: [PATCH 01/15] chore(deps): update pre-commit hooks (#5220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-clang-format: v18.1.5 → v18.1.8](https://github.com/pre-commit/mirrors-clang-format/compare/v18.1.5...v18.1.8) - [github.com/astral-sh/ruff-pre-commit: v0.4.7 → v0.5.0](https://github.com/astral-sh/ruff-pre-commit/compare/v0.4.7...v0.5.0) - [github.com/pre-commit/mirrors-mypy: v1.10.0 → v1.10.1](https://github.com/pre-commit/mirrors-mypy/compare/v1.10.0...v1.10.1) - [github.com/adamchainz/blacken-docs: 1.16.0 → 1.18.0](https://github.com/adamchainz/blacken-docs/compare/1.16.0...1.18.0) - [github.com/PyCQA/pylint: v3.2.2 → v3.2.4](https://github.com/PyCQA/pylint/compare/v3.2.2...v3.2.4) - [github.com/python-jsonschema/check-jsonschema: 0.28.4 → 0.28.6](https://github.com/python-jsonschema/check-jsonschema/compare/0.28.4...0.28.6) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3cec1ebe..92469eb3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,14 +25,14 @@ repos: # Clang format the codebase automatically - repo: https://github.com/pre-commit/mirrors-clang-format - rev: "v18.1.5" + rev: "v18.1.8" hooks: - id: clang-format types_or: [c++, c, cuda] # Ruff, the Python auto-correcting linter/formatter written in Rust - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.4.7 + rev: v0.5.0 hooks: - id: ruff args: ["--fix", "--show-fixes"] @@ -40,7 +40,7 @@ repos: # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: "v1.10.0" + rev: "v1.10.1" hooks: - id: mypy args: [] @@ -79,7 +79,7 @@ repos: # Also code format the docs - repo: https://github.com/adamchainz/blacken-docs - rev: "1.16.0" + rev: "1.18.0" hooks: - id: blacken-docs additional_dependencies: @@ -142,14 +142,14 @@ repos: # PyLint has native support - not always usable, but works for us - repo: https://github.com/PyCQA/pylint - rev: "v3.2.2" + rev: "v3.2.4" hooks: - id: pylint files: ^pybind11 # Check schemas on some of our YAML files - repo: https://github.com/python-jsonschema/check-jsonschema - rev: 0.28.4 + rev: 0.28.6 hooks: - id: check-readthedocs - id: check-github-workflows From bb05e0810b87e74709d9f4c4545f1f57a1b386f5 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 2 Jul 2024 12:58:09 -0400 Subject: [PATCH 02/15] Use PyMutex instead of std::mutex in free-threaded build. (#5219) * Use PyMutex instead of std::mutex in free-threaded build. PyMutex is now part of the public C API as of 3.13.0b3 and generally has slightly less overhead than std::mutex. * style: pre-commit fixes * Fix instance_map_shard padding --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/pybind11/detail/internals.h | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index e61c1687..3f0a6a94 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -148,20 +148,35 @@ struct override_hash { using instance_map = std::unordered_multimap; +#ifdef Py_GIL_DISABLED +// Wrapper around PyMutex to provide BasicLockable semantics +class pymutex { + PyMutex mutex; + +public: + pymutex() : mutex({}) {} + void lock() { PyMutex_Lock(&mutex); } + void unlock() { PyMutex_Unlock(&mutex); } +}; + // Instance map shards are used to reduce mutex contention in free-threaded Python. struct instance_map_shard { - std::mutex mutex; instance_map registered_instances; + pymutex mutex; // alignas(64) would be better, but causes compile errors in macOS before 10.14 (see #5200) - char padding[64 - (sizeof(std::mutex) + sizeof(instance_map)) % 64]; + char padding[64 - (sizeof(instance_map) + sizeof(pymutex)) % 64]; }; +static_assert(sizeof(instance_map_shard) % 64 == 0, + "instance_map_shard size is not a multiple of 64 bytes"); +#endif + /// Internal data structure used to track registered instances and types. /// Whenever binary incompatible changes are made to this structure, /// `PYBIND11_INTERNALS_VERSION` must be incremented. struct internals { #ifdef Py_GIL_DISABLED - std::mutex mutex; + pymutex mutex; #endif // std::type_index -> pybind11's type information type_map registered_types_cpp; @@ -614,7 +629,7 @@ inline local_internals &get_local_internals() { } #ifdef Py_GIL_DISABLED -# define PYBIND11_LOCK_INTERNALS(internals) std::unique_lock lock((internals).mutex) +# define PYBIND11_LOCK_INTERNALS(internals) std::unique_lock lock((internals).mutex) #else # define PYBIND11_LOCK_INTERNALS(internals) #endif @@ -651,7 +666,7 @@ inline auto with_instance_map(const void *ptr, auto idx = static_cast(hash & internals.instance_shards_mask); auto &shard = internals.instance_shards[idx]; - std::unique_lock lock(shard.mutex); + std::unique_lock lock(shard.mutex); return cb(shard.registered_instances); #else (void) ptr; @@ -667,7 +682,7 @@ inline size_t num_registered_instances() { size_t count = 0; for (size_t i = 0; i <= internals.instance_shards_mask; ++i) { auto &shard = internals.instance_shards[i]; - std::unique_lock lock(shard.mutex); + std::unique_lock lock(shard.mutex); count += shard.registered_instances.size(); } return count; From 50acb81b0a0f769dedd265727ade44e03778a977 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 6 Jul 2024 12:22:52 -0700 Subject: [PATCH 03/15] chore(deps): bump certifi from 2024.2.2 to 2024.7.4 in /docs (#5226) Bumps [certifi](https://github.com/certifi/python-certifi) from 2024.2.2 to 2024.7.4. - [Commits](https://github.com/certifi/python-certifi/compare/2024.02.02...2024.07.04) --- updated-dependencies: - dependency-name: certifi dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- docs/requirements.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/requirements.txt b/docs/requirements.txt index 293db6a0..4e53f035 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -16,9 +16,9 @@ breathe==4.35.0 \ --hash=sha256:5165541c3c67b6c7adde8b3ecfe895c6f7844783c4076b6d8d287e4f33d62386 \ --hash=sha256:52c581f42ca4310737f9e435e3851c3d1f15446205a85fbc272f1f97ed74f5be # via -r requirements.in -certifi==2024.2.2 \ - --hash=sha256:0569859f95fc761b18b45ef421b1290a0f65f147e92a1e5eb3e635f9a5e4e66f \ - --hash=sha256:dc383c07b76109f368f6106eee2b593b04a011ea4d55f652c6ca24a754d1cdd1 +certifi==2024.7.4 \ + --hash=sha256:5a1e7645bc0ec61a09e26c36f6106dd4cf40c6db3a1fb6352b0244e7fb057c7b \ + --hash=sha256:c198e21b1289c2ab85ee4e67bb4b4ef3ead0892059901a8d5b622f24a1101e90 # via requests charset-normalizer==3.3.2 \ --hash=sha256:06435b539f889b1f6f4ac1758871aae42dc3a8c0e24ac9e60c2384973ad73027 \ From 9193b8e7143594723e6272e6501e550237199cdc Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 9 Jul 2024 13:27:31 -0700 Subject: [PATCH 04/15] Additional `assert is_disowned()` in test_class_sh_disowning.py (#5234) --- tests/test_class_sh_disowning.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/test_class_sh_disowning.py b/tests/test_class_sh_disowning.py index 63cdd4ed..52568203 100644 --- a/tests/test_class_sh_disowning.py +++ b/tests/test_class_sh_disowning.py @@ -5,17 +5,26 @@ from pybind11_tests import class_sh_disowning as m +def is_disowned(obj): + try: + obj.get() + except ValueError: + return True + return False + + def test_same_twice(): while True: obj1a = m.Atype1(57) obj1b = m.Atype1(62) assert m.same_twice(obj1a, obj1b) == (57 * 10 + 1) * 100 + (62 * 10 + 1) * 10 + assert is_disowned(obj1a) + assert is_disowned(obj1b) obj1c = m.Atype1(0) with pytest.raises(ValueError): # Disowning works for one argument, but not both. m.same_twice(obj1c, obj1c) - with pytest.raises(ValueError): - obj1c.get() + assert is_disowned(obj1c) return # Comment out for manual leak checking (use `top` command). @@ -25,6 +34,8 @@ def test_mixed(): obj1a = m.Atype1(90) obj2a = m.Atype2(25) assert m.mixed(obj1a, obj2a) == (90 * 10 + 1) * 200 + (25 * 10 + 2) * 20 + assert is_disowned(obj1a) + assert is_disowned(obj2a) # The C++ order of evaluation of function arguments is (unfortunately) unspecified: # https://en.cppreference.com/w/cpp/language/eval_order @@ -40,13 +51,6 @@ def test_mixed(): # the already disowned obj1a fails as expected. m.mixed(obj1a, obj2b) - def is_disowned(obj): - try: - obj.get() - except ValueError: - return True - return False - # Either obj1b or obj2b was disowned in the expected failed m.mixed() calls above, but not # both. is_disowned_results = (is_disowned(obj1b), is_disowned(obj2b)) From ccefee4c3187c2892fcf4590b1bbc850134b84bb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Jul 2024 16:51:27 -0400 Subject: [PATCH 05/15] chore(deps): bump actions/attest-build-provenance in the actions group (#5243) Bumps the actions group with 1 update: [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance). Updates `actions/attest-build-provenance` from 1.3.2 to 1.3.3 - [Release notes](https://github.com/actions/attest-build-provenance/releases) - [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md) - [Commits](https://github.com/actions/attest-build-provenance/compare/bdd51370e0416ac948727f861e03c2f05d32d78e...5e9cb68e95676991667494a6a4e59b8a2f13e1d0) --- updated-dependencies: - dependency-name: actions/attest-build-provenance dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/pip.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index fb9c62ca..c1acb8bb 100644 --- a/.github/workflows/pip.yml +++ b/.github/workflows/pip.yml @@ -102,7 +102,7 @@ jobs: - uses: actions/download-artifact@v4 - name: Generate artifact attestation for sdist and wheel - uses: actions/attest-build-provenance@bdd51370e0416ac948727f861e03c2f05d32d78e # v1.3.2 + uses: actions/attest-build-provenance@5e9cb68e95676991667494a6a4e59b8a2f13e1d0 # v1.3.3 with: subject-path: "*/pybind11*" From 43de8014f96ab8fc0028f11a26edbcf9b937cd91 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 16 Jul 2024 14:06:54 -0400 Subject: [PATCH 06/15] fix: make gil_safe_call_once thread-safe in free-threaded CPython (#5246) * fix: Make gil_safe_call_once thread-safe in free-threaded CPython The "is_initialized_" flags is not protected by the GIL in free-threaded Python, so it needs to be an atomic field. Fixes #5245 * style: pre-commit fixes * Apply changes from code review --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/pybind11/gil_safe_call_once.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/include/pybind11/gil_safe_call_once.h b/include/pybind11/gil_safe_call_once.h index eaf84d16..5f9e1b03 100644 --- a/include/pybind11/gil_safe_call_once.h +++ b/include/pybind11/gil_safe_call_once.h @@ -8,6 +8,10 @@ #include #include +#ifdef Py_GIL_DISABLED +# include +#endif + PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) // Use the `gil_safe_call_once_and_store` class below instead of the naive @@ -82,7 +86,12 @@ class gil_safe_call_once_and_store { private: alignas(T) char storage_[sizeof(T)] = {}; std::once_flag once_flag_ = {}; - bool is_initialized_ = false; +#ifdef Py_GIL_DISABLED + std::atomic_bool +#else + bool +#endif + is_initialized_{false}; // The `is_initialized_`-`storage_` pair is very similar to `std::optional`, // but the latter does not have the triviality properties of former, // therefore `std::optional` is not a viable alternative here. From dbf848aff7c37ef8798bc9459a86193e28b1032f Mon Sep 17 00:00:00 2001 From: Ralf Gommers Date: Thu, 18 Jul 2024 08:26:45 +0200 Subject: [PATCH 07/15] docs: extend `PYBIND11_MODULE` documentation, mention `mod_gil_not_used` (#5250) This follows up on PR 5148, which introduced support for free-threaded CPython. --- include/pybind11/detail/common.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index fc66e6f7..9418334c 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -462,6 +462,22 @@ PYBIND11_WARNING_POP return "Hello, World!"; }); } + + The third macro argument is optional (available since 2.13.0), and can be used to + mark the extension module as safe to run without the GIL under a free-threaded CPython + interpreter. Passing this argument has no effect on other interpreters. + + .. code-block:: cpp + + PYBIND11_MODULE(example, m, py::mod_gil_not_used()) { + m.doc() = "pybind11 example module safe to run without the GIL"; + + // Add bindings here + m.def("foo", []() { + return "Hello, Free-threaded World!"; + }); + } + \endrst */ #define PYBIND11_MODULE(name, variable, ...) \ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name) \ From a582ca8a8e2705be848df164158c605e15e010fb Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 18 Jul 2024 14:50:38 -0400 Subject: [PATCH 08/15] tests: run on pyodide (#4745) * tests: run on pyodide Signed-off-by: Henry Schreiner * ci: use cibuildwheel for pyodide test Signed-off-by: Henry Schreiner * tests: revert changes to test_embed Signed-off-by: Henry Schreiner --------- Signed-off-by: Henry Schreiner --- .github/workflows/emscripten.yaml | 30 ++++++++++++++++++++++++++++++ tests/CMakeLists.txt | 10 +++++++++- tests/pyproject.toml | 17 +++++++++++++++++ tests/test_async.py | 5 +++++ tests/test_callbacks.py | 3 +++ tests/test_exceptions.py | 2 +- tests/test_gil_scoped.py | 13 +++++++++---- tests/test_iostream.py | 4 ++++ tests/test_thread.py | 5 +++++ tests/test_virtual_functions.py | 3 +++ 10 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/emscripten.yaml create mode 100644 tests/pyproject.toml diff --git a/.github/workflows/emscripten.yaml b/.github/workflows/emscripten.yaml new file mode 100644 index 00000000..4f12e81c --- /dev/null +++ b/.github/workflows/emscripten.yaml @@ -0,0 +1,30 @@ +name: WASM + +on: + workflow_dispatch: + pull_request: + branches: + - master + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + build-wasm-emscripten: + name: Pyodide wheel + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + with: + submodules: true + fetch-depth: 0 + + - uses: pypa/cibuildwheel@v2.19 + env: + PYODIDE_BUILD_EXPORTS: whole_archive + CFLAGS: -fexceptions + LDFLAGS: -fexceptions + with: + package-dir: tests + only: cp312-pyodide_wasm32 diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f182e249..aae9be72 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -88,7 +88,12 @@ set(PYBIND11_TEST_FILTER if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR) # We're being loaded directly, i.e. not via add_subdirectory, so make this # work as its own project and load the pybind11Config to get the tools we need - find_package(pybind11 REQUIRED CONFIG) + + if(SKBUILD) + add_subdirectory(.. pybind11_src) + else() + find_package(pybind11 REQUIRED CONFIG) + endif() endif() if(NOT CMAKE_BUILD_TYPE AND NOT DEFINED CMAKE_CONFIGURATION_TYPES) @@ -489,6 +494,9 @@ foreach(target ${test_targets}) endforeach() endif() endif() + if(SKBUILD) + install(TARGETS ${target} LIBRARY DESTINATION .) + endif() endforeach() # Provide nice organisation in IDEs diff --git a/tests/pyproject.toml b/tests/pyproject.toml new file mode 100644 index 00000000..469c145d --- /dev/null +++ b/tests/pyproject.toml @@ -0,0 +1,17 @@ +# Warning: this is currently used for pyodide, and is not a general out-of-tree +# builder for the tests (yet). Specifically, wheels can't be built from SDists. + +[build-system] +requires = ["scikit-build-core"] +build-backend = "scikit_build_core.build" + +[project] +name = "pybind11_tests" +version = "0.0.1" +dependencies = ["pytest", "pytest-timeout", "numpy", "scipy"] + +[tool.scikit-build.cmake.define] +PYBIND11_FINDPYTHON = true + +[tool.cibuildwheel] +test-command = "pytest -o timeout=0 -p no:cacheprovider {project}/tests/test_*.py" diff --git a/tests/test_async.py b/tests/test_async.py index 4d33ba65..1705196d 100644 --- a/tests/test_async.py +++ b/tests/test_async.py @@ -1,10 +1,15 @@ from __future__ import annotations +import sys + import pytest asyncio = pytest.importorskip("asyncio") m = pytest.importorskip("pybind11_tests.async_module") +if sys.platform.startswith("emscripten"): + pytest.skip("Can't run a new event_loop in pyodide", allow_module_level=True) + @pytest.fixture() def event_loop(): diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index ce2a6d25..db6d8dec 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -1,5 +1,6 @@ from __future__ import annotations +import sys import time from threading import Thread @@ -153,6 +154,7 @@ def test_python_builtins(): assert m.test_sum_builtin(sum, []) == 0 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_async_callbacks(): # serves as state for async callback class Item: @@ -176,6 +178,7 @@ def gen_f(): assert sum(res) == sum(x + 3 for x in work) +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_async_async_callbacks(): t = Thread(target=test_async_callbacks) t.start() diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index b33997ee..db2a551e 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -75,7 +75,7 @@ def test_cross_module_exceptions(msg): # TODO: FIXME @pytest.mark.xfail( - "env.MACOS and (env.PYPY or pybind11_tests.compiler_info.startswith('Homebrew Clang'))", + "env.MACOS and (env.PYPY or pybind11_tests.compiler_info.startswith('Homebrew Clang')) or sys.platform.startswith('emscripten')", raises=RuntimeError, reason="See Issue #2847, PR #2999, PR #4324", ) diff --git a/tests/test_gil_scoped.py b/tests/test_gil_scoped.py index a1838768..eab92093 100644 --- a/tests/test_gil_scoped.py +++ b/tests/test_gil_scoped.py @@ -71,24 +71,28 @@ def test_cross_module_gil_inner_pybind11_acquired(): m.test_cross_module_gil_inner_pybind11_acquired() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_cross_module_gil_nested_custom_released(): """Makes sure that the GIL can be nested acquired/released by another module from a GIL-released state using custom locking logic.""" m.test_cross_module_gil_nested_custom_released() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_cross_module_gil_nested_custom_acquired(): """Makes sure that the GIL can be nested acquired/acquired by another module from a GIL-acquired state using custom locking logic.""" m.test_cross_module_gil_nested_custom_acquired() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_cross_module_gil_nested_pybind11_released(): """Makes sure that the GIL can be nested acquired/released by another module from a GIL-released state using pybind11 locking logic.""" m.test_cross_module_gil_nested_pybind11_released() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_cross_module_gil_nested_pybind11_acquired(): """Makes sure that the GIL can be nested acquired/acquired by another module from a GIL-acquired state using pybind11 locking logic.""" @@ -103,6 +107,7 @@ def test_nested_acquire(): assert m.test_nested_acquire(0xAB) == "171" +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_multi_acquire_release_cross_module(): for bits in range(16 * 8): internals_ids = m.test_multi_acquire_release_cross_module(bits) @@ -204,7 +209,7 @@ def _run_in_threads(test_fn, num_threads, parallel): thread.join() -# TODO: FIXME, sometimes returns -11 (segfault) instead of 0 on macOS Python 3.9 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) def test_run_in_process_one_thread(test_fn): """Makes sure there is no GIL deadlock when running in a thread. @@ -214,7 +219,7 @@ def test_run_in_process_one_thread(test_fn): assert _run_in_process(_run_in_threads, test_fn, num_threads=1, parallel=False) == 0 -# TODO: FIXME on macOS Python 3.9 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) def test_run_in_process_multiple_threads_parallel(test_fn): """Makes sure there is no GIL deadlock when running in a thread multiple times in parallel. @@ -224,7 +229,7 @@ def test_run_in_process_multiple_threads_parallel(test_fn): assert _run_in_process(_run_in_threads, test_fn, num_threads=8, parallel=True) == 0 -# TODO: FIXME on macOS Python 3.9 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) def test_run_in_process_multiple_threads_sequential(test_fn): """Makes sure there is no GIL deadlock when running in a thread multiple times sequentially. @@ -234,7 +239,7 @@ def test_run_in_process_multiple_threads_sequential(test_fn): assert _run_in_process(_run_in_threads, test_fn, num_threads=8, parallel=False) == 0 -# TODO: FIXME on macOS Python 3.9 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") @pytest.mark.parametrize("test_fn", ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK) def test_run_in_process_direct(test_fn): """Makes sure there is no GIL deadlock when using processes. diff --git a/tests/test_iostream.py b/tests/test_iostream.py index f7eeff50..c3d98778 100644 --- a/tests/test_iostream.py +++ b/tests/test_iostream.py @@ -1,8 +1,11 @@ from __future__ import annotations +import sys from contextlib import redirect_stderr, redirect_stdout from io import StringIO +import pytest + from pybind11_tests import iostream as m @@ -270,6 +273,7 @@ def test_redirect_both(capfd): assert stream2.getvalue() == msg2 +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_threading(): with m.ostream_redirect(stdout=True, stderr=False): # start some threads diff --git a/tests/test_thread.py b/tests/test_thread.py index 4541a305..f12020e4 100644 --- a/tests/test_thread.py +++ b/tests/test_thread.py @@ -1,7 +1,10 @@ from __future__ import annotations +import sys import threading +import pytest + from pybind11_tests import thread as m @@ -24,6 +27,7 @@ def join(self): raise self.e +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_implicit_conversion(): a = Thread(m.test) b = Thread(m.test) @@ -34,6 +38,7 @@ def test_implicit_conversion(): x.join() +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_implicit_conversion_no_gil(): a = Thread(m.test_no_gil) b = Thread(m.test_no_gil) diff --git a/tests/test_virtual_functions.py b/tests/test_virtual_functions.py index 08acaa19..bc017778 100644 --- a/tests/test_virtual_functions.py +++ b/tests/test_virtual_functions.py @@ -1,5 +1,7 @@ from __future__ import annotations +import sys + import pytest import env # noqa: F401 @@ -435,6 +437,7 @@ def lucky_number(self): assert obj.say_everything() == "BT -7" +@pytest.mark.skipif(sys.platform.startswith("emscripten"), reason="Requires threads") def test_issue_1454(): # Fix issue #1454 (crash when acquiring/releasing GIL on another thread in Python 2.7) m.test_gil() From 6d4805ced1f8fc8f503d41381853f6082f3064a3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 19 Jul 2024 07:34:06 +0700 Subject: [PATCH 09/15] Small cleanup/refactoring in support of PR #5213 (#5251) * Factor out detail/value_and_holder.h (from detail/type_caster_base.h) This is in support of PR #5213: * trampoline_self_life_support.h depends on value_and_holder.h * type_caster_base.h depends on trampoline_self_life_support.h * Fix a minor and inconsequential inconsistency in `copyable_holder_caster`: the correct `load_value()` return type is `void` (as defined in `type_caster_generic`) For easy future reference, this is the long-standing inconsistency: * https://github.com/pybind/pybind11/blob/dbf848aff7c37ef8798bc9459a86193e28b1032f/include/pybind11/detail/type_caster_base.h#L634 * https://github.com/pybind/pybind11/blob/dbf848aff7c37ef8798bc9459a86193e28b1032f/include/pybind11/cast.h#L797 Noticed in passing while working on PR #5213. * Add `DANGER ZONE` comment in detail/init.h, similar to a comment added on the smart_holder branch (all the way back in 2021). --- CMakeLists.txt | 1 + include/pybind11/cast.h | 4 +- include/pybind11/detail/init.h | 6 +- include/pybind11/detail/type_caster_base.h | 62 +---------------- include/pybind11/detail/value_and_holder.h | 77 ++++++++++++++++++++++ tests/extra_python_package/test_files.py | 1 + 6 files changed, 86 insertions(+), 65 deletions(-) create mode 100644 include/pybind11/detail/value_and_holder.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 3526a1a6..6e5b8c8f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -154,6 +154,7 @@ set(PYBIND11_HEADERS include/pybind11/detail/internals.h include/pybind11/detail/type_caster_base.h include/pybind11/detail/typeid.h + include/pybind11/detail/value_and_holder.h include/pybind11/attr.h include/pybind11/buffer_info.h include/pybind11/cast.h diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index e41ad2ab..0f3091f6 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -794,11 +794,11 @@ struct copyable_holder_caster : public type_caster_base { } } - bool load_value(value_and_holder &&v_h) { + void load_value(value_and_holder &&v_h) { if (v_h.holder_constructed()) { value = v_h.value_ptr(); holder = v_h.template holder(); - return true; + return; } throw cast_error("Unable to cast from non-held to held instance (T& to Holder) " #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index 4509bd13..79cc930c 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -128,11 +128,13 @@ void construct(value_and_holder &v_h, Cpp *ptr, bool need_alias) { // the holder and destruction happens when we leave the C++ scope, and the holder // class gets to handle the destruction however it likes. v_h.value_ptr() = ptr; - v_h.set_instance_registered(true); // To prevent init_instance from registering it - v_h.type->init_instance(v_h.inst, nullptr); // Set up the holder + v_h.set_instance_registered(true); // Trick to prevent init_instance from registering it + // DANGER ZONE BEGIN: exceptions will leave v_h in an invalid state. + v_h.type->init_instance(v_h.inst, nullptr); // Set up the holder Holder temp_holder(std::move(v_h.holder>())); // Steal the holder v_h.type->dealloc(v_h); // Destroys the moved-out holder remains, resets value ptr to null v_h.set_instance_registered(false); + // DANGER ZONE END. construct_alias_from_cpp(is_alias_constructible{}, v_h, std::move(*ptr)); } else { diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index fd8c81b9..1b57ee83 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -14,6 +14,7 @@ #include "descr.h" #include "internals.h" #include "typeid.h" +#include "value_and_holder.h" #include #include @@ -259,67 +260,6 @@ PYBIND11_NOINLINE handle find_registered_python_instance(void *src, }); } -struct value_and_holder { - instance *inst = nullptr; - size_t index = 0u; - const detail::type_info *type = nullptr; - void **vh = nullptr; - - // Main constructor for a found value/holder: - value_and_holder(instance *i, const detail::type_info *type, size_t vpos, size_t index) - : inst{i}, index{index}, type{type}, - vh{inst->simple_layout ? inst->simple_value_holder - : &inst->nonsimple.values_and_holders[vpos]} {} - - // Default constructor (used to signal a value-and-holder not found by get_value_and_holder()) - value_and_holder() = default; - - // Used for past-the-end iterator - explicit value_and_holder(size_t index) : index{index} {} - - template - V *&value_ptr() const { - return reinterpret_cast(vh[0]); - } - // True if this `value_and_holder` has a non-null value pointer - explicit operator bool() const { return value_ptr() != nullptr; } - - template - H &holder() const { - return reinterpret_cast(vh[1]); - } - bool holder_constructed() const { - return inst->simple_layout - ? inst->simple_holder_constructed - : (inst->nonsimple.status[index] & instance::status_holder_constructed) != 0u; - } - // NOLINTNEXTLINE(readability-make-member-function-const) - void set_holder_constructed(bool v = true) { - if (inst->simple_layout) { - inst->simple_holder_constructed = v; - } else if (v) { - inst->nonsimple.status[index] |= instance::status_holder_constructed; - } else { - inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_holder_constructed; - } - } - bool instance_registered() const { - return inst->simple_layout - ? inst->simple_instance_registered - : ((inst->nonsimple.status[index] & instance::status_instance_registered) != 0); - } - // NOLINTNEXTLINE(readability-make-member-function-const) - void set_instance_registered(bool v = true) { - if (inst->simple_layout) { - inst->simple_instance_registered = v; - } else if (v) { - inst->nonsimple.status[index] |= instance::status_instance_registered; - } else { - inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_instance_registered; - } - } -}; - // Container for accessing and iterating over an instance's values/holders struct values_and_holders { private: diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h new file mode 100644 index 00000000..ca37d70a --- /dev/null +++ b/include/pybind11/detail/value_and_holder.h @@ -0,0 +1,77 @@ +// Copyright (c) 2016-2024 The Pybind Development Team. +// All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#pragma once + +#include "common.h" + +#include +#include + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(detail) + +struct value_and_holder { + instance *inst = nullptr; + size_t index = 0u; + const detail::type_info *type = nullptr; + void **vh = nullptr; + + // Main constructor for a found value/holder: + value_and_holder(instance *i, const detail::type_info *type, size_t vpos, size_t index) + : inst{i}, index{index}, type{type}, + vh{inst->simple_layout ? inst->simple_value_holder + : &inst->nonsimple.values_and_holders[vpos]} {} + + // Default constructor (used to signal a value-and-holder not found by get_value_and_holder()) + value_and_holder() = default; + + // Used for past-the-end iterator + explicit value_and_holder(size_t index) : index{index} {} + + template + V *&value_ptr() const { + return reinterpret_cast(vh[0]); + } + // True if this `value_and_holder` has a non-null value pointer + explicit operator bool() const { return value_ptr() != nullptr; } + + template + H &holder() const { + return reinterpret_cast(vh[1]); + } + bool holder_constructed() const { + return inst->simple_layout + ? inst->simple_holder_constructed + : (inst->nonsimple.status[index] & instance::status_holder_constructed) != 0u; + } + // NOLINTNEXTLINE(readability-make-member-function-const) + void set_holder_constructed(bool v = true) { + if (inst->simple_layout) { + inst->simple_holder_constructed = v; + } else if (v) { + inst->nonsimple.status[index] |= instance::status_holder_constructed; + } else { + inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_holder_constructed; + } + } + bool instance_registered() const { + return inst->simple_layout + ? inst->simple_instance_registered + : ((inst->nonsimple.status[index] & instance::status_instance_registered) != 0); + } + // NOLINTNEXTLINE(readability-make-member-function-const) + void set_instance_registered(bool v = true) { + if (inst->simple_layout) { + inst->simple_instance_registered = v; + } else if (v) { + inst->nonsimple.status[index] |= instance::status_instance_registered; + } else { + inst->nonsimple.status[index] &= (std::uint8_t) ~instance::status_instance_registered; + } + } +}; + +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 5a3f779a..aedbdf1c 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -58,6 +58,7 @@ "include/pybind11/detail/internals.h", "include/pybind11/detail/type_caster_base.h", "include/pybind11/detail/typeid.h", + "include/pybind11/detail/value_and_holder.h", } eigen_headers = { From e933e21e77d4b3c8fe6664b23efe4b0cf151b5da Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 18 Jul 2024 18:16:12 -0700 Subject: [PATCH 10/15] Replace `#include "detail/type_caster_base.h"` with `#include "detail/value_and_holder.h"` in trampoline_self_life_support.h. This was made possible by PR #5251. --- include/pybind11/trampoline_self_life_support.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/trampoline_self_life_support.h b/include/pybind11/trampoline_self_life_support.h index b7e1f12c..47f62395 100644 --- a/include/pybind11/trampoline_self_life_support.h +++ b/include/pybind11/trampoline_self_life_support.h @@ -6,7 +6,7 @@ #include "detail/common.h" #include "detail/smart_holder_poc.h" -#include "detail/type_caster_base.h" +#include "detail/value_and_holder.h" PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) From 8fd495afcf285746726cb1feb63b0c1998d8ded3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 20 Jul 2024 12:18:14 +0700 Subject: [PATCH 11/15] Remove test_exc_namespace_visibility.py and its dependencies. (The test will continue to live in the pybind11k repo.) (#5254) --- tests/CMakeLists.txt | 13 +---- tests/namespace_visibility.inl | 31 ------------ tests/namespace_visibility_1.cpp | 24 --------- tests/namespace_visibility_1s.cpp | 17 ------- tests/namespace_visibility_2.cpp | 17 ------- tests/test_exc_namespace_visibility.py | 68 -------------------------- 6 files changed, 2 insertions(+), 168 deletions(-) delete mode 100644 tests/namespace_visibility.inl delete mode 100644 tests/namespace_visibility_1.cpp delete mode 100644 tests/namespace_visibility_1s.cpp delete mode 100644 tests/namespace_visibility_2.cpp delete mode 100644 tests/test_exc_namespace_visibility.py diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 44fdcc8b..39ce97b1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -153,7 +153,6 @@ set(PYBIND11_TEST_FILES test_eigen_tensor test_enum test_eval - test_exc_namespace_visibility.py test_exceptions test_factory_constructors test_gil_scoped @@ -249,8 +248,6 @@ tests_extra_targets("test_exceptions.py" "cross_module_interleaved_error_already tests_extra_targets("test_gil_scoped.py" "cross_module_gil_utils") tests_extra_targets("test_class_sh_module_local.py" "class_sh_module_local_0;class_sh_module_local_1;class_sh_module_local_2") -tests_extra_targets("test_exc_namespace_visibility.py" - "namespace_visibility_1;namespace_visibility_2") set(PYBIND11_EIGEN_REPO "https://gitlab.com/libeigen/eigen.git" @@ -465,14 +462,8 @@ if(PYBIND11_CUDA_TESTS) endif() foreach(target ${test_targets}) - if("${target}" STREQUAL "pybind11_tests") - set(test_files ${PYBIND11_TEST_FILES}) - elseif("${target}" STREQUAL "namespace_visibility_1") - set(test_files namespace_visibility_1s.cpp) - if(PYBIND11_CUDA_TESTS) - set_property(SOURCE namespace_visibility_1s.cpp PROPERTY LANGUAGE CUDA) - endif() - else() + set(test_files ${PYBIND11_TEST_FILES}) + if(NOT "${target}" STREQUAL "pybind11_tests") set(test_files "") endif() diff --git a/tests/namespace_visibility.inl b/tests/namespace_visibility.inl deleted file mode 100644 index 9ff407a5..00000000 --- a/tests/namespace_visibility.inl +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) 2022 The Pybind Development Team. -// All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -#pragma once - -#include - -#ifdef __GNUG__ -# define PYBIND11_NS_VIS_U /* unspecified */ -# define PYBIND11_NS_VIS_H __attribute__((visibility("hidden"))) -#else -# define PYBIND11_NS_VIS_U -# define PYBIND11_NS_VIS_H -#endif - -#define PYBIND11_NS_VIS_FUNC \ - inline std::ptrdiff_t func() { \ - static std::ptrdiff_t value = 0; \ - return reinterpret_cast(&value); \ - } - -#define PYBIND11_NS_VIS_DEFS \ - m.def("ns_vis_uuu_func", pybind11_ns_vis_uuu::func); \ - m.def("ns_vis_uuh_func", pybind11_ns_vis_uuh::func); \ - m.def("ns_vis_uhu_func", pybind11_ns_vis_uhu::func); \ - m.def("ns_vis_uhh_func", pybind11_ns_vis_uhh::func); \ - m.def("ns_vis_huu_func", pybind11_ns_vis_huu::func); \ - m.def("ns_vis_huh_func", pybind11_ns_vis_huh::func); \ - m.def("ns_vis_hhu_func", pybind11_ns_vis_hhu::func); \ - m.def("ns_vis_hhh_func", pybind11_ns_vis_hhh::func); diff --git a/tests/namespace_visibility_1.cpp b/tests/namespace_visibility_1.cpp deleted file mode 100644 index a5c0a43b..00000000 --- a/tests/namespace_visibility_1.cpp +++ /dev/null @@ -1,24 +0,0 @@ -#include "pybind11/pybind11.h" -#include "namespace_visibility.inl" - -// clang-format off -namespace pybind11_ns_vis_uuu PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uuh PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uhu PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uhh PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_huu PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_huh PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_hhu PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_hhh PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -// ^ ^ -// bit used .............. here -// clang-format on - -void namespace_visibility_1s(pybind11::module_ &m); - -PYBIND11_MODULE(namespace_visibility_1, m) { - PYBIND11_NS_VIS_DEFS - - auto sm = m.def_submodule("submodule"); - namespace_visibility_1s(sm); -} diff --git a/tests/namespace_visibility_1s.cpp b/tests/namespace_visibility_1s.cpp deleted file mode 100644 index 810dd169..00000000 --- a/tests/namespace_visibility_1s.cpp +++ /dev/null @@ -1,17 +0,0 @@ -#include "pybind11/pybind11.h" -#include "namespace_visibility.inl" - -// clang-format off -namespace pybind11_ns_vis_uuu PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uuh PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uhu PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uhh PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_huu PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_huh PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_hhu PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_hhh PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -// ^ ^ -// bit used ............. here -// clang-format on - -void namespace_visibility_1s(pybind11::module_ &m) { PYBIND11_NS_VIS_DEFS } diff --git a/tests/namespace_visibility_2.cpp b/tests/namespace_visibility_2.cpp deleted file mode 100644 index 0fb38597..00000000 --- a/tests/namespace_visibility_2.cpp +++ /dev/null @@ -1,17 +0,0 @@ -#include "pybind11/pybind11.h" -#include "namespace_visibility.inl" - -// clang-format off -namespace pybind11_ns_vis_uuu PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uuh PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uhu PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_uhh PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_huu PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_huh PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_hhu PYBIND11_NS_VIS_U { PYBIND11_NS_VIS_FUNC } -namespace pybind11_ns_vis_hhh PYBIND11_NS_VIS_H { PYBIND11_NS_VIS_FUNC } -// ^ ^ -// bit used ............ here -// clang-format on - -PYBIND11_MODULE(namespace_visibility_2, m) { PYBIND11_NS_VIS_DEFS } diff --git a/tests/test_exc_namespace_visibility.py b/tests/test_exc_namespace_visibility.py deleted file mode 100644 index e94271a5..00000000 --- a/tests/test_exc_namespace_visibility.py +++ /dev/null @@ -1,68 +0,0 @@ -# This is not really a unit test, but probing environment/toolchain/platform-specific -# behavior under the exact same conditions as normal tests. -# The results are useful to understanding the effects of, e.g., removing -# `-fvisibility=hidden` or `__attribute__((visibility("hidden")))`, or linking -# extensions statically with the core Python interpreter. - -# NOTE -# ==== -# The "exc_" in "test_exc_namespace_visibility.py" is a workaround, to avoid a -# test_cross_module_exception_translator (test_exceptions.py) failure. This -# test has to be imported (by pytest) before test_exceptions.py; pytest sorts -# lexically. See https://github.com/pybind/pybind11/pull/4054 for more information. -from __future__ import annotations - -import itertools - -import namespace_visibility_1 -import namespace_visibility_2 -import pytest - -# Please take a quick look at namespace_visibility.h first, to see what is being probed. -# -# EXPECTED is for -fvisibility=hidden or equivalent, as recommended in the docs. -EXPECTED_ALL_UNIQUE_POINTERS_OBSERVED = "AAC:AAc:AaC:Aac:aAC:aAc:aaC:aac" -# ^^^ -# namespace_visibility_1 pointer||| -# namespace_visibility_1.submodule pointer|| (identical letters means same pointer) -# namespace_visibility_2 pointer| -# Upper-case: namespace visibility unspecified -# Lower-case: namespace visibility hidden -# This test probes all 2**3 combinations of u/h ** number-of-sub/modules. -# -# Also observed: -# AAA:AAc:AaC:Aac:aAC:aAc:aaC:aac -fvisibility=default Linux -# AAA:AAc:AaA:Aac:aAC:aAc:aaC:aac -fvisibility=default macOS -# AAA:AAa:AaA:Aaa:aAA:aAa:aaA:aaa everything linked statically - - -def test_namespace_visibility(): - modules = ( - namespace_visibility_1, - namespace_visibility_1.submodule, - namespace_visibility_2, - ) - unique_pointer_labels = "ABC" - unique_pointers_observed = [] - # u = visibility unspecified - # h = visibility hidden - for visibility in itertools.product(*([("u", "h")] * len(modules))): - # See functions in namespace_visibility_*.cpp - func = "ns_vis_" + "".join(visibility) + "_func" - ptrs = [] - uq_ptrs_obs = "" - for vis, m in zip(visibility, modules): - ptr = getattr(m, func)() - ptrs.append(ptr) - lbl = unique_pointer_labels[ptrs.index(ptr)] - if vis == "h": - # Encode u/h info as upper/lower case to make the final result - # as compact as possible. - lbl = lbl.lower() - uq_ptrs_obs += lbl - unique_pointers_observed.append(uq_ptrs_obs) - all_unique_pointers_observed = ":".join(unique_pointers_observed) - if all_unique_pointers_observed != EXPECTED_ALL_UNIQUE_POINTERS_OBSERVED: - pytest.skip( - f"UNUSUAL all_unique_pointers_observed: {all_unique_pointers_observed}" - ) From 800e5e1df387f935f76ab68cab4ee5bed868bad6 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 20 Jul 2024 13:55:22 +0700 Subject: [PATCH 12/15] Remove type_caster_odr_guard feature. (The feature will continue to live in the pybind11k repo.) (#5255) This rolls back https://github.com/pybind/pybind11/pull/4022 (including follow-on tweaks in other PRs). --- CMakeLists.txt | 1 - include/pybind11/cast.h | 19 +-- include/pybind11/detail/descr.h | 160 +++--------------- .../pybind11/detail/type_caster_odr_guard.h | 144 ---------------- tests/CMakeLists.txt | 5 - tests/extra_python_package/test_files.py | 1 - tests/test_descr_src_loc.cpp | 141 --------------- tests/test_descr_src_loc.py | 58 ------- tests/test_type_caster_odr_guard_1.cpp | 100 ----------- tests/test_type_caster_odr_guard_1.py | 55 ------ tests/test_type_caster_odr_guard_2.cpp | 66 -------- tests/test_type_caster_odr_guard_2.py | 25 --- 12 files changed, 30 insertions(+), 745 deletions(-) delete mode 100644 include/pybind11/detail/type_caster_odr_guard.h delete mode 100644 tests/test_descr_src_loc.cpp delete mode 100644 tests/test_descr_src_loc.py delete mode 100644 tests/test_type_caster_odr_guard_1.cpp delete mode 100644 tests/test_type_caster_odr_guard_1.py delete mode 100644 tests/test_type_caster_odr_guard_2.cpp delete mode 100644 tests/test_type_caster_odr_guard_2.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 31dcd85c..b82f4570 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -157,7 +157,6 @@ set(PYBIND11_HEADERS include/pybind11/detail/smart_holder_sfinae_hooks_only.h include/pybind11/detail/smart_holder_type_casters.h include/pybind11/detail/type_caster_base.h - include/pybind11/detail/type_caster_odr_guard.h include/pybind11/detail/typeid.h include/pybind11/detail/value_and_holder.h include/pybind11/attr.h diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index e02cb617..5c49e8b2 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -14,7 +14,6 @@ #include "detail/descr.h" #include "detail/smart_holder_sfinae_hooks_only.h" #include "detail/type_caster_base.h" -#include "detail/type_caster_odr_guard.h" #include "detail/typeid.h" #include "pytypes.h" @@ -48,20 +47,8 @@ class type_caster_for_class_ : public type_caster_base {}; template class type_caster : public type_caster_for_class_ {}; -#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) - -template -using make_caster_for_intrinsic = type_caster_odr_guard>; - -#else - template -using make_caster_for_intrinsic = type_caster; - -#endif - -template -using make_caster = make_caster_for_intrinsic>; +using make_caster = type_caster>; template struct type_uses_smart_holder_type_caster { @@ -1179,8 +1166,8 @@ struct return_value_policy_override< }; // Basic python -> C++ casting; throws if casting fails -template -make_caster_for_intrinsic &load_type(make_caster_for_intrinsic &conv, const handle &handle) { +template +type_caster &load_type(type_caster &conv, const handle &handle) { static_assert(!detail::is_pyobject::value, "Internal error: type_caster should only be used for C++ types"); if (!conv.load(handle, true)) { diff --git a/include/pybind11/detail/descr.h b/include/pybind11/detail/descr.h index 5b20c884..7d546311 100644 --- a/include/pybind11/detail/descr.h +++ b/include/pybind11/detail/descr.h @@ -1,6 +1,3 @@ -// Copyright (c) 2022 The Pybind Development Team. -// All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. /* pybind11/detail/descr.h: Helper type for concatenating type signatures at compile time @@ -23,106 +20,21 @@ PYBIND11_NAMESPACE_BEGIN(detail) # define PYBIND11_DESCR_CONSTEXPR const #endif -// struct src_loc below is to support type_caster_odr_guard.h -// (see https://github.com/pybind/pybind11/pull/4022). -// The ODR guard creates ODR violations itself (see WARNING below & in type_caster_odr_guard.h), -// but is currently the only tool available. -// The ODR is useful to know *for sure* what is safe and what is not, but that is only a -// subset of what actually works in practice, in a specific environment. The implementation -// here exploits the gray area (similar to a white hat hacker). -// The dedicated test_type_caster_odr_guard_1, test_type_caster_odr_guard_2 pair of unit tests -// passes reliably on almost all platforms that meet the compiler requirements (C++17, C++20), -// except one (gcc 9.4.0 debug build). -// In the pybind11 unit tests we want to test the ODR guard in as many environments as possible, -// but it is NOT recommended to enable the guard in regular builds, production, or -// debug. The guard is meant to be used similar to a sanitizer, to check for type_caster ODR -// violations in binaries that are otherwise already fully tested and assumed to be healthy. -// -// * MSVC 2017 does not support __builtin_FILE(), __builtin_LINE(). -// * MSVC 193732825 C++17 windows-2020 is failing for unknown reasons. -// * Intel 2021.6.0.20220226 (g++ 9.4 mode) __builtin_LINE() is unreliable -// (line numbers vary between translation units). -#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE) \ - && !defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) && defined(PYBIND11_CPP17) \ - && !defined(__INTEL_COMPILER) \ - && (!defined(_MSC_VER) \ - || (_MSC_VER >= 1920 /* MSVC 2019 or newer */ \ - && (_MSC_FULL_VER < 193732825 || _MSC_FULL_VER > 193732826 \ - || defined(PYBIND11_CPP20)))) -# define PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD -#endif - -#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) - -// Not using std::source_location because: -// 1. "It is unspecified whether the copy/move constructors and the copy/move -// assignment operators of source_location are trivial and/or constexpr." -// (https://en.cppreference.com/w/cpp/utility/source_location). -// 2. A matching no-op stub is needed (below) to avoid code duplication. -struct src_loc { - const char *file; - unsigned line; - - constexpr src_loc(const char *file, unsigned line) : file(file), line(line) {} - - static constexpr src_loc here(const char *file = __builtin_FILE(), - unsigned line = __builtin_LINE()) { - return src_loc(file, line); - } - - constexpr src_loc if_known_or(const src_loc &other) const { - if (file != nullptr) { - return *this; - } - return other; - } -}; - -#else - -// No-op stub, to avoid code duplication, expected to be optimized out completely. -struct src_loc { - constexpr src_loc(const char *, unsigned) {} - - static constexpr src_loc here(const char * = nullptr, unsigned = 0) { - return src_loc(nullptr, 0); - } - - constexpr src_loc if_known_or(const src_loc &) const { return *this; } -}; - -#endif - -#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) -namespace { // WARNING: This creates an ODR violation in the ODR guard itself, - // but we do not have any alternative at the moment. -// The ODR violation here is a difference in constexpr between multiple TUs. -// All definitions have the same data layout, the only difference is the -// text const char* pointee (the pointees are identical in value), -// src_loc const char* file pointee (the pointees are different in value), -// src_loc unsigned line value. -// See also: Comment above; WARNING in type_caster_odr_guard.h -#endif - /* Concatenate type signatures at compile time */ template struct descr { char text[N + 1]{'\0'}; - const src_loc sloc; - explicit constexpr descr(src_loc sloc) : sloc(sloc) {} + constexpr descr() = default; // NOLINTNEXTLINE(google-explicit-constructor) - constexpr descr(char const (&s)[N + 1], src_loc sloc = src_loc::here()) - : descr(s, make_index_sequence(), sloc) {} + constexpr descr(char const (&s)[N + 1]) : descr(s, make_index_sequence()) {} template - constexpr descr(char const (&s)[N + 1], index_sequence, src_loc sloc = src_loc::here()) - : text{s[Is]..., '\0'}, sloc(sloc) {} + constexpr descr(char const (&s)[N + 1], index_sequence) : text{s[Is]..., '\0'} {} template // NOLINTNEXTLINE(google-explicit-constructor) - constexpr descr(src_loc sloc, char c, Chars... cs) - : text{c, static_cast(cs)..., '\0'}, sloc(sloc) {} + constexpr descr(char c, Chars... cs) : text{c, static_cast(cs)..., '\0'} {} static constexpr std::array types() { return {{&typeid(Ts)..., nullptr}}; @@ -135,8 +47,7 @@ constexpr descr plus_impl(const descr &a, index_sequence, index_sequence) { PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(b); - return descr{ - a.sloc.if_known_or(b.sloc), a.text[Is1]..., b.text[Is2]...}; + return {a.text[Is1]..., b.text[Is2]...}; } template @@ -146,33 +57,27 @@ constexpr descr operator+(const descr &a, } template -constexpr descr const_name(char const (&text)[N], src_loc sloc = src_loc::here()) { - return descr(text, sloc); -} -constexpr descr<0> const_name(char const (&)[1], src_loc sloc = src_loc::here()) { - return descr<0>(sloc); +constexpr descr const_name(char const (&text)[N]) { + return descr(text); } +constexpr descr<0> const_name(char const (&)[1]) { return {}; } template struct int_to_str : int_to_str {}; template struct int_to_str<0, Digits...> { // WARNING: This only works with C++17 or higher. - // src_loc not tracked (not needed in this situation, at least at the moment). - static constexpr auto digits - = descr(src_loc{nullptr, 0}, ('0' + Digits)...); + static constexpr auto digits = descr(('0' + Digits)...); }; // Ternary description (like std::conditional) template -constexpr enable_if_t> -const_name(char const (&text1)[N1], char const (&)[N2], src_loc sloc = src_loc::here()) { - return const_name(text1, sloc); +constexpr enable_if_t> const_name(char const (&text1)[N1], char const (&)[N2]) { + return const_name(text1); } template -constexpr enable_if_t> -const_name(char const (&)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) { - return const_name(text2, sloc); +constexpr enable_if_t> const_name(char const (&)[N1], char const (&text2)[N2]) { + return const_name(text2); } template @@ -186,13 +91,12 @@ constexpr enable_if_t const_name(const T1 &, const T2 &d) { template auto constexpr const_name() -> remove_cv_t::digits)> { - // src_loc not tracked (not needed in this situation, at least at the moment). return int_to_str::digits; } template -constexpr descr<1, Type> const_name(src_loc sloc = src_loc::here()) { - return {sloc, '%'}; +constexpr descr<1, Type> const_name() { + return {'%'}; } // If "_" is defined as a macro, py::detail::_ cannot be provided. @@ -202,18 +106,16 @@ constexpr descr<1, Type> const_name(src_loc sloc = src_loc::here()) { #ifndef _ # define PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY template -constexpr descr _(char const (&text)[N], src_loc sloc = src_loc::here()) { - return const_name(text, sloc); +constexpr descr _(char const (&text)[N]) { + return const_name(text); } template -constexpr enable_if_t> -_(char const (&text1)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) { - return const_name(text1, text2, sloc); +constexpr enable_if_t> _(char const (&text1)[N1], char const (&text2)[N2]) { + return const_name(text1, text2); } template -constexpr enable_if_t> -_(char const (&text1)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) { - return const_name(text1, text2, sloc); +constexpr enable_if_t> _(char const (&text1)[N1], char const (&text2)[N2]) { + return const_name(text1, text2); } template constexpr enable_if_t _(const T1 &d1, const T2 &d2) { @@ -226,16 +128,15 @@ constexpr enable_if_t _(const T1 &d1, const T2 &d2) { template auto constexpr _() -> remove_cv_t::digits)> { - // src_loc not tracked (not needed in this situation, at least at the moment). return const_name(); } template -constexpr descr<1, Type> _(src_loc sloc = src_loc::here()) { - return const_name(sloc); +constexpr descr<1, Type> _() { + return const_name(); } #endif // #ifndef _ -constexpr descr<0> concat(src_loc sloc = src_loc::here()) { return descr<0>{sloc}; } +constexpr descr<0> concat() { return {}; } template constexpr descr concat(const descr &descr) { @@ -246,8 +147,7 @@ constexpr descr concat(const descr &descr) { template constexpr descr operator,(const descr &a, const descr &b) { - // Ensure that src_loc of existing descr is used. - return a + const_name(", ", src_loc{nullptr, 0}) + b; + return a + const_name(", ") + b; } template @@ -259,20 +159,14 @@ template constexpr auto concat(const descr &d, const Args &...args) -> decltype(std::declval>() + concat(args...)) { - // Ensure that src_loc of existing descr is used. - return d + const_name(", ", src_loc{nullptr, 0}) + concat(args...); + return d + const_name(", ") + concat(args...); } #endif template constexpr descr type_descr(const descr &descr) { - // Ensure that src_loc of existing descr is used. - return const_name("{", src_loc{nullptr, 0}) + descr + const_name("}"); + return const_name("{") + descr + const_name("}"); } -#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) -} // namespace -#endif - PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/detail/type_caster_odr_guard.h b/include/pybind11/detail/type_caster_odr_guard.h deleted file mode 100644 index 71d074d1..00000000 --- a/include/pybind11/detail/type_caster_odr_guard.h +++ /dev/null @@ -1,144 +0,0 @@ -// Copyright (c) 2022 The Pybind Development Team. -// All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -#pragma once - -#include "descr.h" - -#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) - -# if !defined(PYBIND11_CPP20) && defined(__GNUC__) && !defined(__clang__) -# pragma GCC diagnostic ignored "-Wsubobject-linkage" -# endif - -# include "../pytypes.h" -# include "common.h" -# include "typeid.h" - -# include -# include -# include -# include -# include -# include -# include -# include - -PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) -PYBIND11_NAMESPACE_BEGIN(detail) - -using type_caster_odr_guard_registry_type = std::unordered_map; - -inline type_caster_odr_guard_registry_type &type_caster_odr_guard_registry() { - // Using the no-destructor idiom (maximizes safety). - static auto *reg = new type_caster_odr_guard_registry_type(); - return *reg; -} - -inline unsigned &type_caster_odr_violation_detected_counter() { - static unsigned counter = 0; - return counter; -} - -inline std::string source_file_line_basename(const char *sfl) { - unsigned i_base = 0; - for (unsigned i = 0; sfl[i] != '\0'; i++) { - if (sfl[i] == '/' || sfl[i] == '\\') { - i_base = i + 1; - } - } - return std::string(sfl + i_base); -} - -// This macro is for cooperation with test_type_caster_odr_guard_?.cpp -# ifndef PYBIND11_DETAIL_TYPE_CASTER_ODR_GUARD_THROW_DISABLED -# define PYBIND11_DETAIL_TYPE_CASTER_ODR_GUARD_THROW_DISABLED false -# endif - -inline void type_caster_odr_guard_impl(const std::type_info &intrinsic_type_info, - const src_loc &sloc, - bool throw_disabled) { - std::string source_file_line_from_sloc - = std::string(sloc.file) + ':' + std::to_string(sloc.line); -// This macro is purely for debugging. -# if defined(PYBIND11_DETAIL_TYPE_CASTER_ODR_GUARD_IMPL_DEBUG) - // std::cout cannot be used here: static initialization could be incomplete. - std::fprintf(stdout, - "\nTYPE_CASTER_ODR_GUARD_IMPL %s %s\n", - clean_type_id(intrinsic_type_info.name()).c_str(), - source_file_line_from_sloc.c_str()); - std::fflush(stdout); -# endif - auto ins = type_caster_odr_guard_registry().insert( - {std::type_index(intrinsic_type_info), source_file_line_from_sloc}); - auto reg_iter = ins.first; - auto added = ins.second; - if (!added - && source_file_line_basename(reg_iter->second.c_str()) - != source_file_line_basename(source_file_line_from_sloc.c_str())) { - std::string msg("ODR VIOLATION DETECTED: pybind11::detail::type_caster<" - + clean_type_id(intrinsic_type_info.name()) + ">: SourceLocation1=\"" - + reg_iter->second + "\", SourceLocation2=\"" + source_file_line_from_sloc - + "\""); - if (throw_disabled) { -# if defined(PYBIND11_DETAIL_TYPE_CASTER_ODR_GUARD_IMPL_DEBUG) - std::fprintf(stderr, "\nDISABLED std::system_error: %s\n", msg.c_str()); - std::fflush(stderr); -# endif - type_caster_odr_violation_detected_counter()++; - } else { - throw std::system_error(std::make_error_code(std::errc::state_not_recoverable), msg); - } - } -} - -namespace { // WARNING: This creates an ODR violation in the ODR guard itself, - // but we do not have any alternative at the moment. -// The ODR violation here does not involve any data at all. -// See also: Comment near top of descr.h & WARNING in descr.h - -struct tu_local_no_data_always_false { - explicit operator bool() const noexcept { return false; } -}; - -} // namespace - -template -struct type_caster_odr_guard : TypeCasterType { - static tu_local_no_data_always_false translation_unit_local; - - type_caster_odr_guard() { - // Possibly, good optimizers will elide this `if` (and below) completely. - // It is needed only to trigger the TU-local mechanisms. - if (translation_unit_local) { - } - } - - // The original author of this function is @amauryfa - template - static handle cast(CType &&src, return_value_policy policy, handle parent, Arg &&...arg) { - if (translation_unit_local) { - } - return TypeCasterType::cast( - std::forward(src), policy, parent, std::forward(arg)...); - } -}; - -template -tu_local_no_data_always_false - type_caster_odr_guard::translation_unit_local - = []() { - // Executed only once per process (e.g. when a PYBIND11_MODULE is initialized). - // Conclusively tested vi test_type_caster_odr_guard_1, test_type_caster_odr_guard_2: - // those tests will fail if the sloc here is not working as intended (TU-local). - type_caster_odr_guard_impl(typeid(IntrinsicType), - TypeCasterType::name.sloc, - PYBIND11_DETAIL_TYPE_CASTER_ODR_GUARD_THROW_DISABLED); - return tu_local_no_data_always_false(); - }(); - -PYBIND11_NAMESPACE_END(detail) -PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) - -#endif // PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 39ce97b1..1007d6d8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -176,8 +176,6 @@ set(PYBIND11_TEST_FILES test_stl_binders test_tagbased_polymorphic test_thread - test_type_caster_odr_guard_1 - test_type_caster_odr_guard_2 test_type_caster_pyobject_ptr test_union test_unnamed_namespace_a @@ -492,9 +490,6 @@ foreach(target ${test_targets}) target_compile_options(${target} PRIVATE /utf-8) endif() - target_compile_definitions(${target} - PRIVATE -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE) - if(EIGEN3_FOUND) target_link_libraries(${target} PRIVATE Eigen3::Eigen) target_compile_definitions(${target} PRIVATE -DPYBIND11_TEST_EIGEN) diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index e993e37b..a61b4284 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -63,7 +63,6 @@ "include/pybind11/detail/smart_holder_sfinae_hooks_only.h", "include/pybind11/detail/smart_holder_type_casters.h", "include/pybind11/detail/type_caster_base.h", - "include/pybind11/detail/type_caster_odr_guard.h", "include/pybind11/detail/typeid.h", "include/pybind11/detail/value_and_holder.h", } diff --git a/tests/test_descr_src_loc.cpp b/tests/test_descr_src_loc.cpp deleted file mode 100644 index 8032f318..00000000 --- a/tests/test_descr_src_loc.cpp +++ /dev/null @@ -1,141 +0,0 @@ -// Copyright (c) 2022 The Pybind Development Team. -// All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -#include "pybind11_tests.h" - -// This test actually works with almost all C++17 compilers, but is currently -// only needed (and tested) for type_caster_odr_guard.h, for simplicity. - -#ifndef PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD - -TEST_SUBMODULE(descr_src_loc, m) { m.attr("block_descr_offset") = py::none(); } - -#else - -namespace pybind11_tests { -namespace descr_src_loc { - -using py::detail::const_name; -using py::detail::src_loc; - -struct block_descr { - static constexpr unsigned offset = __LINE__; - static constexpr auto c0 = py::detail::descr<0>(src_loc::here()); - static constexpr auto c1 = py::detail::descr<3>("Abc"); - static constexpr auto c2 = py::detail::descr<1>(src_loc::here(), 'D'); - static constexpr auto c3 = py::detail::descr<2>(src_loc::here(), 'E', 'f'); -}; - -struct block_const_name { - static constexpr unsigned offset = __LINE__; - static constexpr auto c0 = const_name("G"); - static constexpr auto c1 = const_name("Hi"); - static constexpr auto c2 = const_name<0>(); - static constexpr auto c3 = const_name<1>(); - static constexpr auto c4 = const_name<23>(); - static constexpr auto c5 = const_name(); - static constexpr auto c6 = const_name("J", "K"); - static constexpr auto c7 = const_name("L", "M"); -}; - -# if defined(PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY) -struct block_underscore { - static constexpr unsigned offset = __LINE__; - // Using a macro to avoid copying the block_const_name code garbles the src_loc.line numbers. - static constexpr auto c0 = const_name("G"); - static constexpr auto c1 = const_name("Hi"); - static constexpr auto c2 = const_name<0>(); - static constexpr auto c3 = const_name<1>(); - static constexpr auto c4 = const_name<23>(); - static constexpr auto c5 = const_name(); - static constexpr auto c6 = const_name("J", "K"); - static constexpr auto c7 = const_name("L", "M"); -}; -# endif - -struct block_plus { - static constexpr unsigned offset = __LINE__; - static constexpr auto c0 = const_name("N") + // critical line break - const_name("O"); - static constexpr auto c1 = const_name("P", src_loc(nullptr, 0)) + // critical line break - const_name("Q"); -}; - -struct block_concat { - static constexpr unsigned offset = __LINE__; - static constexpr auto c0 = py::detail::concat(const_name("R")); - static constexpr auto c1 = py::detail::concat(const_name("S"), // critical line break - const_name("T")); - static constexpr auto c2 - = py::detail::concat(const_name("U", src_loc(nullptr, 0)), // critical line break - const_name("V")); -}; - -struct block_type_descr { - static constexpr unsigned offset = __LINE__; - static constexpr auto c0 = py::detail::type_descr(const_name("W")); -}; - -struct block_int_to_str { - static constexpr unsigned offset = __LINE__; - static constexpr auto c0 = py::detail::int_to_str<0>::digits; - static constexpr auto c1 = py::detail::int_to_str<4>::digits; - static constexpr auto c2 = py::detail::int_to_str<56>::digits; -}; - -} // namespace descr_src_loc -} // namespace pybind11_tests - -TEST_SUBMODULE(descr_src_loc, m) { - using namespace pybind11_tests::descr_src_loc; - -# define ATTR_OFFS(B) m.attr(#B "_offset") = B::offset; -# define ATTR_BLKC(B, C) \ - m.attr(#B "_" #C) = py::make_tuple(B::C.text, B::C.sloc.file, B::C.sloc.line); - - ATTR_OFFS(block_descr) - ATTR_BLKC(block_descr, c0) - ATTR_BLKC(block_descr, c1) - ATTR_BLKC(block_descr, c2) - ATTR_BLKC(block_descr, c3) - - ATTR_OFFS(block_const_name) - ATTR_BLKC(block_const_name, c0) - ATTR_BLKC(block_const_name, c1) - ATTR_BLKC(block_const_name, c2) - ATTR_BLKC(block_const_name, c3) - ATTR_BLKC(block_const_name, c4) - ATTR_BLKC(block_const_name, c5) - ATTR_BLKC(block_const_name, c6) - ATTR_BLKC(block_const_name, c7) - - ATTR_OFFS(block_underscore) - ATTR_BLKC(block_underscore, c0) - ATTR_BLKC(block_underscore, c1) - ATTR_BLKC(block_underscore, c2) - ATTR_BLKC(block_underscore, c3) - ATTR_BLKC(block_underscore, c4) - ATTR_BLKC(block_underscore, c5) - ATTR_BLKC(block_underscore, c6) - ATTR_BLKC(block_underscore, c7) - - ATTR_OFFS(block_plus) - ATTR_BLKC(block_plus, c0) - ATTR_BLKC(block_plus, c1) - - ATTR_OFFS(block_concat) - ATTR_BLKC(block_concat, c0) - ATTR_BLKC(block_concat, c1) - ATTR_BLKC(block_concat, c2) - - ATTR_OFFS(block_type_descr) - ATTR_BLKC(block_type_descr, c0) - - ATTR_OFFS(block_int_to_str) - ATTR_BLKC(block_int_to_str, c0) - ATTR_BLKC(block_int_to_str, c1) - ATTR_BLKC(block_int_to_str, c2) -} - -#endif // PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD diff --git a/tests/test_descr_src_loc.py b/tests/test_descr_src_loc.py deleted file mode 100644 index cf16f714..00000000 --- a/tests/test_descr_src_loc.py +++ /dev/null @@ -1,58 +0,0 @@ -from __future__ import annotations - -import pytest - -from pybind11_tests import descr_src_loc as m - -if m.block_descr_offset is None: - block_parametrize = (("all_blocks", None),) -else: - block_parametrize = ( - ("block_descr", (("", 1), ("Abc", 2), ("D", 3), ("Ef", 4))), - ( - "block_const_name", - ( - ("G", 1), - ("Hi", 2), - ("0", 0), - ("1", 0), - ("23", 0), - ("%", 6), - ("J", 7), - ("M", 8), - ), - ), - ( - "block_underscore", - ( - ("G", 2), - ("Hi", 3), - ("0", 0), - ("1", 0), - ("23", 0), - ("%", 7), - ("J", 8), - ("M", 9), - ), - ), - ("block_plus", (("NO", 1), ("PQ", 4))), - ("block_concat", (("R", 1), ("S, T", 2), ("U, V", 6))), - ("block_type_descr", (("{W}", 1),)), - ("block_int_to_str", (("", 0), ("4", 0), ("56", 0))), - ) - - -@pytest.mark.skipif(m.block_descr_offset is None, reason="Not enabled.") -@pytest.mark.parametrize(("block_name", "expected_text_line"), block_parametrize) -def test_block(block_name, expected_text_line): - offset = getattr(m, f"{block_name}_offset") - for ix, (expected_text, expected_line) in enumerate(expected_text_line): - text, file, line = getattr(m, f"{block_name}_c{ix}") - assert text == expected_text - if expected_line: - assert file is not None, expected_text_line - assert file.endswith("test_descr_src_loc.cpp") - assert line == offset + expected_line - else: - assert file is None - assert line == 0 diff --git a/tests/test_type_caster_odr_guard_1.cpp b/tests/test_type_caster_odr_guard_1.cpp deleted file mode 100644 index 13a533c8..00000000 --- a/tests/test_type_caster_odr_guard_1.cpp +++ /dev/null @@ -1,100 +0,0 @@ -#define PYBIND11_DETAIL_TYPE_CASTER_ODR_GUARD_THROW_DISABLED true -#include "pybind11_tests.h" - -// For test of real-world issue. -#include "pybind11/stl.h" - -#include - -namespace mrc_ns { // minimal real caster - -struct type_mrc { - explicit type_mrc(int v = -9999) : value(v) {} - int value; -}; - -struct minimal_real_caster { - static constexpr auto name = py::detail::const_name(); - - static py::handle - cast(type_mrc const &src, py::return_value_policy /*policy*/, py::handle /*parent*/) { - return py::int_(src.value + 1010).release(); // ODR violation. - } - - // Maximizing simplicity. This will go terribly wrong for other arg types. - template - using cast_op_type = const type_mrc &; - - // NOLINTNEXTLINE(google-explicit-constructor) - operator type_mrc const &() { - static type_mrc obj; - obj.value = 11; // ODR violation. - return obj; - } - - bool load(py::handle src, bool /*convert*/) { - // Only accepts str, but the value is ignored. - return py::isinstance(src); - } -}; - -// Intentionally not called from Python: this test is to exercise the ODR guard, -// not stl.h or stl_bind.h. -inline void pass_vector_type_mrc(const std::vector &) {} - -} // namespace mrc_ns - -namespace pybind11 { -namespace detail { -template <> -struct type_caster : mrc_ns::minimal_real_caster {}; -} // namespace detail -} // namespace pybind11 - -TEST_SUBMODULE(type_caster_odr_guard_1, m) { - m.def("type_mrc_to_python", []() { return mrc_ns::type_mrc(101); }); - m.def("type_mrc_from_python", [](const mrc_ns::type_mrc &obj) { return obj.value + 100; }); - m.def("type_caster_odr_guard_registry_values", []() { -#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) - py::list values; - for (const auto ®_iter : py::detail::type_caster_odr_guard_registry()) { - values.append(py::str(reg_iter.second)); - } - return values; -#else - return py::none(); -#endif - }); - m.def("type_caster_odr_violation_detected_count", []() { -#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) - return py::detail::type_caster_odr_violation_detected_counter(); -#else - return py::none(); -#endif - }); - - // See comment near the bottom of test_type_caster_odr_guard_2.cpp. - m.def("pass_vector_type_mrc", mrc_ns::pass_vector_type_mrc); - - m.attr("if_defined__NO_INLINE__") = -#if defined(__NO_INLINE__) - true; -#else - false; -#endif - - m.attr("CUDACC") = -#if defined(__CUDACC_VER_MAJOR__) - PYBIND11_TOSTRING(__CUDACC_VER_MAJOR__) "." PYBIND11_TOSTRING( - __CUDACC_VER_MINOR__) "." PYBIND11_TOSTRING(__CUDACC_VER_BUILD__); -#else - py::none(); -#endif - m.attr("NVCOMPILER") = -#if defined(__NVCOMPILER_MAJOR__) - PYBIND11_TOSTRING(__NVCOMPILER_MAJOR__) "." PYBIND11_TOSTRING( - __NVCOMPILER_MINOR__) "." PYBIND11_TOSTRING(__NVCOMPILER_PATCHLEVEL__); -#else - py::none(); -#endif -} diff --git a/tests/test_type_caster_odr_guard_1.py b/tests/test_type_caster_odr_guard_1.py deleted file mode 100644 index 88a89bd0..00000000 --- a/tests/test_type_caster_odr_guard_1.py +++ /dev/null @@ -1,55 +0,0 @@ -from __future__ import annotations - -import pytest - -import pybind11_tests -import pybind11_tests.type_caster_odr_guard_1 as m - - -def test_type_mrc_to_python(): - val = m.type_mrc_to_python() - if val == 101 + 2020: - pytest.skip( - "UNEXPECTED: test_type_caster_odr_guard_2.cpp prevailed (to_python)." - ) - else: - assert val == 101 + 1010 - - -def test_type_mrc_from_python(): - val = m.type_mrc_from_python("ignored") - if val == 100 + 22: - pytest.skip( - "UNEXPECTED: test_type_caster_odr_guard_2.cpp prevailed (from_python)." - ) - else: - assert val == 100 + 11 - - -def test_type_caster_odr_registry_values(): - reg_values = m.type_caster_odr_guard_registry_values() - if reg_values is None: - pytest.skip("type_caster_odr_guard_registry_values() is None") - else: - assert "test_type_caster_odr_guard_" in "\n".join(reg_values) - - -def _count_0_message(tail): - return ( - "type_caster_odr_violation_detected_count() == 0:" - f" {pybind11_tests.compiler_info}, {pybind11_tests.cpp_std}, " + tail - ) - - -def test_type_caster_odr_violation_detected_counter(): - num_violations = m.type_caster_odr_violation_detected_count() - if num_violations is None: - pytest.skip("type_caster_odr_violation_detected_count() is None") - if num_violations == 0: - if m.if_defined__NO_INLINE__: - pytest.skip(_count_0_message("__NO_INLINE__")) - if m.CUDACC is not None: - pytest.skip(_count_0_message(f"CUDACC = {m.CUDACC}")) - if m.NVCOMPILER is not None: - pytest.skip(_count_0_message(f"NVCOMPILER = {m.NVCOMPILER}")) - assert num_violations == 1 diff --git a/tests/test_type_caster_odr_guard_2.cpp b/tests/test_type_caster_odr_guard_2.cpp deleted file mode 100644 index e0d5054d..00000000 --- a/tests/test_type_caster_odr_guard_2.cpp +++ /dev/null @@ -1,66 +0,0 @@ -#define PYBIND11_DETAIL_TYPE_CASTER_ODR_GUARD_THROW_DISABLED true -#include "pybind11_tests.h" - -// For test of real-world issue. -#include "pybind11/stl_bind.h" - -#include - -namespace mrc_ns { // minimal real caster - -struct type_mrc { - explicit type_mrc(int v = -9999) : value(v) {} - int value; -}; - -struct minimal_real_caster { - static constexpr auto name = py::detail::const_name(); - - static py::handle - cast(type_mrc const &src, py::return_value_policy /*policy*/, py::handle /*parent*/) { - return py::int_(src.value + 2020).release(); // ODR violation. - } - - // Maximizing simplicity. This will go terribly wrong for other arg types. - template - using cast_op_type = const type_mrc &; - - // NOLINTNEXTLINE(google-explicit-constructor) - operator type_mrc const &() { - static type_mrc obj; - obj.value = 22; // ODR violation. - return obj; - } - - bool load(py::handle src, bool /*convert*/) { - // Only accepts str, but the value is ignored. - return py::isinstance(src); - } -}; - -// Intentionally not called from Python: this test is to exercise the ODR guard, -// not stl.h or stl_bind.h. -inline void pass_vector_type_mrc(const std::vector &) {} - -} // namespace mrc_ns - -PYBIND11_MAKE_OPAQUE(std::vector); - -namespace pybind11 { -namespace detail { -template <> -struct type_caster : mrc_ns::minimal_real_caster {}; -} // namespace detail -} // namespace pybind11 - -TEST_SUBMODULE(type_caster_odr_guard_2, m) { - m.def("type_mrc_to_python", []() { return mrc_ns::type_mrc(202); }); - m.def("type_mrc_from_python", [](const mrc_ns::type_mrc &obj) { return obj.value + 200; }); - - // Uncomment and run test_type_caster_odr_guard_1.py to verify that the - // test_type_caster_odr_violation_detected_counter subtest fails - // (num_violations 2 instead of 1). - // Unlike the "controlled ODR violation" for the minimal_real_caster, this ODR violation is - // completely unsafe, therefore it cannot portably be exercised with predictable results. - // m.def("pass_vector_type_mrc", mrc_ns::pass_vector_type_mrc); -} diff --git a/tests/test_type_caster_odr_guard_2.py b/tests/test_type_caster_odr_guard_2.py deleted file mode 100644 index 29aeb832..00000000 --- a/tests/test_type_caster_odr_guard_2.py +++ /dev/null @@ -1,25 +0,0 @@ -from __future__ import annotations - -import pytest - -import pybind11_tests.type_caster_odr_guard_2 as m - - -def test_type_mrc_to_python(): - val = m.type_mrc_to_python() - if val == 202 + 2020: - pytest.skip( - "UNEXPECTED: test_type_caster_odr_guard_2.cpp prevailed (to_python)." - ) - else: - assert val == 202 + 1010 - - -def test_type_mrc_from_python(): - val = m.type_mrc_from_python("ignored") - if val == 200 + 22: - pytest.skip( - "UNEXPECTED: test_type_caster_odr_guard_2.cpp prevailed (from_python)." - ) - else: - assert val == 200 + 11 From 51a968c954edd7afadda564a83729aa39852ee3a Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 20 Jul 2024 15:00:11 +0700 Subject: [PATCH 13/15] Import additional test code originally added under PR #5213 (test_class_sh_property_bakein) (#5256) Corresponding to PR #5213 commit 3406be68772fa3fe20db10b9e149b65f2a18ecec Reduced from use cases in the wild: * `test_readonly_char6_member()`: https://github.com/pytorch/pytorch/blob/4410c44ae6fd8eb36f2358ac76f7d988ca7537c5/torch/csrc/cuda/Module.cpp#L961 * `test_readonly_const_char_ptr_member()`: https://github.com/facebookresearch/nle/blob/862a439a84f52abca94d1f744d57061da12c5831/include/permonst.h#L43 --- tests/test_class_sh_property.cpp | 20 ++++++++++++++++++++ tests/test_class_sh_property.py | 10 ++++++++++ 2 files changed, 30 insertions(+) diff --git a/tests/test_class_sh_property.cpp b/tests/test_class_sh_property.cpp index 35615cec..1f692263 100644 --- a/tests/test_class_sh_property.cpp +++ b/tests/test_class_sh_property.cpp @@ -35,6 +35,15 @@ struct Outer { inline void DisownOuter(std::unique_ptr) {} +struct WithCharArrayMember { + WithCharArrayMember() { std::memcpy(char6_member, "Char6", 6); } + char char6_member[6]; +}; + +struct WithConstCharPtrMember { + const char *const_char_ptr_member = "ConstChar*"; +}; + } // namespace test_class_sh_property PYBIND11_TYPE_CASTER_BASE_HOLDER(test_class_sh_property::ClassicField, @@ -45,6 +54,9 @@ PYBIND11_TYPE_CASTER_BASE_HOLDER(test_class_sh_property::ClassicOuter, PYBIND11_SMART_HOLDER_TYPE_CASTERS(test_class_sh_property::Field) PYBIND11_SMART_HOLDER_TYPE_CASTERS(test_class_sh_property::Outer) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(test_class_sh_property::WithCharArrayMember) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(test_class_sh_property::WithConstCharPtrMember) + TEST_SUBMODULE(class_sh_property, m) { using namespace test_class_sh_property; @@ -83,4 +95,12 @@ TEST_SUBMODULE(class_sh_property, m) { .def_readwrite("m_shcp_readwrite", &Outer::m_shcp); m.def("DisownOuter", DisownOuter); + + py::classh(m, "WithCharArrayMember") + .def(py::init<>()) + .def_readonly("char6_member", &WithCharArrayMember::char6_member); + + py::classh(m, "WithConstCharPtrMember") + .def(py::init<>()) + .def_readonly("const_char_ptr_member", &WithConstCharPtrMember::const_char_ptr_member); } diff --git a/tests/test_class_sh_property.py b/tests/test_class_sh_property.py index 9aeef44e..089cf7ca 100644 --- a/tests/test_class_sh_property.py +++ b/tests/test_class_sh_property.py @@ -152,3 +152,13 @@ def test_unique_ptr_field_proxy_poc(m_attr): with pytest.raises(AttributeError): del field_proxy.num assert field_proxy.num == 82 + + +def test_readonly_char6_member(): + obj = m.WithCharArrayMember() + assert obj.char6_member == "Char6" + + +def test_readonly_const_char_ptr_member(): + obj = m.WithConstCharPtrMember() + assert obj.const_char_ptr_member == "ConstChar*" From 664cbd0f3b6ce53a855ec68ad495f73cfe1111f6 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 21 Jul 2024 10:28:14 +0700 Subject: [PATCH 14/15] Add missing `test_descr_src_loc` in CMakeLists.txt (#30137) * Add missing (for a long time) `test_descr_src_loc` to CMakeLists.txt * Fix bug in test_descr_src_loc.cpp: `!defined(PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY)` * Disable `type_caster` ODR guard for CUDACC and NVCOMPILER: the missing `test_descr_src_loc` in tests/CMakeLists.txt masked that `src_loc` is not reliable. * Fix ruff error (not sure why this passed when testing locally). --- include/pybind11/detail/descr.h | 5 ++++- tests/CMakeLists.txt | 1 + tests/test_descr_src_loc.cpp | 6 ++++++ tests/test_descr_src_loc.py | 5 +++++ tests/test_type_caster_odr_guard_1.cpp | 15 --------------- tests/test_type_caster_odr_guard_1.py | 9 ++------- 6 files changed, 18 insertions(+), 23 deletions(-) diff --git a/include/pybind11/detail/descr.h b/include/pybind11/detail/descr.h index 5b20c884..a18754b6 100644 --- a/include/pybind11/detail/descr.h +++ b/include/pybind11/detail/descr.h @@ -42,9 +42,12 @@ PYBIND11_NAMESPACE_BEGIN(detail) // * MSVC 193732825 C++17 windows-2020 is failing for unknown reasons. // * Intel 2021.6.0.20220226 (g++ 9.4 mode) __builtin_LINE() is unreliable // (line numbers vary between translation units). +// * NVIDIA 12.2.140 (CUDACC) src_loc is unreliable for unknown reasons. +// * NVHPC 23.5.0 (NVCOMPILER) src_loc is unreliable for unknown reasons. #if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE) \ && !defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) && defined(PYBIND11_CPP17) \ - && !defined(__INTEL_COMPILER) \ + && !defined(__INTEL_COMPILER) && !defined(__CUDACC_VER_MAJOR__) \ + && !defined(__NVCOMPILER_MAJOR__) \ && (!defined(_MSC_VER) \ || (_MSC_VER >= 1920 /* MSVC 2019 or newer */ \ && (_MSC_FULL_VER < 193732825 || _MSC_FULL_VER > 193732826 \ diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index feafb8a6..d89e5279 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -151,6 +151,7 @@ set(PYBIND11_TEST_FILES test_copy_move test_custom_type_casters test_custom_type_setup + test_descr_src_loc test_docstring_options test_eigen_matrix test_eigen_tensor diff --git a/tests/test_descr_src_loc.cpp b/tests/test_descr_src_loc.cpp index 8032f318..0333f3d5 100644 --- a/tests/test_descr_src_loc.cpp +++ b/tests/test_descr_src_loc.cpp @@ -110,6 +110,11 @@ TEST_SUBMODULE(descr_src_loc, m) { ATTR_BLKC(block_const_name, c6) ATTR_BLKC(block_const_name, c7) + m.attr("defined_PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY") = +# if !defined(PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY) + false; +# else + true; ATTR_OFFS(block_underscore) ATTR_BLKC(block_underscore, c0) ATTR_BLKC(block_underscore, c1) @@ -119,6 +124,7 @@ TEST_SUBMODULE(descr_src_loc, m) { ATTR_BLKC(block_underscore, c5) ATTR_BLKC(block_underscore, c6) ATTR_BLKC(block_underscore, c7) +# endif ATTR_OFFS(block_plus) ATTR_BLKC(block_plus, c0) diff --git a/tests/test_descr_src_loc.py b/tests/test_descr_src_loc.py index cf16f714..dc14fc6f 100644 --- a/tests/test_descr_src_loc.py +++ b/tests/test_descr_src_loc.py @@ -45,6 +45,11 @@ @pytest.mark.skipif(m.block_descr_offset is None, reason="Not enabled.") @pytest.mark.parametrize(("block_name", "expected_text_line"), block_parametrize) def test_block(block_name, expected_text_line): + if ( + block_name == "block_underscore" + and not m.defined_PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY + ): + pytest.skip("!defined(PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY)") offset = getattr(m, f"{block_name}_offset") for ix, (expected_text, expected_line) in enumerate(expected_text_line): text, file, line = getattr(m, f"{block_name}_c{ix}") diff --git a/tests/test_type_caster_odr_guard_1.cpp b/tests/test_type_caster_odr_guard_1.cpp index 13a533c8..6fcb1152 100644 --- a/tests/test_type_caster_odr_guard_1.cpp +++ b/tests/test_type_caster_odr_guard_1.cpp @@ -82,19 +82,4 @@ TEST_SUBMODULE(type_caster_odr_guard_1, m) { #else false; #endif - - m.attr("CUDACC") = -#if defined(__CUDACC_VER_MAJOR__) - PYBIND11_TOSTRING(__CUDACC_VER_MAJOR__) "." PYBIND11_TOSTRING( - __CUDACC_VER_MINOR__) "." PYBIND11_TOSTRING(__CUDACC_VER_BUILD__); -#else - py::none(); -#endif - m.attr("NVCOMPILER") = -#if defined(__NVCOMPILER_MAJOR__) - PYBIND11_TOSTRING(__NVCOMPILER_MAJOR__) "." PYBIND11_TOSTRING( - __NVCOMPILER_MINOR__) "." PYBIND11_TOSTRING(__NVCOMPILER_PATCHLEVEL__); -#else - py::none(); -#endif } diff --git a/tests/test_type_caster_odr_guard_1.py b/tests/test_type_caster_odr_guard_1.py index 88a89bd0..09f5468a 100644 --- a/tests/test_type_caster_odr_guard_1.py +++ b/tests/test_type_caster_odr_guard_1.py @@ -45,11 +45,6 @@ def test_type_caster_odr_violation_detected_counter(): num_violations = m.type_caster_odr_violation_detected_count() if num_violations is None: pytest.skip("type_caster_odr_violation_detected_count() is None") - if num_violations == 0: - if m.if_defined__NO_INLINE__: - pytest.skip(_count_0_message("__NO_INLINE__")) - if m.CUDACC is not None: - pytest.skip(_count_0_message(f"CUDACC = {m.CUDACC}")) - if m.NVCOMPILER is not None: - pytest.skip(_count_0_message(f"NVCOMPILER = {m.NVCOMPILER}")) + if num_violations == 0 and m.if_defined__NO_INLINE__: + pytest.skip(_count_0_message("__NO_INLINE__")) assert num_violations == 1 From ec557ff61230ec17a015be8959487aa6bd8e3e42 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 22 Jul 2024 15:56:42 +0700 Subject: [PATCH 15/15] Backport non-functional test_class_sh_*.cpp changes from PR #5213: 1. To avoid compiler warnings for unused code in the unnamed namespace. 2. To avoid clang-format changes. (#5258) --- tests/test_class_sh_shared_ptr_copy_move.cpp | 46 ++++++++++--------- tests/test_class_sh_trampoline_basic.cpp | 7 +-- ..._class_sh_trampoline_self_life_support.cpp | 8 +++- ...t_class_sh_trampoline_shared_from_this.cpp | 8 +++- ...class_sh_trampoline_shared_ptr_cpp_arg.cpp | 8 +++- tests/test_class_sh_trampoline_unique_ptr.cpp | 16 +++++-- tests/test_class_sh_virtual_py_cpp_mix.cpp | 10 ++-- 7 files changed, 62 insertions(+), 41 deletions(-) diff --git a/tests/test_class_sh_shared_ptr_copy_move.cpp b/tests/test_class_sh_shared_ptr_copy_move.cpp index 1d4ec3cd..ae1afd6c 100644 --- a/tests/test_class_sh_shared_ptr_copy_move.cpp +++ b/tests/test_class_sh_shared_ptr_copy_move.cpp @@ -50,6 +50,7 @@ PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::FooSmHld) namespace pybind11_tests { TEST_SUBMODULE(class_sh_shared_ptr_copy_move, m) { +#if true // Trick to avoid clang-format changes, in support of PR #5213. namespace py = pybind11; py::class_>(m, "FooShPtr") @@ -57,30 +58,30 @@ TEST_SUBMODULE(class_sh_shared_ptr_copy_move, m) { py::classh(m, "FooSmHld").def("get_history", &FooSmHld::get_history); auto outer = py::class_(m, "Outer").def(py::init()); -#define MAKE_PROP(PropTyp) \ - MAKE_PROP_FOO(ShPtr, PropTyp) \ - MAKE_PROP_FOO(SmHld, PropTyp) - -#define MAKE_PROP_FOO(FooTyp, PropTyp) \ - .def_##PropTyp(#FooTyp "_" #PropTyp "_default", &Outer::FooTyp) \ - .def_##PropTyp( \ - #FooTyp "_" #PropTyp "_copy", &Outer::FooTyp, py::return_value_policy::copy) \ - .def_##PropTyp( \ - #FooTyp "_" #PropTyp "_move", &Outer::FooTyp, py::return_value_policy::move) +# define MAKE_PROP(PropTyp) \ + MAKE_PROP_FOO(ShPtr, PropTyp) \ + MAKE_PROP_FOO(SmHld, PropTyp) + +# define MAKE_PROP_FOO(FooTyp, PropTyp) \ + .def_##PropTyp(#FooTyp "_" #PropTyp "_default", &Outer::FooTyp) \ + .def_##PropTyp( \ + #FooTyp "_" #PropTyp "_copy", &Outer::FooTyp, py::return_value_policy::copy) \ + .def_##PropTyp( \ + #FooTyp "_" #PropTyp "_move", &Outer::FooTyp, py::return_value_policy::move) outer MAKE_PROP(readonly) MAKE_PROP(readwrite); -#undef MAKE_PROP_FOO - -#define MAKE_PROP_FOO(FooTyp, PropTyp) \ - .def_##PropTyp(#FooTyp "_property_" #PropTyp "_default", &Outer::FooTyp) \ - .def_property_##PropTyp(#FooTyp "_property_" #PropTyp "_copy", \ - &Outer::get##FooTyp, \ - py::return_value_policy::copy) \ - .def_property_##PropTyp(#FooTyp "_property_" #PropTyp "_move", \ - &Outer::get##FooTyp, \ - py::return_value_policy::move) +# undef MAKE_PROP_FOO + +# define MAKE_PROP_FOO(FooTyp, PropTyp) \ + .def_##PropTyp(#FooTyp "_property_" #PropTyp "_default", &Outer::FooTyp) \ + .def_property_##PropTyp(#FooTyp "_property_" #PropTyp "_copy", \ + &Outer::get##FooTyp, \ + py::return_value_policy::copy) \ + .def_property_##PropTyp(#FooTyp "_property_" #PropTyp "_move", \ + &Outer::get##FooTyp, \ + py::return_value_policy::move) outer MAKE_PROP(readonly); -#undef MAKE_PROP_FOO -#undef MAKE_PROP +# undef MAKE_PROP_FOO +# undef MAKE_PROP m.def("test_ShPtr_copy", []() { auto o = std::make_shared("copy"); @@ -107,6 +108,7 @@ TEST_SUBMODULE(class_sh_shared_ptr_copy_move, m) { l.append(std::move(o)); return l; }); +#endif } } // namespace pybind11_tests diff --git a/tests/test_class_sh_trampoline_basic.cpp b/tests/test_class_sh_trampoline_basic.cpp index e31bcf71..128701d7 100644 --- a/tests/test_class_sh_trampoline_basic.cpp +++ b/tests/test_class_sh_trampoline_basic.cpp @@ -76,11 +76,12 @@ void wrap(py::module_ m, const char *py_class_name) { } // namespace class_sh_trampoline_basic } // namespace pybind11_tests -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_trampoline_basic::Abase<0>) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_trampoline_basic::Abase<1>) +using namespace pybind11_tests::class_sh_trampoline_basic; + +PYBIND11_SMART_HOLDER_TYPE_CASTERS(Abase<0>) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(Abase<1>) TEST_SUBMODULE(class_sh_trampoline_basic, m) { - using namespace pybind11_tests::class_sh_trampoline_basic; wrap<0>(m, "Abase0"); wrap<1>(m, "Abase1"); } diff --git a/tests/test_class_sh_trampoline_self_life_support.cpp b/tests/test_class_sh_trampoline_self_life_support.cpp index c1eb0354..b1b8969a 100644 --- a/tests/test_class_sh_trampoline_self_life_support.cpp +++ b/tests/test_class_sh_trampoline_self_life_support.cpp @@ -10,7 +10,8 @@ #include #include -namespace { +namespace pybind11_tests { +namespace class_sh_trampoline_self_life_support { struct Big5 { // Also known as "rule of five". std::string history; @@ -41,7 +42,10 @@ struct Big5Trampoline : Big5, py::trampoline_self_life_support { using Big5::Big5; }; -} // namespace +} // namespace class_sh_trampoline_self_life_support +} // namespace pybind11_tests + +using namespace pybind11_tests::class_sh_trampoline_self_life_support; PYBIND11_SMART_HOLDER_TYPE_CASTERS(Big5) diff --git a/tests/test_class_sh_trampoline_shared_from_this.cpp b/tests/test_class_sh_trampoline_shared_from_this.cpp index ae9f2746..ebbe94e9 100644 --- a/tests/test_class_sh_trampoline_shared_from_this.cpp +++ b/tests/test_class_sh_trampoline_shared_from_this.cpp @@ -8,7 +8,8 @@ #include #include -namespace { +namespace pybind11_tests { +namespace class_sh_trampoline_shared_from_this { struct Sft : std::enable_shared_from_this { std::string history; @@ -98,7 +99,10 @@ std::shared_ptr make_pure_cpp_sft_shd_ptr(const std::string &history_seed) std::shared_ptr pass_through_shd_ptr(const std::shared_ptr &obj) { return obj; } -} // namespace +} // namespace class_sh_trampoline_shared_from_this +} // namespace pybind11_tests + +using namespace pybind11_tests::class_sh_trampoline_shared_from_this; PYBIND11_SMART_HOLDER_TYPE_CASTERS(Sft) PYBIND11_SMART_HOLDER_TYPE_CASTERS(SftSharedPtrStash) diff --git a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp index 9e44927a..f4de884e 100644 --- a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp +++ b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp @@ -7,7 +7,8 @@ #include -namespace { +namespace pybind11_tests { +namespace class_sh_trampoline_shared_ptr_cpp_arg { // For testing whether a python subclass of a C++ object dies when the // last python reference is lost @@ -51,7 +52,10 @@ struct SpGoAwayTester { std::shared_ptr m_obj; }; -} // namespace +} // namespace class_sh_trampoline_shared_ptr_cpp_arg +} // namespace pybind11_tests + +using namespace pybind11_tests::class_sh_trampoline_shared_ptr_cpp_arg; PYBIND11_SMART_HOLDER_TYPE_CASTERS(SpBase) PYBIND11_SMART_HOLDER_TYPE_CASTERS(SpBaseTester) diff --git a/tests/test_class_sh_trampoline_unique_ptr.cpp b/tests/test_class_sh_trampoline_unique_ptr.cpp index 141a6e8b..c4b34245 100644 --- a/tests/test_class_sh_trampoline_unique_ptr.cpp +++ b/tests/test_class_sh_trampoline_unique_ptr.cpp @@ -8,7 +8,8 @@ #include -namespace { +namespace pybind11_tests { +namespace class_sh_trampoline_unique_ptr { class Class { public: @@ -30,11 +31,13 @@ class Class { std::uint64_t val_ = 0; }; -} // namespace +} // namespace class_sh_trampoline_unique_ptr +} // namespace pybind11_tests -PYBIND11_SMART_HOLDER_TYPE_CASTERS(Class) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_trampoline_unique_ptr::Class) -namespace { +namespace pybind11_tests { +namespace class_sh_trampoline_unique_ptr { class PyClass : public Class, public py::trampoline_self_life_support { public: @@ -45,9 +48,12 @@ class PyClass : public Class, public py::trampoline_self_life_support { int foo() const override { PYBIND11_OVERRIDE_PURE(int, Class, foo); } }; -} // namespace +} // namespace class_sh_trampoline_unique_ptr +} // namespace pybind11_tests TEST_SUBMODULE(class_sh_trampoline_unique_ptr, m) { + using namespace pybind11_tests::class_sh_trampoline_unique_ptr; + py::classh(m, "Class") .def(py::init<>()) .def("set_val", &Class::setVal) diff --git a/tests/test_class_sh_virtual_py_cpp_mix.cpp b/tests/test_class_sh_virtual_py_cpp_mix.cpp index 2fa9990a..467d55aa 100644 --- a/tests/test_class_sh_virtual_py_cpp_mix.cpp +++ b/tests/test_class_sh_virtual_py_cpp_mix.cpp @@ -46,13 +46,13 @@ struct CppDerivedVirtualOverrider : CppDerived, py::trampoline_self_life_support } // namespace class_sh_virtual_py_cpp_mix } // namespace pybind11_tests -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_virtual_py_cpp_mix::Base) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_virtual_py_cpp_mix::CppDerivedPlain) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_virtual_py_cpp_mix::CppDerived) +using namespace pybind11_tests::class_sh_virtual_py_cpp_mix; -TEST_SUBMODULE(class_sh_virtual_py_cpp_mix, m) { - using namespace pybind11_tests::class_sh_virtual_py_cpp_mix; +PYBIND11_SMART_HOLDER_TYPE_CASTERS(Base) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(CppDerivedPlain) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(CppDerived) +TEST_SUBMODULE(class_sh_virtual_py_cpp_mix, m) { py::classh(m, "Base").def(py::init<>()).def("get", &Base::get); py::classh(m, "CppDerivedPlain").def(py::init<>());