From 0b63dd0eb2ad161d27cf17279a4b08f77f1be91f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Apr 2021 13:47:48 -0700 Subject: [PATCH] Adding initial README_smart_holder.rst. (#2936) * Adding initial README_smart_holder.rst. * Adding README_smart_holder.rst to MANIFEST.in and test_files.py. --- MANIFEST.in | 1 + README.rst | 5 + README_smart_holder.rst | 222 +++++++++++++++++++++++ docs/advanced/smart_ptrs.rst | 7 + tests/extra_python_package/test_files.py | 1 + 5 files changed, 236 insertions(+) create mode 100644 README_smart_holder.rst diff --git a/MANIFEST.in b/MANIFEST.in index caafabf3e..fbfd4fc45 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,4 +1,5 @@ prune ubench +include README_smart_holder.rst recursive-include pybind11/include/pybind11 *.h recursive-include pybind11 *.py recursive-include pybind11 py.typed diff --git a/README.rst b/README.rst index 6c2a25510..1ad52700c 100644 --- a/README.rst +++ b/README.rst @@ -13,6 +13,11 @@ .. start +.. Note:: + + This is the pybind11 **smart_holder** branch. Please refer to + ``README_smart_holder.rst`` for branch-specific information. + .. warning:: Combining older versions of pybind11 (< 2.6.0) with Python 3.9.0 will diff --git a/README_smart_holder.rst b/README_smart_holder.rst new file mode 100644 index 000000000..b0babe24c --- /dev/null +++ b/README_smart_holder.rst @@ -0,0 +1,222 @@ +============================== +pybind11 — smart_holder branch +============================== + + +Overview +======== + +- The smart_holder git branch is a strict superset of the master + branch. Everything that works on master is expected to work exactly the same + with the smart_holder branch. + +- **Smart-pointer interoperability** (``std::unique_ptr``, ``std::shared_ptr``) + is implemented as an **add-on**. + +- The add-on also supports + * passing a Python object back to C++ via ``std::unique_ptr``, safely + **disowning** the Python object. + * safely passing `"trampoline" + `_ + objects (objects with C++ virtual function overrides implemented in + Python) via ``std::unique_ptr`` or ``std::shared_ptr`` back to C++: + associated Python objects are automatically kept alive for the lifetime + of the smart-pointer. + +- The smart_holder branch can be used in two modes: + * **Conservative mode**: ``py::class_`` works exactly as on master. + ``py::classh`` uses ``py::smart_holder``. + * **Progressive mode**: ``py::class_`` uses ``py::smart_holder`` + (i.e. ``py::smart_holder`` is the default holder). + + +What is fundamentally different? +-------------------------------- + +- Traditional pybind11 has the concept of "smart-pointer is holder". + Interoperability between smart-pointers is completely missing. For + example, when using ``std::shared_ptr`` as holder, ``return``-ing + a ``std::unique_ptr`` leads to undefined runtime behavior + (`#1138 `_). A + `systematic analysis is here `_. + +- ``py::smart_holder`` has a richer concept in comparison, with well-defined + runtime behavior. The holder "knows" about both ``std::unique_ptr`` and + ``std::shared_ptr`` and how they interoperate. + +- Caveat (#HelpAppreciated): currently the ``smart_holder`` branch does + not have a well-lit path for including interoperability with custom + smart-pointers. It is expected to be a fairly obvious extension of the + ``smart_holder`` implementation, but will depend on the exact specifications + of each custom smart-pointer type (generalizations are very likely possible). + + +What motivated the development of the smart_holder code? +-------------------------------------------------------- + +- Necessity is the mother. The bigger context is the ongoing retooling of + `PyCLIF `_, to use pybind11 underneath + instead of directly targeting the Python C API. Essentially, the smart_holder + branch is porting established PyCLIF functionality into pybind11. + + +Installation +============ + +Currently ``git clone`` is the only option. We do not have released packages. + +.. code-block:: bash + + git clone --branch smart_holder https://github.com/pybind/pybind11.git + +Everything else is exactly identical to using the default (master) branch. + + +Conservative or Progressive mode? +================================= + +It depends. To a first approximation, for a stand-alone, new project, the +progressive mode will be easiest to use. For larger projects or projects +that integrate with third-party pybind11-based projects, the conservative +mode may be more practical, at least initially, although it comes with the +disadvantage of having to use the ``PYBIND11_SMART_HOLDER_TYPE_CASTERS`` macro. + + +Conservative mode +----------------- + +Here is a minimal example for wrapping a C++ type with the `smart_holder` +type as the holder: + +.. code-block:: cpp + + #include + + struct Foo {}; + + PYBIND11_SMART_HOLDER_TYPE_CASTERS(Foo) + + PYBIND11_MODULE(example_bindings, m) { + namespace py = pybind11; + py::classh(m, "Foo"); + } + +There are three small differences compared to traditional pybind11: + +- ``#include `` is used instead of + ``#include ``. + +- ``py::classh`` is used instead of `py::class_`. + +- The ``PYBIND11_SMART_HOLDER_TYPE_CASTERS(Foo)`` macro is needed. + +To the 2nd bullet point, ``py::classh`` is simply a shortcut for +``py::class_``. The shortcut makes it possible to +switch to using ``py::smart_holder`` without messing up the indentation of +existing code. However, when migrating code that uses ``py::class_>``, currently ``std::shared_ptr`` needs to be +removed manually when switching to ``py::classh`` (#HelpAppreciated this +could probably be avoided with a little bit of template metaprogramming). + +To the 3rd bullet point, the macro also needs to appear in other translation +units with pybind11 bindings that involve Python⇄C++ conversions for +`Foo`. This is the biggest inconvenience of the conservative mode. Practially, +at a larger scale it is best to work with a pair of `.h` and `.cpp` files +for the bindings code, with the macros in the `.h` files. + + +Progressive mode +---------------- + +To work in progressive mode: + +- Add ``-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT`` to the compilation commands. + +- Remove any ``std::shared_ptr<...>`` holders from existing ``py::class_`` + instantiations (#HelpAppreciated this could probably be avoided with a little + bit of template metaprogramming). + +- Only if custom smart-pointers are used: the + `PYBIND11_TYPE_CASTER_BASE_HOLDER` macro is needed [`example + `_]. + +Overall this is probably easier to work with than the conservative mode, but + +- the macro inconvenience is shifted from ``py::smart_holder`` to custom + smart-pointers (but probably much more rarely needed). + +- it will not interoperate with other extensions built against master or + stable, or extensions built in conservative mode. + + +Transition from conservative to progressive mode +------------------------------------------------ + +This still has to be tried out more in practice, but in small-scale situations +it may be feasible to switch directly to progressive mode in a break-fix +fashion. In large-scale situations it seems more likely that an incremental +approach is needed, which could mean incrementally converting ``py::class_`` +to ``py::classh`` including addition of the macros, then flip the switch, +and convert ``py::classh`` back to ``py:class_`` combined with removal of the +macros if desired (at that point it will work equivalently either way). It +may be smart to delay the final cleanup step until all third-party projects +of interest have made the switch, because then the code will continue to +work in either mode. + + +Ideas for the long-term +----------------------- + +The macros are clearly an inconvenience in many situations. Highly +speculative: to avoid the need for the macros, a potential approach would +be to combine the traditional implementation (``type_caster_base``) with +the ``smart_holder_type_caster``, but this will probably be very messy and +not great as a long-term solution. The ``type_caster_base`` code is very +complex already. A more maintainable approach long-term could be to work +out and document a smart_holder-based solution for custom smart-pointers +in pybind11 version ``N``, then purge ``type_caster_base`` in version +``N+1``. #HelpAppreciated. + + +GitHub testing of PRs against the smart_holder branch +----------------------------------------------------- + +PRs against the smart_holder branch need to be tested in both +modes (conservative, progressive), with the only difference that +`PYBIND11_USE_SMART_HOLDER_AS_DEFAULT` is defined for progressive mode +testing. Currently this is handled simply by creating a secondary PR with a +one-line change in ``include/pybind11/detail/smart_holder_sfinae_hooks_only.h`` +(as in e.g. `PR #2879 `_). It +will be best to mark the secondary PR as Draft. Often it is convenient to reuse +the same secondary PR for a series of primary PRs, simply by rebasing on a +primary PR as needed: + +.. code-block:: bash + + git checkout -b sh_primary_pr + # Code development ... + git push # Create a PR as usual, selecting smart_holder from the branch pulldown. + git checkout sh_secondary_pr + git rebase -X theirs sh_primary_pr + git diff # To verify that the one-line change in smart_holder_sfinae_hooks_only.h is the only diff. + git push --force-with-lease # This will trigger the GitHub Actions for the progressive mode. + +The second time through this will only take a minute or two. + + +Related links +============= + +* The smart_holder branch addresses issue + `#1138 `_ and + the ten issues enumerated in the `description of PR 2839 + `_. + +* `Description of PR #2672 + `_, from which + the smart_holder branch was created. + +* Small `slide deck + `_ + presented in meeting with pybind11 maintainers on Feb 22, 2021. Slides 5 + and 6 show performance comparisons. diff --git a/docs/advanced/smart_ptrs.rst b/docs/advanced/smart_ptrs.rst index da57748ca..36a970c29 100644 --- a/docs/advanced/smart_ptrs.rst +++ b/docs/advanced/smart_ptrs.rst @@ -1,6 +1,13 @@ Smart pointers ############## +.. Note:: + + This is the pybind11 **smart_holder** branch, but the information + below has NOT been updated accordingly yet. Please refer to + ``README_smart_holder.rst`` under the top-level pybind11 directory + for updated information about smart pointers. + std::unique_ptr =============== diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 7d2cfd814..3c79ac638 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -92,6 +92,7 @@ sdist_files = { "LICENSE", "MANIFEST.in", "README.rst", + "README_smart_holder.rst", "PKG-INFO", }