From 23c3edcf214086d9759e64e7b5ea5eb28e17a7d7 Mon Sep 17 00:00:00 2001 From: Edward Lockhart Date: Sat, 30 Jan 2021 19:05:13 +0000 Subject: [PATCH] =?UTF-8?q?When=20determining=20if=20a=20shared=5Fptr=20al?= =?UTF-8?q?ready=20exists,=20use=20a=20test=20on=20the=20we=E2=80=A6=20(#2?= =?UTF-8?q?819)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * When determining if a shared_ptr already exists, use a test on the weak_ptr instead of a try/catch block. * When determining if a shared_ptr already exists, use a test on the weak_ptr instead of a try/catch block. * weak_from_this is only available in C++17 and later * Switch to use feature flag instead of C++ version flag. * Add Microsoft-specific check. * Avoid undefined preprocessor macro warning treated as error. * Simplify shared_from_this in init_holder * Include in detail/common.h (~stolen~ borrowed from @bstaletic's #2816) * Move class_::get_shared_from_this to detail::try_get_shared_from_this * Simplify try_get_shared_from_this by using weak_ptr::lock() Co-authored-by: Yannick Jadoul --- include/pybind11/detail/common.h | 23 +++++++++++++++++++++++ include/pybind11/pybind11.h | 15 +++++++-------- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 4d33a7a8a..5560198a0 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -162,6 +162,11 @@ #include #include #include +#if defined(__has_include) +# if __has_include() +# include +# endif +#endif // #define PYBIND11_STR_LEGACY_PERMISSIVE // If DEFINED, pybind11::str can hold PyUnicodeObject or PyBytesObject @@ -870,5 +875,23 @@ public: // Forward-declaration; see detail/class.h std::string get_fully_qualified_tp_name(PyTypeObject*); +template +inline static std::shared_ptr try_get_shared_from_this(std::enable_shared_from_this *holder_value_ptr) { +// Pre C++17, this code path exploits undefined behavior, but is known to work on many platforms. +// Use at your own risk! +// See also https://en.cppreference.com/w/cpp/memory/enable_shared_from_this, and in particular +// the `std::shared_ptr gp1 = not_so_good.getptr();` and `try`-`catch` parts of the example. +#if defined(__cpp_lib_enable_shared_from_this) && (!defined(_MSC_VER) || _MSC_VER >= 1912) + return holder_value_ptr->weak_from_this().lock(); +#else + try { + return holder_value_ptr->shared_from_this(); + } + catch (const std::bad_weak_ptr &) { + return nullptr; + } +#endif +} + PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 4fbb96ea1..08c51c6f9 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1505,14 +1505,13 @@ private: template static void init_holder(detail::instance *inst, detail::value_and_holder &v_h, const holder_type * /* unused */, const std::enable_shared_from_this * /* dummy */) { - try { - auto sh = std::dynamic_pointer_cast( - v_h.value_ptr()->shared_from_this()); - if (sh) { - new (std::addressof(v_h.holder())) holder_type(std::move(sh)); - v_h.set_holder_constructed(); - } - } catch (const std::bad_weak_ptr &) {} + + auto sh = std::dynamic_pointer_cast( + detail::try_get_shared_from_this(v_h.value_ptr())); + if (sh) { + new (std::addressof(v_h.holder())) holder_type(std::move(sh)); + v_h.set_holder_constructed(); + } if (!v_h.holder_constructed() && inst->owned) { new (std::addressof(v_h.holder())) holder_type(v_h.value_ptr());