|
| 1 | +============================== |
| 2 | +pybind11 — smart_holder branch |
| 3 | +============================== |
| 4 | + |
| 5 | + |
| 6 | +Overview |
| 7 | +======== |
| 8 | + |
| 9 | +- The smart_holder git branch is a strict superset of the master |
| 10 | + branch. Everything that works on master is expected to work exactly the same |
| 11 | + with the smart_holder branch. |
| 12 | + |
| 13 | +- **Smart-pointer interoperability** (``std::unique_ptr``, ``std::shared_ptr``) |
| 14 | + is implemented as an **add-on**. |
| 15 | + |
| 16 | +- The add-on also supports |
| 17 | + * passing a Python object back to C++ via ``std::unique_ptr``, safely |
| 18 | + **disowning** the Python object. |
| 19 | + * safely passing `"trampoline" |
| 20 | + <https://pybind11.readthedocs.io/en/stable/advanced/classes.html#overriding-virtual-functions-in-python>`_ |
| 21 | + objects (objects with C++ virtual function overrides implemented in |
| 22 | + Python) via ``std::unique_ptr`` or ``std::shared_ptr`` back to C++: |
| 23 | + associated Python objects are automatically kept alive for the lifetime |
| 24 | + of the smart-pointer. |
| 25 | + |
| 26 | +- The smart_holder branch can be used in two modes: |
| 27 | + * **Conservative mode**: ``py::class_`` works exactly as on master. |
| 28 | + ``py::classh`` uses ``py::smart_holder``. |
| 29 | + * **Progressive mode**: ``py::class_`` uses ``py::smart_holder`` |
| 30 | + (i.e. ``py::smart_holder`` is the default holder). |
| 31 | + |
| 32 | + |
| 33 | +What is fundamentally different? |
| 34 | +-------------------------------- |
| 35 | + |
| 36 | +- Traditional pybind11 has the concept of "smart-pointer is holder". |
| 37 | + Interoperability between smart-pointers is completely missing. For |
| 38 | + example, when using ``std::shared_ptr`` as holder, ``return``-ing |
| 39 | + a ``std::unique_ptr`` leads to undefined runtime behavior |
| 40 | + (`#1138 <https://github.com/pybind/pybind11/issues/1138>`_). A |
| 41 | + `systematic analysis is here <https://github.com/pybind/pybind11/pull/2672#issuecomment-748392993>`_. |
| 42 | + |
| 43 | +- ``py::smart_holder`` has a richer concept in comparison, with well-defined |
| 44 | + runtime behavior. The holder "knows" about both ``std::unique_ptr`` and |
| 45 | + ``std::shared_ptr`` and how they interoperate. |
| 46 | + |
| 47 | +- Caveat (#HelpAppreciated): currently the ``smart_holder`` branch does |
| 48 | + not have a well-lit path for including interoperability with custom |
| 49 | + smart-pointers. It is expected to be a fairly obvious extension of the |
| 50 | + ``smart_holder`` implementation, but will depend on the exact specifications |
| 51 | + of each custom smart-pointer type (generalizations are very likely possible). |
| 52 | + |
| 53 | + |
| 54 | +What motivated the development of the smart_holder code? |
| 55 | +-------------------------------------------------------- |
| 56 | + |
| 57 | +- Necessity is the mother. The bigger context is the ongoing retooling of |
| 58 | + `PyCLIF <https://github.com/google/clif/>`_, to use pybind11 underneath |
| 59 | + instead of directly targeting the Python C API. Essentially, the smart_holder |
| 60 | + branch is porting established PyCLIF functionality into pybind11. |
| 61 | + |
| 62 | + |
| 63 | +Installation |
| 64 | +============ |
| 65 | + |
| 66 | +Currently ``git clone`` is the only option. We do not have released packages. |
| 67 | + |
| 68 | +.. code-block:: bash |
| 69 | +
|
| 70 | + git clone --branch smart_holder https://github.com/pybind/pybind11.git |
| 71 | +
|
| 72 | +Everything else is exactly identical to using the default (master) branch. |
| 73 | + |
| 74 | + |
| 75 | +Conservative or Progressive mode? |
| 76 | +================================= |
| 77 | + |
| 78 | +It depends. To a first approximation, for a stand-alone, new project, the |
| 79 | +progressive mode will be easiest to use. For larger projects or projects |
| 80 | +that integrate with third-party pybind11-based projects, the conservative |
| 81 | +mode may be more practical, at least initially, although it comes with the |
| 82 | +disadvantage of having to use the ``PYBIND11_SMART_HOLDER_TYPE_CASTERS`` macro. |
| 83 | + |
| 84 | + |
| 85 | +Conservative mode |
| 86 | +----------------- |
| 87 | + |
| 88 | +Here is a minimal example for wrapping a C++ type with the `smart_holder` |
| 89 | +type as the holder: |
| 90 | + |
| 91 | +.. code-block:: cpp |
| 92 | +
|
| 93 | + #include <pybind11/smart_holder.h> |
| 94 | +
|
| 95 | + struct Foo {}; |
| 96 | +
|
| 97 | + PYBIND11_SMART_HOLDER_TYPE_CASTERS(Foo) |
| 98 | +
|
| 99 | + PYBIND11_MODULE(example_bindings, m) { |
| 100 | + namespace py = pybind11; |
| 101 | + py::classh<Foo>(m, "Foo"); |
| 102 | + } |
| 103 | +
|
| 104 | +There are three small differences compared to traditional pybind11: |
| 105 | + |
| 106 | +- ``#include <pybind11/smart_holder.h>`` is used instead of |
| 107 | + ``#include <pybind11/pybind11.h>``. |
| 108 | + |
| 109 | +- ``py::classh`` is used instead of `py::class_`. |
| 110 | + |
| 111 | +- The ``PYBIND11_SMART_HOLDER_TYPE_CASTERS(Foo)`` macro is needed. |
| 112 | + |
| 113 | +To the 2nd bullet point, ``py::classh<Foo>`` is simply a shortcut for |
| 114 | +``py::class_<Foo, py::smart_holder>``. The shortcut makes it possible to |
| 115 | +switch to using ``py::smart_holder`` without messing up the indentation of |
| 116 | +existing code. However, when migrating code that uses ``py::class_<Foo, |
| 117 | +std::shared_ptr<Foo>>``, currently ``std::shared_ptr<Foo>`` needs to be |
| 118 | +removed manually when switching to ``py::classh`` (#HelpAppreciated this |
| 119 | +could probably be avoided with a little bit of template metaprogramming). |
| 120 | + |
| 121 | +To the 3rd bullet point, the macro also needs to appear in other translation |
| 122 | +units with pybind11 bindings that involve Python⇄C++ conversions for |
| 123 | +`Foo`. This is the biggest inconvenience of the conservative mode. Practially, |
| 124 | +at a larger scale it is best to work with a pair of `.h` and `.cpp` files |
| 125 | +for the bindings code, with the macros in the `.h` files. |
| 126 | + |
| 127 | + |
| 128 | +Progressive mode |
| 129 | +---------------- |
| 130 | + |
| 131 | +To work in progressive mode: |
| 132 | + |
| 133 | +- Add ``-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT`` to the compilation commands. |
| 134 | + |
| 135 | +- Remove any ``std::shared_ptr<...>`` holders from existing ``py::class_`` |
| 136 | + instantiations (#HelpAppreciated this could probably be avoided with a little |
| 137 | + bit of template metaprogramming). |
| 138 | + |
| 139 | +- Only if custom smart-pointers are used: the |
| 140 | + `PYBIND11_TYPE_CASTER_BASE_HOLDER` macro is needed [`example |
| 141 | + <https://github.com/pybind/pybind11/blob/2f624af1ac8571d603df2d70cb54fc7e2e3a356a/tests/test_multiple_inheritance.cpp#L72>`_]. |
| 142 | + |
| 143 | +Overall this is probably easier to work with than the conservative mode, but |
| 144 | + |
| 145 | +- the macro inconvenience is shifted from ``py::smart_holder`` to custom |
| 146 | + smart-pointers (but probably much more rarely needed). |
| 147 | + |
| 148 | +- it will not interoperate with other extensions built against master or |
| 149 | + stable, or extensions built in conservative mode. |
| 150 | + |
| 151 | + |
| 152 | +Transition from conservative to progressive mode |
| 153 | +------------------------------------------------ |
| 154 | + |
| 155 | +This still has to be tried out more in practice, but in small-scale situations |
| 156 | +it may be feasible to switch directly to progressive mode in a break-fix |
| 157 | +fashion. In large-scale situations it seems more likely that an incremental |
| 158 | +approach is needed, which could mean incrementally converting ``py::class_`` |
| 159 | +to ``py::classh`` including addition of the macros, then flip the switch, |
| 160 | +and convert ``py::classh`` back to ``py:class_`` combined with removal of the |
| 161 | +macros if desired (at that point it will work equivalently either way). It |
| 162 | +may be smart to delay the final cleanup step until all third-party projects |
| 163 | +of interest have made the switch, because then the code will continue to |
| 164 | +work in either mode. |
| 165 | + |
| 166 | + |
| 167 | +Ideas for the long-term |
| 168 | +----------------------- |
| 169 | + |
| 170 | +The macros are clearly an inconvenience in many situations. Highly |
| 171 | +speculative: to avoid the need for the macros, a potential approach would |
| 172 | +be to combine the traditional implementation (``type_caster_base``) with |
| 173 | +the ``smart_holder_type_caster``, but this will probably be very messy and |
| 174 | +not great as a long-term solution. The ``type_caster_base`` code is very |
| 175 | +complex already. A more maintainable approach long-term could be to work |
| 176 | +out and document a smart_holder-based solution for custom smart-pointers |
| 177 | +in pybind11 version ``N``, then purge ``type_caster_base`` in version |
| 178 | +``N+1``. #HelpAppreciated. |
| 179 | + |
| 180 | + |
| 181 | +GitHub testing of PRs against the smart_holder branch |
| 182 | +----------------------------------------------------- |
| 183 | + |
| 184 | +PRs against the smart_holder branch need to be tested in both |
| 185 | +modes (conservative, progressive), with the only difference that |
| 186 | +`PYBIND11_USE_SMART_HOLDER_AS_DEFAULT` is defined for progressive mode |
| 187 | +testing. Currently this is handled simply by creating a secondary PR with a |
| 188 | +one-line change in ``include/pybind11/detail/smart_holder_sfinae_hooks_only.h`` |
| 189 | +(as in e.g. `PR #2879 <https://github.com/pybind/pybind11/pull/2879>`_). It |
| 190 | +will be best to mark the secondary PR as Draft. Often it is convenient to reuse |
| 191 | +the same secondary PR for a series of primary PRs, simply by rebasing on a |
| 192 | +primary PR as needed: |
| 193 | + |
| 194 | +.. code-block:: bash |
| 195 | +
|
| 196 | + git checkout -b sh_primary_pr |
| 197 | + # Code development ... |
| 198 | + git push # Create a PR as usual, selecting smart_holder from the branch pulldown. |
| 199 | + git checkout sh_secondary_pr |
| 200 | + git rebase -X theirs sh_primary_pr |
| 201 | + git diff # To verify that the one-line change in smart_holder_sfinae_hooks_only.h is the only diff. |
| 202 | + git push --force-with-lease # This will trigger the GitHub Actions for the progressive mode. |
| 203 | +
|
| 204 | +The second time through this will only take a minute or two. |
| 205 | + |
| 206 | + |
| 207 | +Related links |
| 208 | +============= |
| 209 | + |
| 210 | +* The smart_holder branch addresses issue |
| 211 | + `#1138 <https://github.com/pybind/pybind11/issues/1138>`_ and |
| 212 | + the ten issues enumerated in the `description of PR 2839 |
| 213 | + <https://github.com/pybind/pybind11/pull/2839#issue-564808678>`_. |
| 214 | + |
| 215 | +* `Description of PR #2672 |
| 216 | + <https://github.com/pybind/pybind11/pull/2672#issue-522688184>`_, from which |
| 217 | + the smart_holder branch was created. |
| 218 | + |
| 219 | +* Small `slide deck |
| 220 | + <https://docs.google.com/presentation/d/1r7auDN0x-b6uf-XCvUnZz6z09raasRcCHBMVDh7PsnQ/>`_ |
| 221 | + presented in meeting with pybind11 maintainers on Feb 22, 2021. Slides 5 |
| 222 | + and 6 show performance comparisons. |
0 commit comments