Skip to content

Commit

Permalink
Merge branch 'smart_holder' into pr2621_patched_sh
Browse files Browse the repository at this point in the history
  • Loading branch information
Ralf W. Grosse-Kunstleve committed Aug 23, 2024
2 parents 8fbce9b + d58cd0d commit 62c7a35
Show file tree
Hide file tree
Showing 27 changed files with 220 additions and 56 deletions.
6 changes: 3 additions & 3 deletions docs/advanced/cast/stl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ the declaration

.. code-block:: cpp
PYBIND11_MAKE_OPAQUE(std::vector<int>);
PYBIND11_MAKE_OPAQUE(std::vector<int>)
before any binding code (e.g. invocations to ``class_::def()``, etc.). This
macro must be specified at the top level (and outside of any namespaces), since
Expand Down Expand Up @@ -207,8 +207,8 @@ The following example showcases usage of :file:`pybind11/stl_bind.h`:
// Don't forget this
#include <pybind11/stl_bind.h>
PYBIND11_MAKE_OPAQUE(std::vector<int>);
PYBIND11_MAKE_OPAQUE(std::map<std::string, double>);
PYBIND11_MAKE_OPAQUE(std::vector<int>)
PYBIND11_MAKE_OPAQUE(std::map<std::string, double>)
// ...
Expand Down
6 changes: 3 additions & 3 deletions docs/advanced/smart_ptrs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ top namespace level before any binding code:

.. code-block:: cpp
PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr<T>);
PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr<T>)
The first argument of :func:`PYBIND11_DECLARE_HOLDER_TYPE` should be a
placeholder name that is used as a template parameter of the second argument.
Expand All @@ -143,7 +143,7 @@ by default. Specify

.. code-block:: cpp
PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr<T>, true);
PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr<T>, true)
if ``SmartPtr<T>`` can always be initialized from a ``T*`` pointer without the
risk of inconsistencies (such as multiple independent ``SmartPtr`` instances
Expand All @@ -161,7 +161,7 @@ specialized:
.. code-block:: cpp
// Always needed for custom holder types
PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr<T>);
PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr<T>)
// Only needed if the type's `.get()` goes by another name
namespace PYBIND11_NAMESPACE { namespace detail {
Expand Down
7 changes: 7 additions & 0 deletions docs/basics.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ For brevity, all code examples assume that the following two lines are present:
namespace py = pybind11;
.. note::

``pybind11/pybind11.h`` includes ``Python.h``, as such it must be the first file
included in any source file or header for `the same reasons as Python.h`_.

.. _`the same reasons as Python.h`: https://docs.python.org/3/extending/extending.html#a-simple-example

Some features may require additional headers, but those will be specified as needed.

.. _simple_example:
Expand Down
17 changes: 17 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,23 @@ New Features:
* Support for CMake older than 3.15 removed. CMake 3.15-3.30 supported.
`#5304 <https://github.com/pybind/pybind11/pull/5304>`_

* The ``array_caster`` in pybind11/stl.h was enhanced to support value types that are not default-constructible.
`#5305 <https://github.com/pybind/pybind11/pull/5305>`_

Version 2.13.5 (August 22, 2024)
--------------------------------

Bug fixes:

* Fix includes when using Windows long paths (``\\?\`` prefix).
`#5321 <https://github.com/pybind/pybind11/pull/5321>`_

* Support ``-Wpedantic`` in C++20 mode.
`#5322 <https://github.com/pybind/pybind11/pull/5322>`_

* Fix and test ``<ranges>`` support for ``py::tuple`` and ``py::list``.
`#5314 <https://github.com/pybind/pybind11/pull/5314>`_

Version 2.13.4 (August 14, 2024)
--------------------------------

Expand Down
18 changes: 14 additions & 4 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -858,8 +858,12 @@ struct copyable_holder_caster<
using base::value;

bool load(handle src, bool convert) {
return base::template load_impl<copyable_holder_caster<type, std::shared_ptr<type>>>(
src, convert);
if (base::template load_impl<copyable_holder_caster<type, std::shared_ptr<type>>>(
src, convert)) {
sh_load_helper.maybe_set_python_instance_is_alias(src);
return true;
}
return false;
}

explicit operator std::shared_ptr<type> *() {
Expand Down Expand Up @@ -914,6 +918,7 @@ struct copyable_holder_caster<
void load_value(value_and_holder &&v_h) {
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
sh_load_helper.loaded_v_h = v_h;
sh_load_helper.was_populated = true;
value = sh_load_helper.get_void_ptr_or_nullptr();
return;
}
Expand Down Expand Up @@ -1041,14 +1046,19 @@ struct move_only_holder_caster<
}

bool load(handle src, bool convert) {
return base::template load_impl<
move_only_holder_caster<type, std::unique_ptr<type, deleter>>>(src, convert);
if (base::template load_impl<
move_only_holder_caster<type, std::unique_ptr<type, deleter>>>(src, convert)) {
sh_load_helper.maybe_set_python_instance_is_alias(src);
return true;
}
return false;
}

void load_value(value_and_holder &&v_h) {
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
sh_load_helper.loaded_v_h = v_h;
sh_load_helper.loaded_v_h.type = typeinfo;
sh_load_helper.was_populated = true;
value = sh_load_helper.get_void_ptr_or_nullptr();
return;
}
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/detail/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

#pragma once

#include "../attr.h"
#include "../options.h"
#include <pybind11/attr.h>
#include <pybind11/options.h>

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail)
Expand Down
8 changes: 8 additions & 0 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,8 @@ PYBIND11_WARNING_POP
}
\endrst */
PYBIND11_WARNING_PUSH
PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")
#define PYBIND11_MODULE(name, variable, ...) \
static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name) \
PYBIND11_MAYBE_UNUSED; \
Expand All @@ -499,6 +501,7 @@ PYBIND11_WARNING_POP
PYBIND11_CATCH_INIT_EXCEPTIONS \
} \
void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ & (variable))
PYBIND11_WARNING_POP

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

