diff --git a/.travis.yml b/.travis.yml index c9ea008a26..d1ebdb0b2b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -193,9 +193,10 @@ install: ${PYPY:+--extra-index-url https://imaginary.ca/trusty-pypi} echo "done." - wget -q -O eigen.tar.gz https://bitbucket.org/eigen/eigen/get/3.3.3.tar.gz - tar xzf eigen.tar.gz - export CMAKE_INCLUDE_PATH="${CMAKE_INCLUDE_PATH:+$CMAKE_INCLUDE_PATH:}$PWD/eigen-eigen-67e894c6cd8f" + mkdir eigen + curl -fsSL https://bitbucket.org/eigen/eigen/get/3.3.4.tar.bz2 | \ + tar --extract -j --directory=eigen --strip-components=1 + export CMAKE_INCLUDE_PATH="${CMAKE_INCLUDE_PATH:+$CMAKE_INCLUDE_PATH:}$PWD/eigen" fi set +e script: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2beaf8d4d0..375735f6ce 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -30,8 +30,18 @@ adhere to the following rules to make the process as smooth as possible: * This project has a strong focus on providing general solutions using a minimal amount of code, thus small pull requests are greatly preferred. -### License +### Licensing of contributions pybind11 is provided under a BSD-style license that can be found in the ``LICENSE`` file. By using, distributing, or contributing to this project, you agree to the terms and conditions of this license. + +You are under no obligation whatsoever to provide any bug fixes, patches, or +upgrades to the features, functionality or performance of the source code +("Enhancements") to anyone; however, if you choose to make your Enhancements +available either publicly, or directly to the author of this software, without +imposing a separate written license agreement for such Enhancements, then you +hereby grant the following license: a non-exclusive, royalty-free perpetual +license to install, use, modify, prepare derivative works, incorporate into +other computer software, distribute, and sublicense such enhancements or +derivative works thereof, in binary and source code form. diff --git a/LICENSE b/LICENSE index ccf4e97878..6f15578cc4 100644 --- a/LICENSE +++ b/LICENSE @@ -25,12 +25,5 @@ CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -You are under no obligation whatsoever to provide any bug fixes, patches, or -upgrades to the features, functionality or performance of the source code -("Enhancements") to anyone; however, if you choose to make your Enhancements -available either publicly, or directly to the author of this software, without -imposing a separate written license agreement for such Enhancements, then you -hereby grant the following license: a non-exclusive, royalty-free perpetual -license to install, use, modify, prepare derivative works, incorporate into -other computer software, distribute, and sublicense such enhancements or -derivative works thereof, in binary and source code form. +Please also refer to the file CONTRIBUTING.md, which clarifies licensing of +external contributions to this project including patches, pull requests, etc. diff --git a/docs/advanced/cast/overview.rst b/docs/advanced/cast/overview.rst index 2ac7d30097..8e7c12916a 100644 --- a/docs/advanced/cast/overview.rst +++ b/docs/advanced/cast/overview.rst @@ -119,6 +119,9 @@ as arguments and return values, refer to the section on binding :ref:`classes`. | ``std::string_view``, | STL C++17 string views | :file:`pybind11/pybind11.h` | | ``std::u16string_view``, etc. | | | +------------------------------------+---------------------------+-------------------------------+ +| ``std::unique_ptr``, | STL (or custom) smart | :file:`pybind11/cast.h` | +| ``std::shared_ptr``, etc. | pointers. | | ++------------------------------------+---------------------------+-------------------------------+ | ``std::pair`` | Pair of two custom types | :file:`pybind11/pybind11.h` | +------------------------------------+---------------------------+-------------------------------+ | ``std::tuple<...>`` | Arbitrary tuple of types | :file:`pybind11/pybind11.h` | diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index 93deeec621..dba5acda41 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -65,10 +65,10 @@ helper class that is defined as follows: .. code-block:: cpp - class PyAnimal : public Animal { + class PyAnimal : public py::wrapper { public: /* Inherit the constructors */ - using Animal::Animal; + using py::wrapper::wrapper; /* Trampoline (need one for each virtual function) */ std::string go(int n_times) override { @@ -90,6 +90,8 @@ function* slots, which defines the name of function in Python. This is required when the C++ and Python versions of the function have different names, e.g. ``operator()`` vs ``__call__``. +The base class ``py::wrapper<>`` is optional, but is recommended as it allows us to attach the lifetime of Python objects directly to C++ objects, explained in :ref:`virtual_inheritance_lifetime`. + The binding code also needs a few minor adaptations (highlighted): .. code-block:: cpp @@ -157,7 +159,7 @@ Here is an example: class Dachschund(Dog): def __init__(self, name): - Dog.__init__(self) # Without this, undefind behavior may occur if the C++ portions are referenced. + Dog.__init__(self) # Without this, undefined behavior may occur if the C++ portions are referenced. self.name = name def bark(self): return "yap!" @@ -232,15 +234,15 @@ override the ``name()`` method): .. code-block:: cpp - class PyAnimal : public Animal { + class PyAnimal : public py::wrapper { public: - using Animal::Animal; // Inherit constructors + using py::wrapper::wrapper; // Inherit constructors std::string go(int n_times) override { PYBIND11_OVERLOAD_PURE(std::string, Animal, go, n_times); } std::string name() override { PYBIND11_OVERLOAD(std::string, Animal, name, ); } }; - class PyDog : public Dog { + class PyDog : public py::wrapper { public: - using Dog::Dog; // Inherit constructors + using py::wrapper::wrapper; // Inherit constructors std::string go(int n_times) override { PYBIND11_OVERLOAD_PURE(std::string, Dog, go, n_times); } std::string name() override { PYBIND11_OVERLOAD(std::string, Dog, name, ); } std::string bark() override { PYBIND11_OVERLOAD(std::string, Dog, bark, ); } @@ -260,9 +262,9 @@ declare or override any virtual methods itself: .. code-block:: cpp class Husky : public Dog {}; - class PyHusky : public Husky { + class PyHusky : public py::wrapper { public: - using Husky::Husky; // Inherit constructors + using py::wrapper::wrapper; // Inherit constructors std::string go(int n_times) override { PYBIND11_OVERLOAD_PURE(std::string, Husky, go, n_times); } std::string name() override { PYBIND11_OVERLOAD(std::string, Husky, name, ); } std::string bark() override { PYBIND11_OVERLOAD(std::string, Husky, bark, ); } @@ -270,14 +272,14 @@ declare or override any virtual methods itself: There is, however, a technique that can be used to avoid this duplication (which can be especially helpful for a base class with several virtual -methods). The technique involves using template trampoline classes, as +methods). The technique (the Curiously Recurring Template Pattern) involves using template trampoline classes, as follows: .. code-block:: cpp - template class PyAnimal : public AnimalBase { + template class PyAnimal : public py::wrapper { public: - using AnimalBase::AnimalBase; // Inherit constructors + using py::wrapper::wrapper; // Inherit constructors std::string go(int n_times) override { PYBIND11_OVERLOAD_PURE(std::string, AnimalBase, go, n_times); } std::string name() override { PYBIND11_OVERLOAD(std::string, AnimalBase, name, ); } }; @@ -997,5 +999,122 @@ described trampoline: MSVC 2015 has a compiler bug (fixed in version 2017) which requires a more explicit function binding in the form of - ``.def("foo", static_cast(&Publicist::foo));`` - where ``int (A::*)() const`` is the type of ``A::foo``. +.. ``.def("foo", static_cast(&Publicist::foo));`` +.. where ``int (A::*)() const`` is the type of ``A::foo``. + +.. _virtual_inheritance_lifetime: + +Virtual Inheritance and Lifetime +================================ + +When an instance of a Python subclass of a ``pybind11``-bound C++ class is instantiated, there are effectively two "portions": the C++ portion of the base class's alias instance, and the Python portion (``__dict__``) of the derived class instance. +Generally, the lifetime of an instance of a Python subclass of a ``pybind11``-bound C++ class will not pose an issue as long as the instance is owned in Python - that is, you can call virtual methods from C++ or Python and have the correct behavior. + +However, if this Python-constructed instance is passed to C++ such that there are no other Python references, then C++ must keep the Python portion of the instance alive until either (a) the C++ reference is destroyed via ``delete`` or (b) the object is passed back to Python. ``pybind11`` supports both cases, but **only** when (i) the class inherits from :class:`py::wrapper`, (ii) there is only single-inheritance in the bound C++ classes, and (iii) the holder type for the class is either :class:`std::shared_ptr` (suggested) or :class:`std::unique_ptr` (default). + +.. seealso:: + + :ref:`holders` has more information regaring holders and how general ownership transfer should function. + +When ``pybind11`` detects case (a), it will store a reference to the Python object in :class:`py::wrapper` using :class:`py::object`, such that if the instance is deleted by C++, then it will also release the Python object (via :func:`py::wrapper::~wrapper()`). The wrapper will have a unique reference to the Python object (as any other circumstance would trigger case (b)), so the Python object should be destroyed immediately upon the instance's destruction. +This will be a cyclic reference per Python's memory management, but this is not an issue as the memory is now managed via C++. + +For :class:`std::shared_ptr`, this case is detected by placing a shim :func:`__del__` method on the Python subclass when ``pybind11`` detects an instance being created. This shim will check for case (a), and if it holds, will "resurrect" since it created a new reference using :class:`py::object`. + +For :class:`std::unique_ptr`, this case is detected when calling `py::cast>`, which itself implies ownership transfer. + +.. seealso:: + + See :ref:`unique_ptr_ownership` for information about how ownership can be transferred via a cast or argument involving ``unique_ptr``. + +When ``pybind11`` detects case (b) (e.g. ``py::cast()`` is called to convert a C++ instance to `py::object`) and (a) has previously occurred, such that C++ manages the lifetime of the object, then :class:`py::wrapper` will release the Python reference to allow Python to manage the lifetime of the object. + +.. note:: + + This mechanism will be generally robust against reference cycles in Python as this couples the two "portions"; however, it does **not** protect against reference cycles with :class:`std::shared_ptr`. You should take care and use :class:`std::weak_ref` or raw pointers (with care) when needed. + +.. note:: + + There will a slight difference in destructor order if the complete instance is destroyed in C++ or in Python; however, this difference will only be a difference in ordering in when :func:`py::wrapper::~wrapper()` (and your alias destructor) is called in relation to :func:`__del__` for the subclass. For more information, see the documentation comments for :class:`py::wrapper`. + +For this example, we will build upon the above code for ``Animal`` with alias ``PyAnimal``, and the Python subclass ``Cat``, but will introduce a situation where C++ may have sole ownership: a container. In this case, it will be ``Cage``, which can contain or release an animal. + +.. note:: + + For lifetime, it is important to use a more Python-friendly holder, which in this case would be :class:`std::shared_ptr`, permitting an ease to share ownership. + +.. code-block:: cpp + + class Animal { + public: + virtual ~Animal() { } + virtual std::string go(int n_times) = 0; + }; + + class PyAnimal : public py::wrapper { + public: + /* Inherit the constructors */ + using py::wrapper::wrapper; + std::string go(int n_times) override { + PYBIND11_OVERLOAD_PURE(std::string, Animal, go, n_times); + } + }; + + class Cage { + public: + void add(std::shared_ptr animal) { + animal_ = animal; + } + std::shared_ptr release() { + return std::move(animal_); + } + private: + std::shared_ptr animal_; + }; + +And the following bindings: + +.. code-block:: cpp + + PYBIND11_MODULE(example, m) { + py::class_> animal(m, "Animal"); + animal + .def(py::init<>()) + .def("go", &Animal::go); + + py::class_> cage(m, "Cage") + .def(py::init<>()) + .def("add", &Cage::add) + .def("release", &Cage::release); + } + +With the following Python preface: + +.. code-block:: pycon + + >>> from examples import * + >>> class Cat(Animal): + ... def go(self, n_times): + ... return "meow! " * n_times + ... + >>> cage = Cage() + +Normally, if you keep the object alive in Python, then no additional instrumentation is necessary: + +.. code-block:: pycon + + >>> cat = Cat() + >>> c.add(cat) # This object lives in both Python and C++. + >>> c.release().go(2) + meow! meow! + +However, if you pass an instance that Python later wishes to destroy, without :class:`py::wrapper`, we would get an error that ``go`` is not implented, +as the `Cat` portion would have been destroyed and no longer visible for the trampoline. With the wrapper, ``pybind11`` will intercept this event and keep the Python portion alive: + +.. code-block:: pycon + + >>> c.add(Cat()) + >>> c.release().go(2) + meow! meow! + +Note that both the C++ and Python portion of ``cat`` will be destroyed once ``cage`` is destroyed. diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 87481ba32f..5faf11f0d2 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -216,6 +216,21 @@ avoids this issue involves weak reference with a cleanup callback: // Create a weak reference with a cleanup callback and initially leak it (void) py::weakref(m.attr("BaseClass"), cleanup_callback).release(); +.. note:: + + PyPy (at least version 5.9) does not garbage collect objects when the + interpreter exits. An alternative approach (which also works on CPython) is to use + the :py:mod:`atexit` module [#f7]_, for example: + + .. code-block:: cpp + + auto atexit = py::module::import("atexit"); + atexit.attr("register")(py::cpp_function([]() { + // perform cleanup here -- this function is called with the GIL held + })); + + .. [#f7] https://docs.python.org/3/library/atexit.html + Generating documentation using Sphinx ===================================== diff --git a/docs/advanced/smart_ptrs.rst b/docs/advanced/smart_ptrs.rst index da57748ca5..4ef707e55a 100644 --- a/docs/advanced/smart_ptrs.rst +++ b/docs/advanced/smart_ptrs.rst @@ -1,5 +1,17 @@ -Smart pointers -############## +.. _holders: + +Smart pointers and holders +########################## + +Holders +======= + +The binding generator for classes, :class:`class_`, can be passed a template +type that denotes a special *holder* type that is used to manage references to +the object. If no such holder type template argument is given, the default for +a type named ``Type`` is ``std::unique_ptr``, which means that the object +is deallocated when Python's reference count goes to zero. It is possible to switch to other types of reference counting wrappers or smart +pointers, which is useful in codebases that rely on them, such as ``std::shared_ptr``, or even a custom type. std::unique_ptr =============== @@ -15,31 +27,100 @@ instances wrapped in C++11 unique pointers, like so m.def("create_example", &create_example); -In other words, there is nothing special that needs to be done. While returning -unique pointers in this way is allowed, it is *illegal* to use them as function -arguments. For instance, the following function signature cannot be processed -by pybind11. +In other words, there is nothing special that needs to be done. + +.. _unique_ptr_ownership: + +Transferring ownership +---------------------- + +It is also possible to pass ``std::unique_ptr`` as a function +argument, or use ``py::cast>(std::move(obj))``. Note that this tells pybind11 to not manage the memory for this object, and delegate that to ``std::unique_ptr``. +For instance, the following function signature can be processed by pybind11: .. code-block:: cpp void do_something_with_example(std::unique_ptr ex) { ... } -The above signature would imply that Python needs to give up ownership of an -object that is passed to this function, which is generally not possible (for -instance, the object might be referenced elsewhere). +The above signature does imply that Python needs to give up ownership of an +object that is passed to this function. There are two ways to do this: + +1. Simply pass the object in. The reference count of the object can be greater than one (non-unique) when passing the object in. **However**, you *must* ensure that the object has only **one** reference when C++ (which owns the C++ object). + + To expand on this, when transferring ownership for ``std::unique_ptr``, this means that Pybind11 no longer owns the reference, which means that if C++ lets the ``std::unique_ptr`` destruct but if there is a dangling reference in Python, then you will encounter undefined behavior. + + Examples situations: + + * The C++ function is terminal (i.e. will destroy the object once it completes). This is generally not an issue unless a Python portion of the object has a non-trivial ``__del__`` method. + * The Python object is passed to a C++ container which only tracks ``std::unique_ptr``. If the container goes out of scope, the Python object reference will be invalid. + + .. note:: + + For polymorphic types that inherit from :class:`py::wrapper`, ``pybind11`` *can* warn about these situations. + You may enable this behavior with ``#define PYBIND11_WARN_DANGLING_UNIQUE_PYREF``. This will print a warning to ``std::err`` if this case is detected. + +2. Pass a Python "move container" (a mutable object that can "release" the reference to the object). This can be a single-item list, or any Python class / instance that has the field ``_is_move_container = True`` and has a ``release()`` function. + + .. note:: + + When using a move container, this expects that the provided object is a **unique** reference, or will throw an error otherwise. This is a little more verbose, but will make debugging *much* easier. + + As an example in C++: + + .. code-block:: cpp + + void terminal_func(std::unique_ptr obj) { + obj.do_something(); // `obj` will be destroyed when the function exits. + } + + // Binding + py::class_ example(m, "Example"); + m.def("terminal_func", &terminal_func); + + In Python, say you would normally do this: + + .. code-block:: pycon + + >>> obj = Example() + >>> terminal_func(obj) + + As mentioned in the comment, you *must* ensure that `obj` is not used past this invocation, as the underlying data has been destroyed. To be more careful, you may "move" the object. The following will throw an error: + + .. code-block:: pycon + + >>> obj = Example() + >>> terminal_func([obj]) + + However, this will work, using a "move" container: + + .. code-block:: pycon + + >>> obj = Example() + >>> obj_move = [obj] + >>> del obj + >>> terminal_func(obj_move) + >>> print(obj_move) # Reference will have been removed. + [None] + + or even: + + .. code-block:: pycon + + >>> terminal_func([Example()]) + + .. note:: + + ``terminal_func(Example())`` also works, but still leaves a dangling reference, which is only a problem if it is polymorphic and has a non-trivial ``__del__`` method. + + .. warning:: + + This reference counting mechanism is **not** robust aganist cyclic references. If you need some sort of cyclic reference, *please* consider using ``weakref.ref`` in Python. std::shared_ptr =============== -The binding generator for classes, :class:`class_`, can be passed a template -type that denotes a special *holder* type that is used to manage references to -the object. If no such holder type template argument is given, the default for -a type named ``Type`` is ``std::unique_ptr``, which means that the object -is deallocated when Python's reference count goes to zero. - -It is possible to switch to other types of reference counting wrappers or smart -pointers, which is useful in codebases that rely on them. For instance, the -following snippet causes ``std::shared_ptr`` to be used instead. +If you have an existing code base with ``std::shared_ptr``, or you wish to enable reference counting in C++ as well, then you may use this type as a holder. +As an example, the following snippet causes ``std::shared_ptr`` to be used instead. .. code-block:: cpp @@ -111,6 +192,12 @@ There are two ways to resolve this issue: class Child : public std::enable_shared_from_this { }; +.. seealso:: + + While ownership transfer is generally not an issue with ``std::shared_ptr``, it becomes an issue when an instance of a Python subclass of a pybind11 class is effectively managed by C++ (e.g. all live references to the object are from C++, and all reference in Python have "died"). + + See :ref:`virtual_inheritance_lifetime` for more information. + .. _smart_pointers: Custom smart pointers @@ -147,7 +234,7 @@ Please take a look at the :ref:`macro_notes` before using this feature. By default, pybind11 assumes that your custom smart pointer has a standard interface, i.e. provides a ``.get()`` member function to access the underlying -raw pointer. If this is not the case, pybind11's ``holder_helper`` must be +raw pointer, and a ``.release()`` member function for move-only holders. If this is not the case, pybind11's ``holder_helper`` must be specialized: .. code-block:: cpp @@ -171,3 +258,20 @@ provides ``.get()`` functionality via ``.getPointer()``. The file :file:`tests/test_smart_ptr.cpp` contains a complete example that demonstrates how to work with custom reference-counting holder types in more detail. + +.. warning:: + + Holder type conversion (see :ref:`smart_ptrs_casting`) and advanced ownership transfer (see :ref:`virtual_inheritance_lifetime`) is **not** supported for custom shared pointer types, due to constraints on dynamic type erasure. + +.. _smart_ptrs_casting: + +Casting smart pointers +====================== + +As shown in the :ref:`conversion_table`, you may cast to any of the available holders (e.g. ``py::cast>(obj)``) that can properly provide access to the underlying holder. + +``pybind11`` will raise an error if there is an incompatible cast. You may of course cast to the exact same holder type. You may also move a ``std::unique_ptr`` into a ``std::shared_ptr``, as this is allowed. **However**, you may not convert a ``std::shared_ptr`` to a ``std::unique_ptr`` as you cannot release an object that is managed by ``std::shared_ptr``. + +Additionally, conversion to ``std::unique_ptr`` is not supported if ``Deleter`` is not ``std::default_deleter``. + +Conversion to a different custom smart pointer is not supported. diff --git a/docs/changelog.rst b/docs/changelog.rst index b0d958d491..ec8dc72039 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,6 +18,9 @@ v2.3.0 (Not yet released) * Added support for write only properties. `#1144 `_. +* The ``value()`` method of ``py::enum_`` now accepts an optional docstring + that will be shown in the documentation of the associated enumeration. + v2.2.1 (September 14, 2017) ----------------------------------------------------- diff --git a/docs/faq.rst b/docs/faq.rst index d44a272bb4..bfe8303661 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -241,3 +241,19 @@ Common gotchas to watch out for involve not ``free()``-ing memory region that that were ``malloc()``-ed in another shared library, using data structures with incompatible ABIs, and so on. pybind11 is very careful not to make these types of mistakes. + +How to cite this project? +========================= + +We suggest the following BibTeX template to cite pybind11 in scientific +discourse: + +.. code-block:: bash + + @misc{pybind11, + author = {Wenzel Jakob and Jason Rhinelander and Dean Moldovan}, + year = {2017}, + note = {https://github.com/pybind/pybind11}, + title = {pybind11 -- Seamless operability between C++11 and Python} + } + diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index dce875a6b9..9c5ab8a9c4 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -221,11 +221,14 @@ struct type_record { void *(*operator_new)(size_t) = ::operator new; /// Function pointer to class_<..>::init_instance - void (*init_instance)(instance *, const void *) = nullptr; + void (*init_instance)(instance *, holder_erased) = nullptr; /// Function pointer to class_<..>::dealloc void (*dealloc)(detail::value_and_holder &) = nullptr; + /// See `type_info::has_cpp_release`. + instance::type_release_info_t release_info; + /// List of base classes of the newly created type list bases; diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 715ec932d9..76e5d53631 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -156,12 +156,16 @@ inline const std::vector &all_type_info(PyTypeObject *type) * ancestors are pybind11-registered. Throws an exception if there are multiple bases--use * `all_type_info` instead if you want to support multiple bases. */ -PYBIND11_NOINLINE inline detail::type_info* get_type_info(PyTypeObject *type) { +PYBIND11_NOINLINE inline detail::type_info* get_type_info(PyTypeObject *type, bool do_throw = true) { auto &bases = all_type_info(type); if (bases.size() == 0) return nullptr; - if (bases.size() > 1) - pybind11_fail("pybind11::detail::get_type_info: type has multiple pybind11-registered bases"); + if (bases.size() > 1) { + if (do_throw) + pybind11_fail("pybind11::detail::get_type_info: type has multiple pybind11-registered bases"); + else + return nullptr; + } return bases.front(); } @@ -229,6 +233,7 @@ struct value_and_holder { template H &holder() const { return reinterpret_cast(vh[1]); } + void* holder_ptr() const { return &vh[1]; } bool holder_constructed() const { return inst->simple_layout ? inst->simple_holder_constructed @@ -479,6 +484,75 @@ inline PyThreadState *get_thread_state_unchecked() { inline void keep_alive_impl(handle nurse, handle patient); inline PyObject *make_new_instance(PyTypeObject *type); +enum class LoadType { + PureCpp, + DerivedCppSinglePySingle, + DerivedCppSinglePyMulti, + DerivedCppMulti, + /// Polymorphic casting or copy-based casting may be necessary. + ConversionNeeded, +}; + +typedef type_info* base_ptr_t; +typedef const std::vector bases_t; + +inline LoadType determine_load_type(handle src, const type_info* typeinfo, + const bases_t** out_bases = nullptr, + base_ptr_t* out_base = nullptr) { + // Null out inputs. + if (out_bases) + *out_bases = nullptr; + if (out_base) + *out_base = nullptr; + PyTypeObject *srctype = Py_TYPE(src.ptr()); + // See `type_caster_generic::load_impl` below for more detail on comments. + + // Case 1: If src is an exact type match for the target type then we can reinterpret_cast + // the instance's value pointer to the target type: + if (srctype == typeinfo->type) { + // TODO(eric.cousineau): Determine if the type is upcast from a type, which is + // still a pure C++ object? + return LoadType::PureCpp; + } + // Case 2: We have a derived class + else if (PyType_IsSubtype(srctype, typeinfo->type)) { + const bases_t& bases = all_type_info(srctype); + if (out_bases) + *out_bases = &bases; // Copy to output for caching. + const bool no_cpp_mi = typeinfo->simple_type; + // Case 2a: the python type is a Python-inherited derived class that inherits from just + // one simple (no MI) pybind11 class, or is an exact match, so the C++ instance is of + // the right type and we can use reinterpret_cast. + // (This is essentially the same as case 2b, but because not using multiple inheritance + // is extremely common, we handle it specially to avoid the loop iterator and type + // pointer lookup overhead) + // TODO(eric.cousineau): This seems to also capture C++-registered classes as well, not just Python-derived + // classes. + if (bases.size() == 1 && (no_cpp_mi || bases.front()->type == typeinfo->type)) { + return LoadType::DerivedCppSinglePySingle; + } + // Case 2b: the python type inherits from multiple C++ bases. Check the bases to see if + // we can find an exact match (or, for a simple C++ type, an inherited match); if so, we + // can safely reinterpret_cast to the relevant pointer. + else if (bases.size() > 1) { + for (auto base : bases) { + if (no_cpp_mi ? PyType_IsSubtype(base->type, typeinfo->type) : base->type == typeinfo->type) { + if (out_base) { + *out_base = base; + } + return LoadType::DerivedCppSinglePyMulti; + } + } + } + // Case 2c: C++ multiple inheritance is involved and we couldn't find an exact type match + // in the registered bases, above, so try implicit casting (needed for proper C++ casting + // when MI is involved). + return LoadType::DerivedCppMulti; + } else { + return LoadType::ConversionNeeded; + } +} + class type_caster_generic { public: PYBIND11_NOINLINE type_caster_generic(const std::type_info &type_info) @@ -495,7 +569,7 @@ class type_caster_generic { const detail::type_info *tinfo, void *(*copy_constructor)(const void *), void *(*move_constructor)(const void *), - const void *existing_holder = nullptr) { + holder_erased existing_holder = {}) { if (!tinfo) // no type info: error will be set already return handle(); @@ -503,11 +577,64 @@ class type_caster_generic { if (src == nullptr) return none().release(); + const bool take_ownership = policy == return_value_policy::automatic || policy == return_value_policy::take_ownership; + // We only come across `!existing_holder` if we are coming from `cast` and not `cast_holder`. + const bool is_bare_ptr = !existing_holder.ptr() && existing_holder.type_id() == HolderTypeId::Unknown; + auto it_instances = get_internals().registered_instances.equal_range(src); for (auto it_i = it_instances.first; it_i != it_instances.second; ++it_i) { for (auto instance_type : detail::all_type_info(Py_TYPE(it_i->second))) { - if (instance_type && same_type(*instance_type->cpptype, *tinfo->cpptype)) - return handle((PyObject *) it_i->second).inc_ref(); + if (instance_type && same_type(*instance_type->cpptype, *tinfo->cpptype)) { + instance* const inst = it_i->second; + + bool try_to_reclaim = false; + if (!is_bare_ptr) { + switch (instance_type->release_info.holder_type_id) { + case detail::HolderTypeId::UniquePtr: { + try_to_reclaim = take_ownership; + break; + } + case detail::HolderTypeId::SharedPtr: { + if (take_ownership) { + // Only try to reclaim the object if (a) it is not owned and (b) has no holder. + if (!inst->simple_holder_constructed) { + if (inst->owned) + throw std::runtime_error("Internal error?"); + try_to_reclaim = true; + } + } + break; + } + default: { + // Otherwise, do not try any reclaiming. + break; + } + } + } + if (try_to_reclaim) { + // If this object has already been registered, but we wish to take ownership of it, + // then use the `has_cpp_release` mechanisms to reclaim ownership. + // @note This should be the sole occurrence of this registered object when releasing back. + // @note This code path should not be invoked for pure C++ + + // TODO(eric.cousineau): This may be still be desirable if this is a raw pointer... + // Need to think of a desirable workflow - and if there is possible interop. + if (!existing_holder) { + throw std::runtime_error("Internal error: Should have non-null holder."); + } + // TODO(eric.cousineau): This field may not be necessary if the lowest-level type is valid. + // See `move_only_holder_caster::load_value`. + if (!inst->reclaim_from_cpp) { + throw std::runtime_error("Instance is registered but does not have a registered reclaim method. Internal error?"); + } + return inst->reclaim_from_cpp(inst, existing_holder).release(); + } else { + // TODO(eric.cousineau): Should really check that ownership is consistent. + // e.g. if we say to take ownership of a pointer that is passed, does not have a holder... + // In the end, pybind11 would let ownership slip, and leak memory, possibly violating RAII (if someone is using that...) + return handle((PyObject *) it_i->second).inc_ref(); + } + } } } @@ -559,13 +686,13 @@ class type_caster_generic { throw cast_error("unhandled return_value_policy: should not happen!"); } + // TODO(eric.cousineau): Propagate `holder_erased` through this chain. tinfo->init_instance(wrapper, existing_holder); - return inst.release(); } // Base methods for generic caster; there are overridden in copyable_holder_caster - void load_value(value_and_holder &&v_h) { + void load_value(value_and_holder &&v_h, LoadType) { auto *&vptr = v_h.value_ptr(); // Lazy allocation for unallocated values: if (vptr == nullptr) { @@ -638,49 +765,35 @@ class type_caster_generic { auto &this_ = static_cast(*this); this_.check_holder_compat(); - PyTypeObject *srctype = Py_TYPE(src.ptr()); - - // Case 1: If src is an exact type match for the target type then we can reinterpret_cast - // the instance's value pointer to the target type: - if (srctype == typeinfo->type) { - this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder()); - return true; - } - // Case 2: We have a derived class - else if (PyType_IsSubtype(srctype, typeinfo->type)) { - auto &bases = all_type_info(srctype); - bool no_cpp_mi = typeinfo->simple_type; - - // Case 2a: the python type is a Python-inherited derived class that inherits from just - // one simple (no MI) pybind11 class, or is an exact match, so the C++ instance is of - // the right type and we can use reinterpret_cast. - // (This is essentially the same as case 2b, but because not using multiple inheritance - // is extremely common, we handle it specially to avoid the loop iterator and type - // pointer lookup overhead) - if (bases.size() == 1 && (no_cpp_mi || bases.front()->type == typeinfo->type)) { - this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder()); + const bases_t* bases = nullptr; + base_ptr_t base_py_multi = nullptr; + LoadType load_type = determine_load_type(src, typeinfo, &bases, &base_py_multi); + switch (load_type) { + case LoadType::PureCpp: { + this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder(), + load_type); return true; } - // Case 2b: the python type inherits from multiple C++ bases. Check the bases to see if - // we can find an exact match (or, for a simple C++ type, an inherited match); if so, we - // can safely reinterpret_cast to the relevant pointer. - else if (bases.size() > 1) { - for (auto base : bases) { - if (no_cpp_mi ? PyType_IsSubtype(base->type, typeinfo->type) : base->type == typeinfo->type) { - this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder(base)); - return true; - } - } + case LoadType::DerivedCppSinglePySingle: { + this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder(), + load_type); + return true; } - - // Case 2c: C++ multiple inheritance is involved and we couldn't find an exact type match - // in the registered bases, above, so try implicit casting (needed for proper C++ casting - // when MI is involved). - if (this_.try_implicit_casts(src, convert)) + case LoadType::DerivedCppSinglePyMulti: { + this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder(base_py_multi), + load_type); return true; + } + case LoadType::DerivedCppMulti: { + if (this_.try_implicit_casts(src, convert)) + return true; + } + case LoadType::ConversionNeeded: { + break; + } } - // Perform an implicit conversion + // If nothing else succeeds, perform an implicit conversion if (convert) { for (auto &converter : typeinfo->implicit_conversions) { auto temp = reinterpret_steal(converter(src.ptr(), typeinfo->type)); @@ -828,8 +941,11 @@ template class type_caster_base : public type_caster_generic { make_copy_constructor(src), make_move_constructor(src)); } - static handle cast_holder(const itype *src, const void *holder) { + static handle cast_holder(const itype *src, holder_erased holder) { auto st = src_and_type(src); + if (!holder) { + throw std::runtime_error("Internal error: Should not have null holder"); + } return type_caster_generic::cast( st.first, return_value_policy::take_ownership, {}, st.second, nullptr, nullptr, holder); @@ -980,11 +1096,12 @@ struct type_caster::value && !is_std_char_t static handle cast(T src, return_value_policy /* policy */, handle /* parent */) { if (std::is_floating_point::value) { return PyFloat_FromDouble((double) src); - } else if (sizeof(T) <= sizeof(long)) { + } else if (sizeof(T) <= sizeof(ssize_t)) { + // This returns a long automatically if needed if (std::is_signed::value) - return PyLong_FromLong((long) src); + return PYBIND11_LONG_FROM_SIGNED(src); else - return PyLong_FromUnsignedLong((unsigned long) src); + return PYBIND11_LONG_FROM_UNSIGNED(src); } else { if (std::is_signed::value) return PyLong_FromLongLong((long long) src); @@ -1372,6 +1489,12 @@ struct holder_helper { static auto get(const T &p) -> decltype(p.get()) { return p.get(); } }; +inline const detail::type_info* get_lowest_type(handle src, bool do_throw = true) { + auto* py_type = (PyTypeObject*)src.get_type().ptr(); + return detail::get_type_info(py_type, do_throw); +} + + /// Type caster for holder types like std::shared_ptr, etc. template struct copyable_holder_caster : public type_caster_base { @@ -1384,7 +1507,10 @@ struct copyable_holder_caster : public type_caster_base { using base::typeinfo; using base::value; - bool load(handle src, bool convert) { + handle src; + + bool load(handle src_in, bool convert) { + src = src_in; return base::template load_impl>(src, convert); } @@ -1400,11 +1526,20 @@ struct copyable_holder_caster : public type_caster_base { explicit operator holder_type&() { return holder; } #endif + // Risk increasing the `shared_ptr` ref count temporarily to maintain writeable + // semantics without too much `const_cast<>` ooginess. + static handle cast(holder_type &&src, return_value_policy, handle) { + const auto *ptr = holder_helper::get(src); + return type_caster_base::cast_holder(ptr, holder_erased(&src)); + } + static handle cast(const holder_type &src, return_value_policy, handle) { const auto *ptr = holder_helper::get(src); - return type_caster_base::cast_holder(ptr, &src); + return type_caster_base::cast_holder(ptr, holder_erased(&src)); } + // TODO(eric.cousineau): Define cast_op_type??? + protected: friend class type_caster_generic; void check_holder_compat() { @@ -1412,7 +1547,27 @@ struct copyable_holder_caster : public type_caster_base { throw cast_error("Unable to load a custom holder type from a default-holder instance"); } - bool load_value(value_and_holder &&v_h) { + bool load_value(value_and_holder &&v_h, LoadType load_type) { + bool do_release_to_cpp = false; + const type_info* lowest_type = nullptr; + if (src.ref_count() == 1 && load_type == LoadType::DerivedCppSinglePySingle) { + // Go ahead and release ownership to C++, if able. + auto* py_type = (PyTypeObject*)src.get_type().ptr(); + lowest_type = detail::get_type_info(py_type); + // Double-check that we did not get along C++ inheritance. + // TODO(eric.cousineau): Generalize this to unique_ptr<> case as well. + const bool is_actually_pure_cpp = lowest_type->type == py_type; + if (!is_actually_pure_cpp) { + if (lowest_type->release_info.can_derive_from_wrapper) { + do_release_to_cpp = true; + } else { + std::cerr << "WARNING! Casting to std::shared_ptr<> will cause Python subclass of pybind11 C++ instance to lose its Python portion. " + "Make your base class extend from pybind11::wrapper<> to prevent aliasing." + << std::endl; + } + } + } + if (v_h.holder_constructed()) { value = v_h.value_ptr(); holder = v_h.template holder(); @@ -1425,6 +1580,17 @@ struct copyable_holder_caster : public type_caster_base { "of type '" + type_id() + "''"); #endif } + + // Release *after* we already have + if (do_release_to_cpp) { + assert(v_h.inst->owned); + assert(lowest_type->release_info.release_to_cpp); + // Increase reference count to pass to release mechanism. + object obj = reinterpret_borrow(src); + lowest_type->release_info.release_to_cpp(v_h.inst, &holder, std::move(obj)); + } + + return true; } template ::value, int> = 0> @@ -1447,6 +1613,7 @@ struct copyable_holder_caster : public type_caster_base { holder_type holder; + constexpr static detail::HolderTypeId holder_type_id = detail::get_holder_type_id::value; }; /// Specialize for the common std::shared_ptr, so users don't need to @@ -1454,15 +1621,142 @@ template class type_caster> : public copyable_holder_caster> { }; template -struct move_only_holder_caster { +struct move_only_holder_caster : type_caster_base { + using base = type_caster_base; + static_assert(std::is_base_of>::value, + "Holder classes are only supported for custom types"); + using base::base; + using base::cast; + using base::typeinfo; + using base::value; + static_assert(std::is_base_of, type_caster>::value, "Holder classes are only supported for custom types"); static handle cast(holder_type &&src, return_value_policy, handle) { + // Move `src` so that `holder_helper<>::get()` can call `release` if need be. + // That way, if we mix `holder_type`s, we don't have to worry about `existing_holder` + // from being mistakenly reinterpret_cast'd to `shared_ptr` (#1138). auto *ptr = holder_helper::get(src); - return type_caster_base::cast_holder(ptr, &src); + return type_caster_base::cast_holder(ptr, holder_erased(&src)); + } + + // Disable these? +// explicit operator type*() { return this->value; } +// explicit operator type&() { return *(this->value); } + +// explicit operator holder_type*() { return &holder; } + + // Force rvalue. + template + using cast_op_type = holder_type&&; + + explicit operator holder_type&&() { return std::move(holder); } + + object extract_obj(handle src) { + // See if this is a supported `move` container. + bool is_move_container = false; + object obj = none(); + // TODO(eric.cousineau): See if we might need to safeguard against objects that are + // implicitly convertible from `list`. + // Can try to cast to T* first, and if that fails, assume it's a move container. + if (isinstance(src, (PyObject*)&PyList_Type) && PyList_Size(src.ptr()) == 1) { + // Extract the object from a single-item list, and remove the existing reference so we have exclusive control. + // @note This will break implicit casting when constructing from vectors, but eh, who cares. + // Swap. + list li = src.cast(); + obj = li[0]; + li[0] = none(); + is_move_container = true; + } else if (hasattr(src, "_is_move_container")) { + // Try to extract the value with `release()`. + obj = src.attr("release")(); + is_move_container = true; + } else { + obj = reinterpret_borrow(src); + } + if (is_move_container && obj.ref_count() != 1) { + throw std::runtime_error("Non-unique reference from a move-container, cannot cast to unique_ptr."); + } + return obj; } + + bool load(handle src, bool /*convert*/) { + // Allow loose reference management (if it's just a plain object) or require tighter reference + // management if it's a move container. + object obj = extract_obj(src); + // Do not use `load_impl`, as it's not structured conveniently for `unique_ptr`. + // Specifically, trying to delegate to resolving to conversion. + // return base::template load_impl>(src, convert); + check_holder_compat(); + auto v_h = reinterpret_cast(obj.ptr())->get_value_and_holder(); + LoadType load_type = determine_load_type(obj, typeinfo); + return load_value(std::move(obj), std::move(v_h), load_type); + } + static constexpr auto name = type_caster_base::name; + +protected: + friend class type_caster_generic; + void check_holder_compat() { + if (!typeinfo->default_holder) + throw cast_error("Unable to load a non-default holder type (unique_ptr)"); + } + + bool load_value(object obj_exclusive, value_and_holder &&v_h, LoadType load_type) { + // TODO(eric.cousineau): This should try and find the downcast-lowest + // level (closest to child) `release_to_cpp` method that is derived-releasable + // (which derives from `wrapper`). + // This should resolve general casting, and should also avoid alias + // branch issues: + // Example: `Base` has wrapper `PyBase` which extends `wrapper`. + // `Child` extends `Base`, has its own wrapper `PyChild`, which extends + // `wrapper`. + // Anything deriving from `Child` does not derive from `PyBase`, so we should + // NOT try to release using `PyBase`s mechanism. + // Additionally, if `Child` does not have a wrapper (for whatever reason) and is extended, + // then we still can NOT use `PyBase` since it's not part of the hierachy. + + // Try to get the lowest-hierarchy level of the type. + // This requires that we are single-inheritance at most. + const detail::type_info* lowest_type = nullptr; + switch (load_type) { + case LoadType::PureCpp: { + // We already have the lowest type. + lowest_type = typeinfo; + break; + } + // If the base type is explicitly mentioned, then we can rely on `DerivedCppSinglePySingle` being used. + case LoadType::DerivedCppSinglePySingle: + // However, if it is not, it may be that we have a C++ type inheriting from another C++ type without the inheritance being registered. + // In this case, we delegate by effectively downcasting in Python by finding the lowest-level type. + // @note No `break` here on purpose! + case LoadType::ConversionNeeded: { + // Try to get the lowest-hierarchy (closets to child class) of the type. + // The usage of `get_type_info` implicitly requires single inheritance. + auto* py_type = (PyTypeObject*)obj_exclusive.get_type().ptr(); + lowest_type = detail::get_type_info(py_type); + break; + } + case LoadType::DerivedCppMulti: { + throw std::runtime_error( + "pybind11 does not support avoiding slicing with " + "multiple inheritance"); + } + default: { + throw std::runtime_error("Unsupported load type"); + } + } + if (!lowest_type) + throw std::runtime_error("No valid lowest type. Internal error?"); + auto& release_info = lowest_type->release_info; + if (!release_info.release_to_cpp) + throw std::runtime_error("No release mechanism in lowest type?"); + release_info.release_to_cpp(v_h.inst, &holder, std::move(obj_exclusive)); + return true; + } + + holder_type holder; }; template @@ -1575,7 +1869,7 @@ template type_caster &load_type(type_ca throw cast_error("Unable to cast Python instance to C++ type (compile in debug mode for details)"); #else throw cast_error("Unable to cast Python instance of type " + - (std::string) str(handle.get_type()) + " to C++ type '" + type_id() + "''"); + (std::string) str(handle.get_type()) + " to C++ type '" + type_id() + "'"); #endif } return conv; diff --git a/include/pybind11/complex.h b/include/pybind11/complex.h index 5dac27cc4e..3f89638571 100644 --- a/include/pybind11/complex.h +++ b/include/pybind11/complex.h @@ -25,9 +25,13 @@ template struct format_descriptor, detail::enable_i static std::string format() { return std::string(value); } }; +#ifndef PYBIND11_CPP17 + template constexpr const char format_descriptor< std::complex, detail::enable_if_t::value>>::value[3]; +#endif + NAMESPACE_BEGIN(detail) template struct is_fmt_numeric, detail::enable_if_t::value>> { diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 1da36bf321..9b5bd791a1 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -158,6 +158,8 @@ #define PYBIND11_BYTES_SIZE PyBytes_Size #define PYBIND11_LONG_CHECK(o) PyLong_Check(o) #define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o) +#define PYBIND11_LONG_FROM_SIGNED(o) PyLong_FromSsize_t((ssize_t) o) +#define PYBIND11_LONG_FROM_UNSIGNED(o) PyLong_FromSize_t((size_t) o) #define PYBIND11_BYTES_NAME "bytes" #define PYBIND11_STRING_NAME "str" #define PYBIND11_SLICE_OBJECT PyObject @@ -180,6 +182,8 @@ #define PYBIND11_BYTES_SIZE PyString_Size #define PYBIND11_LONG_CHECK(o) (PyInt_Check(o) || PyLong_Check(o)) #define PYBIND11_LONG_AS_LONGLONG(o) (PyInt_Check(o) ? (long long) PyLong_AsLong(o) : PyLong_AsLongLong(o)) +#define PYBIND11_LONG_FROM_SIGNED(o) PyInt_FromSsize_t((ssize_t) o) // Returns long if needed. +#define PYBIND11_LONG_FROM_UNSIGNED(o) PyInt_FromSize_t((size_t) o) // Returns long if needed. #define PYBIND11_BYTES_NAME "str" #define PYBIND11_STRING_NAME "unicode" #define PYBIND11_SLICE_OBJECT PySliceObject @@ -354,6 +358,8 @@ enum class return_value_policy : uint8_t { reference_internal }; +class object; + NAMESPACE_BEGIN(detail) inline static constexpr int log2(size_t n, int k = 0) { return (n <= 1) ? k : log2(n >> 1, k + 1); } @@ -373,6 +379,85 @@ constexpr size_t instance_simple_holder_in_ptrs() { return size_in_ptrs(sizeof(std::shared_ptr)); } +enum class HolderTypeId { + Unknown, + UniquePtr, + SharedPtr, +}; +template +struct get_holder_type_id { + static constexpr HolderTypeId value = HolderTypeId::Unknown; +}; +template +struct get_holder_type_id, void> { + static constexpr HolderTypeId value = HolderTypeId::SharedPtr; +}; +template +struct get_holder_type_id, void> { + // TODO(eric.cousineau): Should this only specialize for `std::default_deleter`? + static constexpr HolderTypeId value = HolderTypeId::UniquePtr; +}; + +class holder_erased { + public: + holder_erased() = default; + holder_erased(const holder_erased&) = default; + holder_erased& operator=(const holder_erased&) = default; + + template + holder_erased(const holder_type* holder) + : ptr_(const_cast(holder)), + type_id_(get_holder_type_id::value), + is_const_(true) {} + + template + holder_erased(holder_type* holder) + : holder_erased(static_cast(holder)) { + is_const_ = false; + } + + holder_erased(const void* ptr, HolderTypeId type_id) + : ptr_(const_cast(ptr)), + type_id_(type_id), + is_const_(true) {} + + holder_erased(void* ptr, HolderTypeId type_id) + : ptr_(ptr), + type_id_(type_id), + is_const_(false) {} + + holder_erased(std::nullptr_t) {} + + void* ptr() const { return ptr_; } + HolderTypeId type_id() const { return type_id_; } + + template + holder_type& mutable_cast() const { + if (is_const_) + throw std::runtime_error("Trying to mutate const reference?"); + return do_cast(); + } + + template + const holder_type& cast() const { + return do_cast(); + } + + operator bool() const { return ptr_; } + private: + template + holder_type& do_cast() const { + if (type_id_ != get_holder_type_id::value) { + throw std::runtime_error("Mismatch on holder type."); + } + return *reinterpret_cast(ptr_); + } + + void* ptr_{}; + HolderTypeId type_id_{HolderTypeId::Unknown}; + bool is_const_{true}; +}; + // Forward declarations struct type_info; struct value_and_holder; @@ -423,6 +508,26 @@ struct instance { /// If true, get_internals().patients has an entry for this object bool has_patients : 1; + typedef void (*release_to_cpp_t)(instance* inst, holder_erased external_holder, object&& obj); + typedef object (*reclaim_from_cpp_t)(instance* inst, holder_erased external_holder); + + struct type_release_info_t { + // Release an instance to C++ for pure C++ instances or Python-derived classes. + release_to_cpp_t release_to_cpp = nullptr; + + // For classes wrapped in `wrapper<>`. See `move_only_holder_caster` for more info. + // Pure / direct C++ objects do not need any fancy releasing mechanisms. They are simply + // unwrapped and passed back. + bool can_derive_from_wrapper = false; + + // The holder that is contained by this class. + HolderTypeId holder_type_id = HolderTypeId::Unknown; + }; + /// If the instance is a Python-derived type that is owned in C++, then this method + /// will permit the instance to be reclaimed back by Python. + // TODO(eric.cousineau): This may not be necessary. See note in `type_caster_generic::cast`. + reclaim_from_cpp_t reclaim_from_cpp = nullptr; + /// Initializes all of the above type/values/holders data (but not the instance values themselves) void allocate_layout(); @@ -704,9 +809,13 @@ template struct format_descriptor constexpr const char format_descriptor< T, detail::enable_if_t::value>>::value[2]; +#endif + /// RAII wrapper that temporarily clears any Python error state struct error_scope { PyObject *type, *value, *trace; diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 213cbaeb21..f769faffc4 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -91,7 +91,7 @@ struct type_info { const std::type_info *cpptype; size_t type_size, holder_size_in_ptrs; void *(*operator_new)(size_t); - void (*init_instance)(instance *, const void *); + void (*init_instance)(instance *, holder_erased); void (*dealloc)(value_and_holder &v_h); std::vector implicit_conversions; std::vector> implicit_casts; @@ -108,6 +108,8 @@ struct type_info { bool default_holder : 1; /* true if this is a type registered with py::module_local */ bool module_local : 1; + + instance::type_release_info_t release_info; }; /// Tracks the `internals` and `type_info` ABI version independent of the main library version diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 2a234cef7f..cdf0db666c 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -17,6 +17,11 @@ # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wconversion" # pragma GCC diagnostic ignored "-Wdeprecated-declarations" +# ifdef __clang__ +// Eigen generates a bunch of implicit-copy-constructor-is-deprecated warnings with -Wdeprecated +// under Clang, so disable that warning here: +# pragma GCC diagnostic ignored "-Wdeprecated" +# endif # if __GNUC__ >= 7 # pragma GCC diagnostic ignored "-Wint-in-bool-context" # endif @@ -107,6 +112,10 @@ struct eigen_extract_stride> template struct eigen_extract_stride> { using type = StrideType; }; +template bool is_pyobject_() { + return static_cast(npy_format_descriptor::value) == npy_api::NPY_OBJECT_; +} + // Helper struct for extracting information from an Eigen type template struct EigenProps { using Type = Type_; @@ -139,14 +148,19 @@ template struct EigenProps { const auto dims = a.ndim(); if (dims < 1 || dims > 2) return false; - + bool is_pyobject = false; + if (is_pyobject_()) + is_pyobject = true; + ssize_t scalar_size = (is_pyobject ? static_cast(sizeof(PyObject*)) : + static_cast(sizeof(Scalar))); if (dims == 2) { // Matrix type: require exact match (or dynamic) EigenIndex np_rows = a.shape(0), np_cols = a.shape(1), - np_rstride = a.strides(0) / static_cast(sizeof(Scalar)), - np_cstride = a.strides(1) / static_cast(sizeof(Scalar)); + np_rstride = a.strides(0) / scalar_size, + np_cstride = a.strides(1) / scalar_size; + if ((fixed_rows && np_rows != rows) || (fixed_cols && np_cols != cols)) return false; @@ -156,7 +170,7 @@ template struct EigenProps { // Otherwise we're storing an n-vector. Only one of the strides will be used, but whichever // is used, we want the (single) numpy stride value. const EigenIndex n = a.shape(0), - stride = a.strides(0) / static_cast(sizeof(Scalar)); + stride = a.strides(0) / scalar_size; if (vector) { // Eigen type is a compile-time vector if (fixed && size != n) @@ -207,11 +221,51 @@ template struct EigenProps { template handle eigen_array_cast(typename props::Type const &src, handle base = handle(), bool writeable = true) { constexpr ssize_t elem_size = sizeof(typename props::Scalar); array a; - if (props::vector) - a = array({ src.size() }, { elem_size * src.innerStride() }, src.data(), base); - else - a = array({ src.rows(), src.cols() }, { elem_size * src.rowStride(), elem_size * src.colStride() }, - src.data(), base); + using Scalar = typename props::Type::Scalar; + bool is_pyoject = static_cast(npy_format_descriptor::value) == npy_api::NPY_OBJECT_; + + if (!is_pyoject) { + if (props::vector) + a = array({ src.size() }, { elem_size * src.innerStride() }, src.data(), base); + else + a = array({ src.rows(), src.cols() }, { elem_size * src.rowStride(), elem_size * src.colStride() }, + src.data(), base); + } + else { + if (props::vector) { + a = array( + npy_format_descriptor::dtype(), + { (size_t) src.size() }, + nullptr, + base + ); + auto policy = base ? return_value_policy::automatic_reference : return_value_policy::copy; + for (ssize_t i = 0; i < src.size(); ++i) { + const Scalar src_val = props::fixed_rows ? src(0, i) : src(i, 0); + auto value_ = reinterpret_steal(make_caster::cast(src_val, policy, base)); + if (!value_) + return handle(); + a.attr("itemset")(i, value_); + } + } + else { + a = array( + npy_format_descriptor::dtype(), + {(size_t) src.rows(), (size_t) src.cols()}, + nullptr, + base + ); + auto policy = base ? return_value_policy::automatic_reference : return_value_policy::copy; + for (ssize_t i = 0; i < src.rows(); ++i) { + for (ssize_t j = 0; j < src.cols(); ++j) { + auto value_ = reinterpret_steal(make_caster::cast(src(i, j), policy, base)); + if (!value_) + return handle(); + a.attr("itemset")(i, j, value_); + } + } + } + } if (!writeable) array_proxy(a.ptr())->flags &= ~detail::npy_api::NPY_ARRAY_WRITEABLE_; @@ -265,14 +319,46 @@ struct type_caster::value>> { auto fits = props::conformable(buf); if (!fits) return false; - + int result = 0; // Allocate the new type, then build a numpy reference into it value = Type(fits.rows, fits.cols); - auto ref = reinterpret_steal(eigen_ref_array(value)); - if (dims == 1) ref = ref.squeeze(); - else if (ref.ndim() == 1) buf = buf.squeeze(); - - int result = detail::npy_api::get().PyArray_CopyInto_(ref.ptr(), buf.ptr()); + bool is_pyobject = is_pyobject_(); + + if (!is_pyobject) { + auto ref = reinterpret_steal(eigen_ref_array(value)); + if (dims == 1) ref = ref.squeeze(); + else if (ref.ndim() == 1) buf = buf.squeeze(); + result = + detail::npy_api::get().PyArray_CopyInto_(ref.ptr(), buf.ptr()); + } + else { + if (dims == 1) { + if (Type::RowsAtCompileTime == Eigen::Dynamic) + value.resize(buf.shape(0), 1); + if (Type::ColsAtCompileTime == Eigen::Dynamic) + value.resize(1, buf.shape(0)); + + for (ssize_t i = 0; i < buf.shape(0); ++i) { + make_caster conv_val; + if (!conv_val.load(buf.attr("item")(i).cast(), convert)) + return false; + value(i) = cast_op(conv_val); + } + } else { + if (Type::RowsAtCompileTime == Eigen::Dynamic || Type::ColsAtCompileTime == Eigen::Dynamic) { + value.resize(buf.shape(0), buf.shape(1)); + } + for (ssize_t i = 0; i < buf.shape(0); ++i) { + for (ssize_t j = 0; j < buf.shape(1); ++j) { + // p is the const void pointer to the item + make_caster conv_val; + if (!conv_val.load(buf.attr("item")(i,j).cast(), convert)) + return false; + value(i,j) = cast_op(conv_val); + } + } + } + } if (result < 0) { // Copy failed! PyErr_Clear(); @@ -424,6 +510,7 @@ struct type_caster< // storage order conversion. (Note that we refuse to use this temporary copy when loading an // argument for a Ref with M non-const, i.e. a read-write reference). Array copy_or_ref; + typename std::remove_cv::type val; public: bool load(handle src, bool convert) { // First check whether what we have is already an array of the right type. If not, we can't @@ -431,6 +518,11 @@ struct type_caster< bool need_copy = !isinstance(src); EigenConformable fits; + bool is_pyobject = false; + if (is_pyobject_()) { + is_pyobject = true; + need_copy = true; + } if (!need_copy) { // We don't need a converting copy, but we also need to check whether the strides are // compatible with the Ref's stride requirements @@ -453,15 +545,53 @@ struct type_caster< // We need to copy: If we need a mutable reference, or we're not supposed to convert // (either because we're in the no-convert overload pass, or because we're explicitly // instructed not to copy (via `py::arg().noconvert()`) we have to fail loading. - if (!convert || need_writeable) return false; + if (!is_pyobject && (!convert || need_writeable)) { + return false; + } Array copy = Array::ensure(src); if (!copy) return false; fits = props::conformable(copy); - if (!fits || !fits.template stride_compatible()) + if (!fits || !fits.template stride_compatible()) { return false; - copy_or_ref = std::move(copy); - loader_life_support::add_patient(copy_or_ref); + } + + if (!is_pyobject) { + copy_or_ref = std::move(copy); + loader_life_support::add_patient(copy_or_ref); + } + else { + auto dims = copy.ndim(); + if (dims == 1) { + if (Type::RowsAtCompileTime == Eigen::Dynamic || Type::ColsAtCompileTime == Eigen::Dynamic) { + val.resize(copy.shape(0), 1); + } + for (ssize_t i = 0; i < copy.shape(0); ++i) { + make_caster conv_val; + if (!conv_val.load(copy.attr("item")(i).template cast(), + convert)) + return false; + val(i) = cast_op(conv_val); + + } + } else { + if (Type::RowsAtCompileTime == Eigen::Dynamic || Type::ColsAtCompileTime == Eigen::Dynamic) { + val.resize(copy.shape(0), copy.shape(1)); + } + for (ssize_t i = 0; i < copy.shape(0); ++i) { + for (ssize_t j = 0; j < copy.shape(1); ++j) { + // p is the const void pointer to the item + make_caster conv_val; + if (!conv_val.load(copy.attr("item")(i, j).template cast(), + convert)) + return false; + val(i, j) = cast_op(conv_val); + } + } + } + ref.reset(new Type(val)); + return true; + } } ref.reset(); diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 6fd8fdf377..b00c4dace2 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1227,6 +1227,21 @@ template struct npy_format_descriptor { ::pybind11::detail::npy_format_descriptor::register_dtype \ ({PYBIND11_MAP2_LIST (PYBIND11_FIELD_DESCRIPTOR_EX, Type, __VA_ARGS__)}) +#define PYBIND11_NUMPY_OBJECT_DTYPE(Type) \ + namespace pybind11 { namespace detail { \ + template <> struct npy_format_descriptor { \ + public: \ + enum { value = npy_api::NPY_OBJECT_ }; \ + static pybind11::dtype dtype() { \ + if (auto ptr = npy_api::get().PyArray_DescrFromType_(value)) { \ + return reinterpret_borrow(ptr); \ + } \ + pybind11_fail("Unsupported buffer format!"); \ + } \ + static constexpr auto name = _("object"); \ + }; \ + }} + #endif // __CLION_IDE__ template diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index b489bb2493..26440b56e2 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -47,6 +47,9 @@ NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +template +void unused(Args&&...) {} + /// Wraps an arbitrary C++ function/method/lambda function/.. into a callable Python object class cpp_function : public function { public: @@ -70,14 +73,14 @@ class cpp_function : public function { /// Construct a cpp_function from a class method (non-const) template cpp_function(Return (Class::*f)(Arg...), const Extra&... extra) { - initialize([f](Class *c, Arg... args) -> Return { return (c->*f)(args...); }, + initialize([f](Class *c, Arg... args) -> Return { return (c->*f)(std::forward(args)...); }, (Return (*) (Class *, Arg...)) nullptr, extra...); } /// Construct a cpp_function from a class method (const) template cpp_function(Return (Class::*f)(Arg...) const, const Extra&... extra) { - initialize([f](const Class *c, Arg... args) -> Return { return (c->*f)(args...); }, + initialize([f](const Class *c, Arg... args) -> Return { return (c->*f)(std::forward(args)...); }, (Return (*)(const Class *, Arg ...)) nullptr, extra...); } @@ -899,6 +902,8 @@ class generic_type : public object { tinfo->default_holder = rec.default_holder; tinfo->module_local = rec.module_local; + tinfo->release_info = rec.release_info; + auto &internals = get_internals(); auto tindex = std::type_index(*rec.type); tinfo->direct_conversions = &internals.direct_conversions[tindex]; @@ -1008,6 +1013,149 @@ auto method_adaptor(Return (Class::*pmf)(Args...) const) -> Return (Derived::*)( return pmf; } + + +template +struct wrapper_interface_impl { + static void use_cpp_lifetime(type* cppobj, object&& obj, detail::HolderTypeId holder_type_id) { + auto* tr = dynamic_cast*>(cppobj); + if (tr == nullptr) { + // This has been invoked at too high of a level; should use a + // downcast class's `release_to_cpp` mechanism (if it supports it). + throw std::runtime_error( + "Attempting to release to C++ using pybind11::wrapper<> " + "at too high of a level. Use a class type lower in the hierarchy, such that " + "the Python-derived instance actually is part of the lineage of " + "pybind11::wrapper"); + } + // Let the external holder take ownership, but keep instance registered. + tr->use_cpp_lifetime(std::move(obj), holder_type_id); + } + + static object release_cpp_lifetime(type* cppobj) { + auto* tr = dynamic_cast*>(cppobj); + if (tr == nullptr) { + // This shouldn't happen here... + throw std::runtime_error("Internal error?"); + } + // Return newly created object. + return tr->release_cpp_lifetime(); + } + static wrapper* run(type*, std::false_type) { + return nullptr; + } +}; + +template +struct wrapper_interface_impl { + static void use_cpp_lifetime(type*, object&&, detail::HolderTypeId) { + // This should be captured by runtime flag. + // TODO(eric.cousineau): Runtime flag may not be necessary. + throw std::runtime_error("Internal error?"); + } + static object release_cpp_lifetime(type*) { + throw std::runtime_error("Internal error?"); + } +}; + +template +struct holder_check_impl { + template + static bool check_destruct(...) { + // Noop by default. + return true; + } + template + static bool allow_null_external_holder(const holder_type&) { + return false; + } + template + static bool attempt_holder_transfer(holder_type& holder, detail::holder_erased external_holder_raw) { + // Only called when holder types are different. + unused(holder, external_holder_raw); + throw std::runtime_error("Unable to transfer between holders of different types"); + } + template + static bool accept_holder(detail::holder_erased external_holder_raw, holder_type& holder) { + // Only called when holder types are different. + unused(holder, external_holder_raw); + throw std::runtime_error("Unable to transfer between holders of different types"); + } +}; + +template <> +struct holder_check_impl : public holder_check_impl { + template + static bool check_destruct(detail::instance* inst, detail::holder_erased holder_raw) { + const holder_type& h = holder_raw.cast(); + handle src((PyObject*)inst); + const detail::type_info *lowest_type = get_lowest_type(src, false); + if (!lowest_type) + // We have multiple inheritance, skip. + return true; + auto load_type = detail::determine_load_type(src, lowest_type); + // Check use_count(), assuming that we have an accurate count (no competing threads?) + if (load_type == detail::LoadType::DerivedCppSinglePySingle) { + if (h.use_count() > 1) { + // Increase reference count + const auto& release_info = lowest_type->release_info; + if (release_info.can_derive_from_wrapper) { + // Increase reference count + object obj = reinterpret_borrow(src); + // Release to C++. + holder_type* null_holder = nullptr; + release_info.release_to_cpp(inst, detail::holder_erased(null_holder), std::move(obj)); + return false; + } + } + } + return true; + } + + template + static bool allow_null_external_holder(const holder_type& holder) { + // Called by `release_to_cpp`. + if (holder.use_count() == 1) + // TODO(eric.cousineau): This may not hold true if we pass temporaries??? + // Or if we've copied a `holder` in copyable_holder_caster... + throw std::runtime_error("Internal error: Should have non-null shared_ptr<> external_holder if use_count() == 1"); + else + return true; + } + + template + static bool accept_holder(detail::holder_erased external_holder_raw, holder_type& holder) { + // Only accept shared_ptr from `external_holder_raw`. + if (external_holder_raw.type_id() == detail::HolderTypeId::UniquePtr) { + using T = typename holder_type::element_type; + auto& external_holder = external_holder_raw.mutable_cast>(); + // Transfer to internal. + holder = std::move(external_holder); + return true; + } else { + throw std::runtime_error("Unable to transfer between holders of different types"); + } + } +}; + + +template <> +struct holder_check_impl : public holder_check_impl { + template + static bool attempt_holder_transfer(holder_type& holder, detail::holder_erased external_holder_raw) { + // Only accept shared_ptr from `external_holder_raw`. + if (external_holder_raw.type_id() == detail::HolderTypeId::SharedPtr) { + using T = typename holder_type::element_type; + auto& external_holder = external_holder_raw.mutable_cast>(); + // Transfer to external. + external_holder = std::move(holder); + return true; + } else { + return false; + } + } +}; + template class class_ : public detail::generic_type { template using is_holder = detail::is_holder_type; @@ -1021,7 +1169,9 @@ class class_ : public detail::generic_type { using type = type_; using type_alias = detail::exactly_one_t; constexpr static bool has_alias = !std::is_void::value; + constexpr static bool has_wrapper = std::is_base_of, type_alias>::value; using holder_type = detail::exactly_one_t, options...>; + constexpr static detail::HolderTypeId holder_type_id = detail::get_holder_type_id::value; static_assert(detail::all_of...>::value, "Unknown/invalid class_ template parameters provided"); @@ -1053,6 +1203,12 @@ class class_ : public detail::generic_type { record.dealloc = dealloc; record.default_holder = std::is_same>::value; + // TODO(eric.cousineau): Determine if it is possible to permit releasing without a wrapper... + auto& release_info = record.release_info; + release_info.can_derive_from_wrapper = has_wrapper; + release_info.release_to_cpp = release_to_cpp; + release_info.holder_type_id = holder_type_id; + set_operator_new(&record); /* Register base classes specified via template arguments to class_, if any */ @@ -1069,6 +1225,166 @@ class class_ : public detail::generic_type { } } + static detail::type_info* get_type_info() { + std::type_index id(typeid(type)); + return detail::get_type_info(id); + } + + typedef wrapper_interface_impl wrapper_interface; + using holder_check = holder_check_impl; + + static bool allow_destruct(detail::instance* inst, detail::holder_erased holder) { + // TODO(eric.cousineau): There should not be a case where shared_ptr<> lives in + // C++ and Python, with it being owned by C++. Check this. + return holder_check::template check_destruct(inst, holder); + } + + static void del_wrapped(handle self, object del_orig) { + // This should be called when the item is *actually* being deleted + // TODO(eric.cousineau): Do we care about use cases where the user manually calls this? + detail::instance* inst = (detail::instance*)self.ptr(); + const detail::type_info *lowest_type = detail::get_lowest_type(self); + auto& release_info = lowest_type->release_info; + // The references are as follows: + // 1. When Python calls __del__ via tp_del (default slot) + // 2. When Python gets the instance-bound __del__ method. + // TODO(eric.cousineau): Confirm this ^ + // 3. When pybind11 gets the argument + const int orig_count = self.ref_count(); + + auto v_h = inst->get_value_and_holder(lowest_type); + detail::holder_erased holder_raw(v_h.holder_ptr(), release_info.holder_type_id); + + // TODO(eric.cousineau): Ensure that this does not prevent destruction if + // the Python interpreter is finalizing... + // Is there a way to do this without a custom handler? + + // Purposely do NOT capture `object` to refcount low. + // TODO(eric.cousineau): `allow_destruct` should be registered in `type_info`. + // Right now, this doesn't really type-erase anything... + if (allow_destruct(inst, holder_raw)) { + // Call the old destructor. + if (!del_orig.is(none())) { + del_orig(self); + } + } else { + // This should have been kept alive by an increment in number of references. + unused(orig_count); + assert(self.ref_count() == orig_count + 1); + } + } + + static void release_to_cpp(detail::instance* inst, detail::holder_erased external_holder_raw, object&& obj) { + using detail::LoadType; + auto v_h = inst->get_value_and_holder(); + auto* tinfo = get_type_info(); + if (!inst->owned || !v_h.holder_constructed()) { + throw std::runtime_error("C++ object must be owned by pybind11 when attempting to release to C++"); + } + LoadType load_type = determine_load_type(obj, tinfo); + switch (load_type) { + case LoadType::PureCpp: { + // Given that `obj` is now exclusive, then once it goes out of scope, + // then the registered instance for this object should be destroyed, and this + // should become a pure C++ object, without any ties to `pybind11`. + // Also, even if this instance is of a class derived from a Base that has a + // wrapper-wrapper alias, we do not need to worry about not being in the correct + // hierarchy, since we will simply release from it. + + // TODO(eric.cousineau): Presently, there is no support for a consistent use of weak references. + // If a PureCpp object is released from Python, then all weak references are invalidated, + // even if it comes back... + // Ideally, this could check if there are weak references. But to whom should the lifetime be extended? + // Perhaps the first weak reference that is available? + break; + } + case LoadType::DerivedCppSinglePySingle: { + if (!tinfo->release_info.can_derive_from_wrapper) { + // This could be relaxed if there is an optional `release` mechanism for holders. + // However, there is still slicing. + throw std::runtime_error( + "Python-extended C++ class does not inherit from pybind11::wrapper<>, " + "and the instance will be sliced. Either avoid this situation, or " + "the type extends pybind11::wrapper<>."); + } + auto* cppobj = reinterpret_cast(v_h.value_ptr()); + wrapper_interface::use_cpp_lifetime(cppobj, std::move(obj), holder_type_id); + break; + } + default: { + throw std::runtime_error("Unsupported load type (multiple inheritance)"); + } + } + bool transfer_holder = true; + holder_type& holder = v_h.holder(); + if (!external_holder_raw.ptr()) { + if (holder_check::allow_null_external_holder(holder)) + transfer_holder = false; + else + throw std::runtime_error("Internal error: Null external holder"); + } + if (transfer_holder) { + if (external_holder_raw.type_id() == holder_type_id) { + holder_type& external_holder = external_holder_raw.mutable_cast(); + external_holder = std::move(holder); + } else { + // Only allow unique_ptr<> -> shared_ptr<> + holder_check::attempt_holder_transfer(holder, external_holder_raw); + } + } + holder.~holder_type(); + v_h.set_holder_constructed(false); + inst->owned = false; + // Register this type's reclamation procedure, since it's wrapper may have the contained object. + inst->reclaim_from_cpp = reclaim_from_cpp; + } + + static object reclaim_from_cpp(detail::instance* inst, detail::holder_erased external_holder_raw) { + using detail::LoadType; + auto v_h = inst->get_value_and_holder(); + auto* tinfo = get_type_info(); + // TODO(eric.cousineau): Should relax this to not require a holder be constructed, + // only that the holder itself be default (unique_ptr<>). + if (inst->owned || v_h.holder_constructed()) { + throw std::runtime_error("Derived Python object should live in C++"); + } + if (!external_holder_raw) { + throw std::runtime_error("Internal error - not external holder?"); + } + // Is this valid? + handle h(reinterpret_cast(inst)); + LoadType load_type = determine_load_type(h, tinfo); + { + // TODO(eric.cousineau): Consider releasing a raw pointer, to make it easier for + // interop with purely raw pointers? Nah, just rely on release. + holder_type& holder = v_h.holder(); + holder_type& external_holder = external_holder_raw.mutable_cast(); + new (&holder) holder_type(std::move(external_holder)); + v_h.set_holder_constructed(true); + + // Show that it has been reclaimed. + inst->reclaim_from_cpp = nullptr; + } + object obj; + switch (load_type) { + case LoadType::PureCpp: { + // Nothing complex needed here. + obj = reinterpret_borrow(h); + break; + } + case LoadType::DerivedCppSinglePySingle: { + auto* cppobj = reinterpret_cast(v_h.value_ptr()); + obj = wrapper_interface::release_cpp_lifetime(cppobj); + break; + } + default: { + throw std::runtime_error("Unsupported load type"); + } + } + inst->owned = true; + return obj; + } + template ::value, int> = 0> static void add_base(detail::type_record &rec) { rec.add_base(typeid(Base), [](void *src) -> void * { @@ -1297,8 +1613,15 @@ class class_ : public detail::generic_type { if (holder_ptr) { init_holder_from_existing(v_h, holder_ptr, std::is_copy_constructible()); v_h.set_holder_constructed(); - } else if (inst->owned || detail::always_construct_holder::value) { - new (&v_h.holder()) holder_type(v_h.value_ptr()); + } else { + init_holder_simple(inst, v_h, v_h.value_ptr()); + } + } + + /// Initialize a holder object simply (to be converted). + static void init_holder_simple(detail::instance *inst, detail::value_and_holder &v_h, type* value) { + if (inst->owned || detail::always_construct_holder::value) { + new (&v_h.holder()) holder_type(value); v_h.set_holder_constructed(); } } @@ -1307,13 +1630,70 @@ class class_ : public detail::generic_type { /// instance. Should be called as soon as the `type` value_ptr is set for an instance. Takes an /// optional pointer to an existing holder to use; if not specified and the instance is /// `.owned`, a new holder will be constructed to manage the value pointer. - static void init_instance(detail::instance *inst, const void *holder_ptr) { + static void init_instance(detail::instance *inst, detail::holder_erased holder_ptr) { + using namespace detail; auto v_h = inst->get_value_and_holder(detail::get_type_info(typeid(type))); + type* value_ptr = v_h.value_ptr(); if (!v_h.instance_registered()) { - register_instance(inst, v_h.value_ptr(), v_h.type); + register_instance(inst, value_ptr, v_h.type); v_h.set_instance_registered(); } - init_holder(inst, v_h, (const holder_type *) holder_ptr, v_h.value_ptr()); + if (!holder_ptr) { + init_holder(inst, v_h, nullptr, value_ptr); + } else if (holder_ptr.type_id() == holder_type_id) { + init_holder(inst, v_h, &holder_ptr.cast(), value_ptr); + } else { + // Create a new, empty holder, and then transfer. + init_holder_simple(inst, v_h, nullptr); + if (!v_h.holder_constructed()) { + throw std::runtime_error("Bad edge case"); + } + holder_type& holder = v_h.holder(); + holder_check::accept_holder(holder_ptr, holder); + } + + // TODO(eric.cousineau): Inject override of __del__ for intercepting C++ stuff + handle self((PyObject*)inst); + handle h_type = self.get_type(); + + // Use hacky Python-style inheritance check. + PyTypeObject *py_type = (PyTypeObject*)h_type.ptr(); + bool is_py_derived = py_type->tp_dealloc != detail::pybind11_object_dealloc; + + bool can_add_del = true; + + // Get non-instance-bound method (analogous `tp_del`). + // Is there a way to check if `__del__` is an instance-assigned + // method? (Rather than a class method?) + object del_orig = getattr(h_type, "__del__", none()); + +#if defined(PYPY_VERSION) + // PyPy will not execute an arbitrarily-added `__del__` method. + // Later version of PyPy throw an error if this happens. + // Workaround: Define a no-op `__del__` if this feature is useful. + if (del_orig.is(none())) { + can_add_del = false; + } +#endif + + // Check tp_dealloc + if (is_py_derived && can_add_del) { + // TODO(eric.cousineau): Consider moving this outside of this class, + // to potentially enable multiple inheritance. + const std::string orig_field = "_pybind11_del_orig"; + if (!hasattr(h_type, orig_field.c_str())) { + // NOTE: This is NOT tied to this particular type. + auto del_new = [orig_field](handle h_self) { + // TODO(eric.cousineau): Make this global, not tied to this type. + object del_orig = getattr(h_self.get_type(), orig_field.c_str()); + del_wrapped(h_self, del_orig); + }; + // Replace with an Python-instance-unbound function. + object new_dtor_py = cpp_function(del_new, is_method(h_type)); + setattr(h_type, "__del__", new_dtor_py); + setattr(h_type, orig_field.c_str(), del_orig); + } + } } /// Deallocates an instance; via holder, if constructed; otherwise via operator delete. @@ -1375,15 +1755,30 @@ template class enum_ : public class_ { auto m_entries_ptr = m_entries.inc_ref().ptr(); def("__repr__", [name, m_entries_ptr](Type value) -> pybind11::str { for (const auto &kv : reinterpret_borrow(m_entries_ptr)) { - if (pybind11::cast(kv.second) == value) + if (pybind11::cast(kv.second[int_(0)]) == value) return pybind11::str("{}.{}").format(name, kv.first); } return pybind11::str("{}.???").format(name); }); - def_property_readonly_static("__members__", [m_entries_ptr](object /* self */) { + def_property_readonly_static("__doc__", [m_entries_ptr](handle self) { + std::string docstring; + const char *tp_doc = ((PyTypeObject *) self.ptr())->tp_doc; + if (tp_doc) + docstring += std::string(tp_doc) + "\n\n"; + docstring += "Members:"; + for (const auto &kv : reinterpret_borrow(m_entries_ptr)) { + auto key = std::string(pybind11::str(kv.first)); + auto comment = kv.second[int_(1)]; + docstring += "\n\n " + key; + if (!comment.is_none()) + docstring += " : " + (std::string) pybind11::str(comment); + } + return docstring; + }); + def_property_readonly_static("__members__", [m_entries_ptr](handle /* self */) { dict m; for (const auto &kv : reinterpret_borrow(m_entries_ptr)) - m[kv.first] = kv.second; + m[kv.first] = kv.second[int_(0)]; return m; }, return_value_policy::copy); def(init([](Scalar i) { return static_cast(i); })); @@ -1431,15 +1826,15 @@ template class enum_ : public class_ { /// Export enumeration entries into the parent scope enum_& export_values() { for (const auto &kv : m_entries) - m_parent.attr(kv.first) = kv.second; + m_parent.attr(kv.first) = kv.second[int_(0)]; return *this; } /// Add an enumeration entry - enum_& value(char const* name, Type value) { + enum_& value(char const* name, Type value, const char *doc = nullptr) { auto v = pybind11::cast(value, return_value_policy::copy); this->attr(name) = v; - m_entries[pybind11::str(name)] = v; + m_entries[pybind11::str(name)] = std::make_pair(v, doc); return *this; } diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index d7fa17775c..8227b1090e 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -11,6 +11,7 @@ #include "detail/common.h" #include "buffer_info.h" +#include #include #include @@ -295,6 +296,9 @@ class error_already_set : public std::runtime_error { PyErr_Fetch(&type.ptr(), &value.ptr(), &trace.ptr()); } + error_already_set(const error_already_set &) = default; + error_already_set(error_already_set &&) = default; + inline ~error_already_set(); /// Give the currently-held error back to Python, if any. If there is currently a Python error @@ -1297,6 +1301,114 @@ inline iterator iter(handle obj) { } /// @} python_builtins +/// Trampoline class to permit attaching a derived Python object's data +/// (namely __dict__) to an actual C++ class. +/// If the object lives purely in C++, then there should only be one reference to +/// this data. +template +class wrapper : public Base { + protected: + using Base::Base; + + public: + // TODO(eric.cousineau): Complain if this is not virtual? (and remove `virtual` specifier in dtor?) + + virtual ~wrapper() { + delete_py_if_in_cpp(); + } + + /// To be used by the holder casters, by means of `wrapper_interface<>`. + // TODO(eric.cousineau): Make this private to ensure contract? + void use_cpp_lifetime(object&& patient, detail::HolderTypeId holder_type_id) { + if (lives_in_cpp()) { + throw std::runtime_error("Instance already lives in C++"); + } + holder_type_id_ = holder_type_id; + patient_ = std::move(patient); + // @note It would be nice to put `revive_python3` here, but this is called by + // `PyObject_CallFinalizer`, which will end up reversing its effect anyways. + } + + /// To be used by `move_only_holder_caster`. + object release_cpp_lifetime() { + if (!lives_in_cpp()) { + throw std::runtime_error("Instance does not live in C++"); + } + revive_python3(); + // Remove existing reference. + object tmp = std::move(patient_); + assert(!patient_); + return tmp; + } + + protected: + /// Call this if, for whatever reason, your C++ wrapper class `Base` has a non-trivial + /// destructor that needs to keep information available to the Python-extended class. + /// In this case, you want to delete the Python object *before* you do any work in your wrapper class. + /// + /// As an example, say you have `Base`, and `PyBase` is your wrapper class which extends `wrapper`. + /// By default, if the instance is owned in C++ and deleted, then the destructor order will be: + /// ~PyBase() + /// do_stuff() + /// ~wrapper() + /// delete_py_if_in_cpp() + /// PyChild.__del__ - ERROR: Should have been called before `do_stuff() + /// ~Base() + /// If you explicitly call `delete_py_if_in_cpp()`, then you will get the desired order: + /// ~PyBase() + /// delete_py_if_in_cpp() + /// PyChild.__del__ - GOOD: Workzzz. Called before `do_stuff()`. + /// do_stuff() + /// ~wrapper() + /// delete_py_if_in_cpp() - No-op. Python object has been released. + /// ~Base() + // TODO(eric.cousineau): Verify this with an example workflow. + void delete_py_if_in_cpp() { + if (lives_in_cpp()) { + // Ensure that we still are the unique one, such that the Python classes + // destructor will be called. +#ifdef PYBIND11_WARN_DANGLING_UNIQUE_PYREF + if (holder_type_id_ == detail::HolderTypeId::UniquePtr) { + if (patient_.ref_count() != 1) { + // TODO(eric.cousineau): Add Python class name + std::string class_name = patient_.get_type().str(); + std::cerr + << "WARNING(pybind11): When destroying Python subclass (" << class_name << "), " + << "of a pybind11 class using a unique_ptr holder in C++, " + << "ref_count == " << patient_.ref_count() << " != 1, which may cause undefined behavior." << std::endl + << " Please consider reviewing your code to trim existing references, or use a move-compatible container." << std::endl; + } + } +#endif // PYBIND11_WARN_DANGLING_UNIQUE_HOLDER + // Release object. + release_cpp_lifetime(); + } + } + + // Python3 unfortunately will not implicitly call `__del__` multiple times, + // even if the object is resurrected. This is a dirty workaround. + // @see https://bugs.python.org/issue32377 + inline void revive_python3() { +#if PY_VERSION_HEX >= 0x03000000 + // Reverse single-finalization constraint in Python3. + if (_PyGC_FINALIZED(patient_.ptr())) { + _PyGC_SET_FINALIZED(patient_.ptr(), 0); + } +#endif // PY_VERSION_HEX >= 0x03000000 + } + + private: + bool lives_in_cpp() const { + // NOTE: This is *false* if, for whatever reason, the wrapper class is + // constructed in C++... Meh. Not gonna worry about that situation. + return static_cast(patient_); + } + + object patient_; + detail::HolderTypeId holder_type_id_{detail::HolderTypeId::Unknown}; +}; + + NAMESPACE_BEGIN(detail) template iterator object_api::begin() const { return iter(derived()); } template iterator object_api::end() const { return iterator::sentinel(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5953e0baac..615af172e3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -51,6 +51,7 @@ set(PYBIND11_TEST_FILES test_numpy_vectorize.cpp test_opaque_types.cpp test_operator_overloading.cpp + test_ownership_transfer.cpp test_pickling.cpp test_pytypes.cpp test_sequences_and_iterators.cpp @@ -124,7 +125,7 @@ function(pybind11_enable_warnings target_name) if(MSVC) target_compile_options(${target_name} PRIVATE /W4) else() - target_compile_options(${target_name} PRIVATE -Wall -Wextra -Wconversion -Wcast-qual) + target_compile_options(${target_name} PRIVATE -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated) endif() if(PYBIND11_WERROR) diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index e5413c2ccc..450813403f 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -155,4 +155,9 @@ TEST_SUBMODULE(builtin_casters, m) { // test_complex m.def("complex_cast", [](float x) { return "{}"_s.format(x); }); m.def("complex_cast", [](std::complex x) { return "({}, {})"_s.format(x.real(), x.imag()); }); + + // test int vs. long (Python 2) + m.def("int_cast", []() {return (int) 42;}); + m.def("long_cast", []() {return (long) 42;}); + m.def("longlong_cast", []() {return ULLONG_MAX;}); } diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 2f311f152f..01d0437b58 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -323,3 +323,16 @@ def test_numpy_bool(): assert convert(np.bool_(False)) is False assert noconvert(np.bool_(True)) is True assert noconvert(np.bool_(False)) is False + + +def test_int_long(): + """In Python 2, a C++ int should return a Python int rather than long + if possible: longs are not always accepted where ints are used (such + as the argument to sys.exit()). A C++ long long is always a Python + long.""" + + import sys + must_be_long = type(getattr(sys, 'maxint', 1) + 1) + assert isinstance(m.int_cast(), int) + assert isinstance(m.long_cast(), int) + assert isinstance(m.longlong_cast(), must_be_long) diff --git a/tests/test_call_policies.cpp b/tests/test_call_policies.cpp index 8642188f9e..fd24557834 100644 --- a/tests/test_call_policies.cpp +++ b/tests/test_call_policies.cpp @@ -36,6 +36,8 @@ TEST_SUBMODULE(call_policies, m) { class Child { public: Child() { py::print("Allocating child."); } + Child(const Child &) = default; + Child(Child &&) = default; ~Child() { py::print("Releasing child."); } }; py::class_(m, "Child") diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 927adf3f95..57db3ab701 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -14,6 +14,9 @@ TEST_SUBMODULE(class_, m) { // test_instance struct NoConstructor { + NoConstructor() = default; + NoConstructor(const NoConstructor &) = default; + NoConstructor(NoConstructor &&) = default; static NoConstructor *new_instance() { auto *ptr = new NoConstructor(); print_created(ptr, "via new_instance"); @@ -82,7 +85,12 @@ TEST_SUBMODULE(class_, m) { m.def("dog_bark", [](const Dog &dog) { return dog.bark(); }); // test_automatic_upcasting - struct BaseClass { virtual ~BaseClass() {} }; + struct BaseClass { + BaseClass() = default; + BaseClass(const BaseClass &) = default; + BaseClass(BaseClass &&) = default; + virtual ~BaseClass() {} + }; struct DerivedClass1 : BaseClass { }; struct DerivedClass2 : BaseClass { }; diff --git a/tests/test_class.py b/tests/test_class.py index d94b61bb3a..f161ab6e21 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -238,16 +238,26 @@ class PyDog(m.Dog): pass for cls in m.Dog, PyDog: - refcount_1 = getrefcount(cls) - molly = [cls("Molly") for _ in range(10)] - refcount_2 = getrefcount(cls) - - del molly - pytest.gc_collect() - refcount_3 = getrefcount(cls) - - assert refcount_1 == refcount_3 - assert refcount_2 > refcount_1 + for i in range(5): + refcount_1 = getrefcount(cls) + molly = [cls("Molly") for _ in range(10)] + refcount_2 = getrefcount(cls) + + del molly + pytest.gc_collect() + refcount_3 = getrefcount(cls) + + if cls == PyDog and i == 0: + # If this is the first time the derived class is called, then + # creating the instance created the dtor hook, introducing one more reference. + # @note This changes to `refcount_1 == refcount_3` in Python3. + assert refcount_1 + 1 == refcount_3 or refcount_1 == refcount_3 + else: + assert refcount_1 == refcount_3 + assert refcount_2 > refcount_1 + + # @note Deleting `PyDog` does not return the refcount of `m.Dog` back to zero, due to the + # destructor shim. def test_reentrant_implicit_conversion_failure(msg): diff --git a/tests/test_constants_and_functions.cpp b/tests/test_constants_and_functions.cpp index 8c9ef7f67b..e8ec74b7bc 100644 --- a/tests/test_constants_and_functions.cpp +++ b/tests/test_constants_and_functions.cpp @@ -49,7 +49,14 @@ namespace test_exc_sp { int f1(int x) noexcept { return x+1; } int f2(int x) noexcept(true) { return x+2; } int f3(int x) noexcept(false) { return x+3; } +#if defined(__GNUG__) +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wdeprecated" +#endif int f4(int x) throw() { return x+4; } // Deprecated equivalent to noexcept(true) +#if defined(__GNUG__) +# pragma GCC diagnostic pop +#endif struct C { int m1(int x) noexcept { return x-1; } int m2(int x) const noexcept { return x-2; } @@ -57,8 +64,15 @@ struct C { int m4(int x) const noexcept(true) { return x-4; } int m5(int x) noexcept(false) { return x-5; } int m6(int x) const noexcept(false) { return x-6; } +#if defined(__GNUG__) +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wdeprecated" +#endif int m7(int x) throw() { return x-7; } int m8(int x) const throw() { return x-8; } +#if defined(__GNUG__) +# pragma GCC diagnostic pop +#endif }; } diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index 20be0aa11e..4535472ba1 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -12,10 +12,14 @@ #include #include #include +#include +#include "Eigen/src/Core/util/DisableStupidWarnings.h" using MatrixXdR = Eigen::Matrix; - - +typedef Eigen::AutoDiffScalar ADScalar; +typedef Eigen::Matrix VectorXADScalar; +typedef Eigen::Matrix VectorXADScalarR; +PYBIND11_NUMPY_OBJECT_DTYPE(ADScalar); // Sets/resets a testing reference matrix to have values of 10*r + c, where r and c are the // (1-based) row/column number. @@ -74,7 +78,9 @@ TEST_SUBMODULE(eigen, m) { using FixedMatrixR = Eigen::Matrix; using FixedMatrixC = Eigen::Matrix; using DenseMatrixR = Eigen::Matrix; + using DenseADScalarMatrixR = Eigen::Matrix; using DenseMatrixC = Eigen::Matrix; + using DenseADScalarMatrixC = Eigen::Matrix; using FourRowMatrixC = Eigen::Matrix; using FourColMatrixC = Eigen::Matrix; using FourRowMatrixR = Eigen::Matrix; @@ -86,10 +92,14 @@ TEST_SUBMODULE(eigen, m) { // various tests m.def("double_col", [](const Eigen::VectorXf &x) -> Eigen::VectorXf { return 2.0f * x; }); + m.def("double_adscalar_col", [](const VectorXADScalar &x) -> VectorXADScalar { return 2.0f * x; }); m.def("double_row", [](const Eigen::RowVectorXf &x) -> Eigen::RowVectorXf { return 2.0f * x; }); + m.def("double_adscalar_row", [](const VectorXADScalarR &x) -> VectorXADScalarR { return 2.0f * x; }); m.def("double_complex", [](const Eigen::VectorXcf &x) -> Eigen::VectorXcf { return 2.0f * x; }); m.def("double_threec", [](py::EigenDRef x) { x *= 2; }); + m.def("double_adscalarc", [](py::EigenDRef x) { x *= 2; }); m.def("double_threer", [](py::EigenDRef x) { x *= 2; }); + m.def("double_adscalarr", [](py::EigenDRef x) { x *= 2; }); m.def("double_mat_cm", [](Eigen::MatrixXf x) -> Eigen::MatrixXf { return 2.0f * x; }); m.def("double_mat_rm", [](DenseMatrixR x) -> DenseMatrixR { return 2.0f * x; }); @@ -134,6 +144,12 @@ TEST_SUBMODULE(eigen, m) { return m; }, py::return_value_policy::reference); + // Increments ADScalar Matrix + m.def("incr_adscalar_matrix", [](Eigen::Ref m, double v) { + m += DenseADScalarMatrixC::Constant(m.rows(), m.cols(), v); + return m; + }, py::return_value_policy::reference); + // Same, but accepts a matrix of any strides m.def("incr_matrix_any", [](py::EigenDRef m, double v) { m += Eigen::MatrixXd::Constant(m.rows(), m.cols(), v); @@ -168,12 +184,16 @@ TEST_SUBMODULE(eigen, m) { // return value referencing/copying tests: class ReturnTester { Eigen::MatrixXd mat = create(); + DenseADScalarMatrixR ad_mat = create_ADScalar_mat(); public: ReturnTester() { print_created(this); } ~ReturnTester() { print_destroyed(this); } - static Eigen::MatrixXd create() { return Eigen::MatrixXd::Ones(10, 10); } + static Eigen::MatrixXd create() { return Eigen::MatrixXd::Ones(10, 10); } + static DenseADScalarMatrixR create_ADScalar_mat() { DenseADScalarMatrixR ad_mat(2, 2); + ad_mat << 1, 2, 3, 7; return ad_mat; } static const Eigen::MatrixXd createConst() { return Eigen::MatrixXd::Ones(10, 10); } Eigen::MatrixXd &get() { return mat; } + DenseADScalarMatrixR& get_ADScalarMat() {return ad_mat;} Eigen::MatrixXd *getPtr() { return &mat; } const Eigen::MatrixXd &view() { return mat; } const Eigen::MatrixXd *viewPtr() { return &mat; } @@ -192,6 +212,7 @@ TEST_SUBMODULE(eigen, m) { .def_static("create", &ReturnTester::create) .def_static("create_const", &ReturnTester::createConst) .def("get", &ReturnTester::get, rvp::reference_internal) + .def("get_ADScalarMat", &ReturnTester::get_ADScalarMat, rvp::reference_internal) .def("get_ptr", &ReturnTester::getPtr, rvp::reference_internal) .def("view", &ReturnTester::view, rvp::reference_internal) .def("view_ptr", &ReturnTester::view, rvp::reference_internal) @@ -211,6 +232,18 @@ TEST_SUBMODULE(eigen, m) { .def("corners_const", &ReturnTester::cornersConst, rvp::reference_internal) ; + py::class_(m, "AutoDiffXd") + .def("__init__", + [](ADScalar & self, + double value, + const Eigen::VectorXd& derivatives) { + new (&self) ADScalar(value, derivatives); + }) + .def("value", [](const ADScalar & self) { + return self.value(); + }) + ; + // test_special_matrix_objects // Returns a DiagonalMatrix with diagonal (1,2,3,...) m.def("incr_diag", [](int k) { @@ -295,6 +328,9 @@ TEST_SUBMODULE(eigen, m) { m.def("iss1105_col", [](Eigen::VectorXd) { return true; }); m.def("iss1105_row", [](Eigen::RowVectorXd) { return true; }); + m.def("iss1105_col_obj", [](VectorXADScalar) { return true; }); + m.def("iss1105_row_obj", [](VectorXADScalarR) { return true; }); + // test_named_arguments // Make sure named arguments are working properly: m.def("matrix_multiply", [](const py::EigenDRef A, const py::EigenDRef B) diff --git a/tests/test_eigen.py b/tests/test_eigen.py index 1f263db443..7f0f7ac252 100644 --- a/tests/test_eigen.py +++ b/tests/test_eigen.py @@ -151,6 +151,59 @@ def test_nonunit_stride_from_python(): np.testing.assert_array_equal(counting_mat, [[0., 2, 2], [6, 16, 10], [6, 14, 8]]) +def conv_double_to_adscalar(arr, vice_versa=False): + flat_arr = arr.flatten() + new_arr = np.zeros(flat_arr.shape, dtype=object) + + for i in range(0, flat_arr.shape[0]): + if vice_versa: + new_arr[i] = flat_arr[i].value() + else: + new_arr[i] = m.AutoDiffXd(flat_arr[i], np.ones(1)) + + return new_arr.reshape(arr.shape) + + +def test_eigen_passing_adscalar(): + adscalar_mat = conv_double_to_adscalar(ref) + adscalar_vec_col = adscalar_mat[:, 0] + adscalar_vec_row = adscalar_mat[0, :] + + # Checking if a Python vector is getting doubled, when passed into a dynamic + # row or col vector in Eigen. + adscalar_double_col = m.double_adscalar_col(adscalar_vec_col) + adscalar_double_row = m.double_adscalar_row(adscalar_vec_row) + np.testing.assert_array_equal(conv_double_to_adscalar(adscalar_double_col, vice_versa=True), + 2 * ref[:, 0]) + np.testing.assert_array_equal(conv_double_to_adscalar(adscalar_double_row, vice_versa=True), + 2 * ref[0, :]) + + # Adding 7 to the a dynamic matrix using reference. + incremented_adscalar_mat = conv_double_to_adscalar(m.incr_adscalar_matrix(adscalar_mat, 7.), + vice_versa=True) + np.testing.assert_array_equal(incremented_adscalar_mat, ref + 7) + # The original adscalar_mat remains unchanged in spite of passing by reference. + np.testing.assert_array_equal(conv_double_to_adscalar(adscalar_mat, vice_versa=True), ref) + + # Changes in Python are not reflected in C++ when internal_reference is returned + return_tester = m.ReturnTester() + a = return_tester.get_ADScalarMat() + a[1, 1] = m.AutoDiffXd(4, np.ones(1)) + b = return_tester.get_ADScalarMat() + assert(np.isclose(b[1, 1].value(), 7.)) + + # Checking Issue 1105 + assert m.iss1105_col_obj(adscalar_vec_col[:, None]) + assert m.iss1105_row_obj(adscalar_vec_row[None, :]) + + with pytest.raises(TypeError) as excinfo: + m.iss1105_row_obj(adscalar_vec_col[:, None]) + assert "incompatible function arguments" in str(excinfo) + with pytest.raises(TypeError) as excinfo: + m.iss1105_col_obj(adscalar_vec_row[None, :]) + assert "incompatible function arguments" in str(excinfo) + + def test_negative_stride_from_python(msg): """Eigen doesn't support (as of yet) negative strides. When a function takes an Eigen matrix by copy or const reference, we can pass a numpy array that has negative strides. Otherwise, an diff --git a/tests/test_enum.cpp b/tests/test_enum.cpp index 49f31ba1f0..4cd14a96a6 100644 --- a/tests/test_enum.cpp +++ b/tests/test_enum.cpp @@ -15,9 +15,9 @@ TEST_SUBMODULE(enums, m) { EOne = 1, ETwo }; - py::enum_(m, "UnscopedEnum", py::arithmetic()) - .value("EOne", EOne) - .value("ETwo", ETwo) + py::enum_(m, "UnscopedEnum", py::arithmetic(), "An unscoped enumeration") + .value("EOne", EOne, "Docstring for EOne") + .value("ETwo", ETwo, "Docstring for ETwo") .export_values(); // test_scoped_enum diff --git a/tests/test_enum.py b/tests/test_enum.py index d8eff5278c..d3f5b4d1bf 100644 --- a/tests/test_enum.py +++ b/tests/test_enum.py @@ -18,6 +18,22 @@ def test_unscoped_enum(): assert m.UnscopedEnum.__members__ == \ {"EOne": m.UnscopedEnum.EOne, "ETwo": m.UnscopedEnum.ETwo} + assert m.UnscopedEnum.__doc__ == \ + '''An unscoped enumeration + +Members: + + EOne : Docstring for EOne + + ETwo : Docstring for ETwo''' or m.UnscopedEnum.__doc__ == \ + '''An unscoped enumeration + +Members: + + ETwo : Docstring for ETwo + + EOne : Docstring for EOne''' + # no TypeError exception for unscoped enum ==/!= int comparisons y = m.UnscopedEnum.ETwo assert y == 2 diff --git a/tests/test_factory_constructors.cpp b/tests/test_factory_constructors.cpp index 687a5bf43f..5cfbfdc3f8 100644 --- a/tests/test_factory_constructors.cpp +++ b/tests/test_factory_constructors.cpp @@ -285,6 +285,7 @@ TEST_SUBMODULE(factory_constructors, m) { // test_reallocations // Class that has verbose operator_new/operator_delete calls struct NoisyAlloc { + NoisyAlloc(const NoisyAlloc &) = default; NoisyAlloc(int i) { py::print(py::str("NoisyAlloc(int {})").format(i)); } NoisyAlloc(double d) { py::print(py::str("NoisyAlloc(double {})").format(d)); } ~NoisyAlloc() { py::print("~NoisyAlloc()"); } diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py index 86b2c3b4bb..6b7793cfa6 100644 --- a/tests/test_methods_and_attributes.py +++ b/tests/test_methods_and_attributes.py @@ -4,10 +4,10 @@ def test_methods_and_attributes(): - instance1 = m.ExampleMandA() - instance2 = m.ExampleMandA(32) + instance1 = m.ExampleMandA() # 1 def.ctor + instance2 = m.ExampleMandA(32) # 1 ctor - instance1.add1(instance2) + instance1.add1(instance2) # 1 copy ctor + 1 move instance1.add2(instance2) instance1.add3(instance2) instance1.add4(instance2) @@ -20,7 +20,7 @@ def test_methods_and_attributes(): assert str(instance1) == "ExampleMandA[value=320]" assert str(instance2) == "ExampleMandA[value=32]" - assert str(instance1.self1()) == "ExampleMandA[value=320]" + assert str(instance1.self1()) == "ExampleMandA[value=320]" # 1 copy ctor + 1 move assert str(instance1.self2()) == "ExampleMandA[value=320]" assert str(instance1.self3()) == "ExampleMandA[value=320]" assert str(instance1.self4()) == "ExampleMandA[value=320]" @@ -58,8 +58,8 @@ def test_methods_and_attributes(): assert cstats.alive() == 0 assert cstats.values() == ["32"] assert cstats.default_constructions == 1 - assert cstats.copy_constructions == 3 - assert cstats.move_constructions >= 1 + assert cstats.copy_constructions == 2 + assert cstats.move_constructions == 2 assert cstats.copy_assignments == 0 assert cstats.move_assignments == 0 diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index 35f9d9c4e3..2842543f17 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -43,6 +43,24 @@ int WithStatic2::static_value2 = 2; int VanillaStaticMix1::static_value = 12; int VanillaStaticMix2::static_value = 12; +template > +class Container { +public: + Container(Ptr ptr) + : ptr_(std::move(ptr)) {} + T* get() const { return ptr_.get(); } + Ptr release() { return std::move(ptr_); } + + static void def(py::module &m, const std::string& name) { + py::class_(m, name.c_str()) + .def(py::init()) + .def("get", &Container::get) + .def("release", &Container::release); + } +private: + Ptr ptr_; +}; + TEST_SUBMODULE(multiple_inheritance, m) { // test_multiple_inheritance_mix1 @@ -77,6 +95,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { py::class_(m, "MIType") .def(py::init()); + Container::def(m, "ContainerBase1"); // test_multiple_inheritance_python_many_bases #define PYBIND11_BASEN(N) py::class_>(m, "BaseN" #N).def(py::init()).def("f" #N, [](BaseN &b) { return b.i + N; }) @@ -124,14 +143,16 @@ TEST_SUBMODULE(multiple_inheritance, m) { std::shared_ptr>(m, "Base12a", py::multiple_inheritance()) .def(py::init()); + Container>::def(m, "ContainerBase2a"); + m.def("bar_base2a", [](Base2a *b) { return b->bar(); }); m.def("bar_base2a_sharedptr", [](std::shared_ptr b) { return b->bar(); }); // test_mi_unaligned_base // test_mi_base_return // Issue #801: invalid casting to derived type with MI bases - struct I801B1 { int a = 1; virtual ~I801B1() = default; }; - struct I801B2 { int b = 2; virtual ~I801B2() = default; }; + struct I801B1 { int a = 1; I801B1() = default; I801B1(const I801B1 &) = default; virtual ~I801B1() = default; }; + struct I801B2 { int b = 2; I801B2() = default; I801B2(const I801B2 &) = default; virtual ~I801B2() = default; }; struct I801C : I801B1, I801B2 {}; struct I801D : I801C {}; // Indirect MI // Unregistered classes: @@ -205,7 +226,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { // test_diamond_inheritance // Issue #959: segfault when constructing diamond inheritance instance // All of these have int members so that there will be various unequal pointers involved. - struct B { int b; virtual ~B() = default; }; + struct B { int b; B() = default; B(const B&) = default; virtual ~B() = default; }; struct C0 : public virtual B { int c0; }; struct C1 : public virtual B { int c1; }; struct D : public C0, public C1 { int d; }; diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index 475dd3b3d8..71b93f099f 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -347,3 +347,32 @@ def test_diamond_inheritance(): assert d is d.c0().b() assert d is d.c1().b() assert d is d.c0().c1().b().c0().b() + + +@pytest.unsupported_on_pypy +def test_mi_ownership_constraint(): + # See `test_ownership_transfer` for positive tests. + + # unique_ptr + with pytest.raises(RuntimeError) as excinfo: + c = m.ContainerBase1(m.MIType(10, 100)) + assert "multiple inheritance" in str(excinfo.value) + + # shared_ptr + # Should not throw an error. + obj = m.Base12a(10, 100) + assert obj.bar() == 100 + c = m.ContainerBase2a(obj) + # Use variable to avoid style violations. + assert c is not None + + # TODO(eric.cousineau): This currently causes a segfault in both + # this branch and on `master` (a303c6f). + # Figure out if there is a way to fix this? + + # assert c.get().bar() == 100 + + # # Should throw an error. + # c = m.ContainerBase2a(m.Bae12a(10, 100)) + # assert c.get().foo() == 10 + # assert c.get().bar() == 100 diff --git a/tests/test_ownership_transfer.cpp b/tests/test_ownership_transfer.cpp new file mode 100644 index 0000000000..86c7f0ff6c --- /dev/null +++ b/tests/test_ownership_transfer.cpp @@ -0,0 +1,179 @@ +/* + tests/test_ownership_transfer.cpp -- test ownership transfer semantics. + + Copyright (c) 2017 Eric Cousineau + + All rights reserved. Use of this source code is governed by a + BSD-style license that can be found in the LICENSE file. +*/ + +#if defined(_MSC_VER) && _MSC_VER < 1910 +# pragma warning(disable: 4702) // unreachable code in system header +#endif + +#include +#include "pybind11_tests.h" +#include "object.h" + +enum Label : int { + BaseBadLabel, + ChildBadLabel, + BaseLabel, + ChildLabel, + + BaseBadUniqueLabel, + ChildBadUniqueLabel, + BaseUniqueLabel, + ChildUniqueLabel, +}; + +// For attaching instances of `ConstructorStats`. +template +class Stats {}; + + +template +class DefineBase { + public: + DefineBase(int value) + : value_(value) { + track_created(this, value); + } + // clang does not like having an implicit copy constructor when the + // class is virtual (and rightly so). + DefineBase(const DefineBase&) = delete; + virtual ~DefineBase() { + track_destroyed(this); + } + virtual int value() const { return value_; } + private: + int value_{}; +}; + +template +class DefineBaseContainer { + public: + using T = DefineBase