Expand Down Expand Up @@ -637,6 +640,11 @@ struct instance {
bool simple_instance_registered : 1;
/// If true, get_internals().patients has an entry for this object
bool has_patients : 1;
// Cannot use PYBIND11_INTERNALS_VERSION >= 6 here without refactoring.
#if PYBIND11_VERSION_MAJOR >= 3
/// If true, this Python object needs to be kept alive for the lifetime of the C++ value.
bool is_alias : 1;
#endif

/// Initializes all of the above type/values/holders data (but not the instance values
/// themselves)
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
#include "common.h"

#if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
# include "../gil.h"
# include <pybind11/gil.h>
#endif

#include "../pytypes.h"
#include <pybind11/pytypes.h>

#include <exception>
#include <mutex>
Expand Down
4 changes: 1 addition & 3 deletions include/pybind11/detail/struct_smart_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ struct smart_holder {
bool vptr_is_external_shared_ptr : 1;
bool is_populated : 1;
bool is_disowned : 1;
bool pointee_depends_on_holder_owner : 1; // SMART_HOLDER_WIP: See PR #2839.

// Design choice: smart_holder is movable but not copyable.
smart_holder(smart_holder &&) = default;
Expand All @@ -146,8 +145,7 @@ struct smart_holder {

smart_holder()
: vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false},
vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false},
pointee_depends_on_holder_owner{false} {}
vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false} {}

bool has_pointee() const { return vptr != nullptr; }

Expand Down
20 changes: 15 additions & 5 deletions include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@

#pragma once

#include "../gil.h"
#include "../pytypes.h"
#include "../trampoline_self_life_support.h"
#include <pybind11/gil.h>
#include <pybind11/pytypes.h>
#include <pybind11/trampoline_self_life_support.h>

#include "common.h"
#include "descr.h"
#include "dynamic_raw_ptr_cast_if_possible.h"
Expand Down Expand Up @@ -704,6 +705,15 @@ inline std::unique_ptr<T, D> unique_with_deleter(T *raw_ptr, std::unique_ptr<D>

template <typename T>
struct load_helper : value_and_holder_helper {
bool was_populated = false;
bool python_instance_is_alias = false;

void maybe_set_python_instance_is_alias(handle src) {
if (was_populated) {
python_instance_is_alias = reinterpret_cast<instance *>(src.ptr())->is_alias;
}
}

static std::shared_ptr<T> make_shared_ptr_with_responsible_parent(T *raw_ptr, handle parent) {
return std::shared_ptr<T>(raw_ptr, shared_ptr_parent_life_support(parent.ptr()));
}
Expand All @@ -724,7 +734,7 @@ struct load_helper : value_and_holder_helper {
throw std::runtime_error("Non-owning holder (load_as_shared_ptr).");
}
auto *type_raw_ptr = static_cast<T *>(void_raw_ptr);
if (hld.pointee_depends_on_holder_owner) {
if (python_instance_is_alias) {
auto *vptr_gd_ptr = std::get_deleter<pybindit::memory::guarded_delete>(hld.vptr);
if (vptr_gd_ptr != nullptr) {
std::shared_ptr<void> released_ptr = vptr_gd_ptr->released_ptr.lock();
Expand Down Expand Up @@ -778,7 +788,7 @@ struct load_helper : value_and_holder_helper {

auto *self_life_support
= dynamic_raw_ptr_cast_if_possible<trampoline_self_life_support>(raw_type_ptr);
if (self_life_support == nullptr && holder().pointee_depends_on_holder_owner) {
if (self_life_support == nullptr && python_instance_is_alias) {
throw value_error("Alias class (also known as trampoline) does not inherit from "
"py::trampoline_self_life_support, therefore the ownership of this "
"instance cannot safely be transferred to C++.");
Expand Down
3 changes: 2 additions & 1 deletion include/pybind11/eigen/matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

#pragma once

#include "../numpy.h"
#include <pybind11/numpy.h>

#include "common.h"

/* HINT: To suppress warnings originating from the Eigen headers, use -isystem.
Expand Down
3 changes: 2 additions & 1 deletion include/pybind11/eigen/tensor.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@

#pragma once

#include "../numpy.h"
#include <pybind11/numpy.h>

#include "common.h"

#if defined(__GNUC__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
Expand Down
7 changes: 3 additions & 4 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -2271,7 +2271,8 @@ class class_ : public detail::generic_type {
}
auto *uninitialized_location = std::addressof(v_h.holder<holder_type>());
auto *value_ptr_w_t = v_h.value_ptr<type>();
bool pointee_depends_on_holder_owner
// Try downcast from `type` to `type_alias`:
inst->is_alias
= detail::dynamic_raw_ptr_cast_if_possible<type_alias>(value_ptr_w_t) != nullptr;
if (holder_void_ptr) {
// Note: inst->owned ignored.
Expand All @@ -2281,14 +2282,12 @@ class class_ : public detail::generic_type {
uninitialized_location, value_ptr_w_t, value_ptr_w_t)) {
if (inst->owned) {
new (uninitialized_location) holder_type(holder_type::from_raw_ptr_take_ownership(
value_ptr_w_t, /*void_cast_raw_ptr*/ pointee_depends_on_holder_owner));
value_ptr_w_t, /*void_cast_raw_ptr*/ inst->is_alias));
} else {
new (uninitialized_location)
holder_type(holder_type::from_raw_ptr_unowned(value_ptr_w_t));
}
}
v_h.holder<holder_type>().pointee_depends_on_holder_owner
= pointee_depends_on_holder_owner;
v_h.set_holder_constructed();
}

Expand Down
2 changes: 2 additions & 0 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1259,6 +1259,7 @@ class sequence_fast_readonly {
using pointer = arrow_proxy<const handle>;

sequence_fast_readonly(handle obj, ssize_t n) : ptr(PySequence_Fast_ITEMS(obj.ptr()) + n) {}
sequence_fast_readonly() = default;

// NOLINTNEXTLINE(readability-const-return-type) // PR #3263
reference dereference() const { return *ptr; }
Expand All @@ -1281,6 +1282,7 @@ class sequence_slow_readwrite {
using pointer = arrow_proxy<const sequence_accessor>;

sequence_slow_readwrite(handle obj, ssize_t index) : obj(obj), index(index) {}
sequence_slow_readwrite() = default;

reference dereference() const { return {obj, static_cast<size_t>(index)}; }
void increment() { ++index; }
Expand Down
10 changes: 5 additions & 5 deletions include/pybind11/stl/filesystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

#pragma once

#include "../pybind11.h"
#include "../detail/common.h"
#include "../detail/descr.h"
#include "../cast.h"
#include "../pytypes.h"
#include <pybind11/cast.h>
#include <pybind11/detail/common.h>
#include <pybind11/detail/descr.h>
#include <pybind11/pybind11.h>
#include <pybind11/pytypes.h>

#include <string>

Expand Down
3 changes: 3 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,9 @@ function(pybind11_enable_warnings target_name)
-Wdeprecated
-Wundef
-Wnon-virtual-dtor)
if(DEFINED CMAKE_CXX_STANDARD AND NOT CMAKE_CXX_STANDARD VERSION_LESS 20)
target_compile_options(${target_name} PRIVATE -Wpedantic)
endif()
endif()

if(PYBIND11_WERROR)
Expand Down
16 changes: 8 additions & 8 deletions tests/local_bindings.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,21 @@ class LocalSimpleException : public std::exception {
std::string message = "";
};

PYBIND11_MAKE_OPAQUE(LocalVec);
PYBIND11_MAKE_OPAQUE(LocalVec2);
PYBIND11_MAKE_OPAQUE(LocalMap);
PYBIND11_MAKE_OPAQUE(NonLocalVec);
// PYBIND11_MAKE_OPAQUE(NonLocalVec2); // same type as LocalVec2
PYBIND11_MAKE_OPAQUE(NonLocalMap);
PYBIND11_MAKE_OPAQUE(NonLocalMap2);
PYBIND11_MAKE_OPAQUE(LocalVec)
PYBIND11_MAKE_OPAQUE(LocalVec2)
PYBIND11_MAKE_OPAQUE(LocalMap)
PYBIND11_MAKE_OPAQUE(NonLocalVec)
// PYBIND11_MAKE_OPAQUE(NonLocalVec2) // same type as LocalVec2
PYBIND11_MAKE_OPAQUE(NonLocalMap)
PYBIND11_MAKE_OPAQUE(NonLocalMap2)

// Simple bindings (used with the above):
template <typename T, int Adjust = 0, typename... Args>
py::class_<T> bind_local(Args &&...args) {
return py::class_<T>(std::forward<Args>(args)...).def(py::init<int>()).def("get", [](T &i) {
return i.i + Adjust;
});
};
}

// Simulate a foreign library base class (to match the example in the docs):
namespace pets {
Expand Down
2 changes: 1 addition & 1 deletion tests/test_callbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ TEST_SUBMODULE(callbacks, m) {
m.def("dummy_function2", [](int i, int j) { return i + j; });
m.def(
"roundtrip",
[](std::function<int(int)> f, bool expect_none = false) {
[](std::function<int(int)> f, bool expect_none) {
if (expect_none && f) {
throw std::runtime_error("Expected None to be converted to empty std::function");
}
Expand Down
4 changes: 2 additions & 2 deletions tests/test_eigen_matrix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void reset_refs() {
}

// Returns element 2,1 from a matrix (used to test copy/nocopy)
double get_elem(const Eigen::Ref<const Eigen::MatrixXd> &m) { return m(2, 1); };
double get_elem(const Eigen::Ref<const Eigen::MatrixXd> &m) { return m(2, 1); }

// Returns a matrix with 10*r + 100*c added to each matrix element (to help test that the matrix
// reference is referencing rows/columns correctly).
Expand All @@ -76,7 +76,7 @@ struct CustomOperatorNew {
Eigen::Matrix4d a = Eigen::Matrix4d::Zero();
Eigen::Matrix4d b = Eigen::Matrix4d::Identity();

EIGEN_MAKE_ALIGNED_OPERATOR_NEW;
EIGEN_MAKE_ALIGNED_OPERATOR_NEW
};

TEST_SUBMODULE(eigen_matrix, m) {
Expand Down
Loading

0 comments on commit 62c7a35

Please sign in to comment.