From 172f3126b93375a6d422210b70beba68c9047beb Mon Sep 17 00:00:00 2001 From: rwgk Date: Tue, 6 Aug 2024 03:37:30 -0700 Subject: [PATCH] Enable building `//third_party/clif/pybind11` with just the smart_holder branch. This is to support b/356954502: Move third_party/pybind11 back to smart_holder branch It is/will still be possible to build with a pybind11 version that supports the PyCLIF-pybind11 unification. I.e. this command still passes: ``` blaze test third_party/clif/... devtools/clif/... --//devtools/clif/python/codegen_mode=pybind11 ``` TGP-tested in combination with all CLs switching google3 back to the smart_holder branch: * https://rwgk.users.x20web.corp.google.com/hg_xl_archive/hgx_back2sh2_as_of_2024-08-03%2B0756.txt * http://tap/OCL:658995293:BASE:659091159:1722693935474:976f6583 PiperOrigin-RevId: 659890012 --- clif/pybind11/clif_type_casters.cc | 9 +- clif/pybind11/clif_type_casters.h | 119 +++++++++++++------------ clif/pybind11/runtime.h | 6 ++ clif/pybind11/status_return_override.h | 19 +++- 4 files changed, 91 insertions(+), 62 deletions(-) diff --git a/clif/pybind11/clif_type_casters.cc b/clif/pybind11/clif_type_casters.cc index 7bd1e09f..c2dff8c6 100644 --- a/clif/pybind11/clif_type_casters.cc +++ b/clif/pybind11/clif_type_casters.cc @@ -16,14 +16,14 @@ #include +#include "clif/pybind11/status_return_override.h" #include "clif/python/postconv.h" #include "clif/python/types.h" - namespace clif_pybind11 { -clif::py::PostConv PostConvFromReturnValuePolicyPack( - const pybind11::return_value_policy_pack& rvpp) { +clif::py::PostConv PostConvFromReturnValuePolicyPack(const rvp_or_rvpp& rvpp) { +#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_PACK) if (rvpp.vec_rvpp.empty()) { auto policy = pybind11::return_value_policy(rvpp); if (policy == pybind11::return_value_policy::_return_as_bytes) { @@ -37,6 +37,9 @@ clif::py::PostConv PostConvFromReturnValuePolicyPack( post_conv.push_back(PostConvFromReturnValuePolicyPack(child_rvpp)); } return clif::py::PostConv(post_conv); +#else + return clif::UnicodeFromBytesIfPossible; +#endif } } // namespace clif_pybind11 diff --git a/clif/pybind11/clif_type_casters.h b/clif/pybind11/clif_type_casters.h index 54f79905..77f9dfe0 100644 --- a/clif/pybind11/clif_type_casters.h +++ b/clif/pybind11/clif_type_casters.h @@ -25,6 +25,7 @@ #include #include "clif/pybind11/runtime.h" +#include "clif/pybind11/status_return_override.h" #include "clif/python/postconv.h" #include "clif/python/pyobject_ptr_conv.h" #include "clif/python/types.h" @@ -110,73 +111,81 @@ constexpr bool HasPyObjFrom() { pybind11::object ReduceExImpl(pybind11::handle self, int protocol); -clif::py::PostConv PostConvFromReturnValuePolicyPack( - const pybind11::return_value_policy_pack& rvpp); +clif::py::PostConv PostConvFromReturnValuePolicyPack(const rvp_or_rvpp& rvpp); } // namespace clif_pybind11 namespace pybind11 { namespace detail { -#define CLIF_TYPE_CASTER_CAST \ - template (), int> = 0> \ - static handle cast(Type& src, const return_value_policy_pack& rvpp, \ - handle /* parent */) { \ - using ::clif::Clif_PyObjFrom; \ - return Clif_PyObjFrom(std::move(src), \ - clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ - } \ - \ - template (), int> = 0> \ - static handle cast(const Type& src, const return_value_policy_pack& rvpp, \ - handle /* parent */) { \ - using ::clif::Clif_PyObjFrom; \ - return Clif_PyObjFrom(std::move(src), \ - clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ - } \ - \ - template (), int> = 0> \ - static handle cast(Type&& src, const return_value_policy_pack& rvpp, \ - handle /* parent */) { \ - using ::clif::Clif_PyObjFrom; \ - return Clif_PyObjFrom(std::move(src), \ - clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ - } \ - \ - template (), int> = 0> \ - static handle cast(Type* src, const return_value_policy_pack& rvpp, \ - handle /* parent */) { \ - using ::clif::Clif_PyObjFrom; \ - return Clif_PyObjFrom(src, \ - clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ - } \ - \ - template (), int> = 0> \ - static handle cast(const Type* src, const return_value_policy_pack& rvpp, \ - handle /* parent */) { \ - using ::clif::Clif_PyObjFrom; \ - return Clif_PyObjFrom(src, \ - clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ - } \ - \ - template (), int> = 0> \ - static handle cast(Type** src, const return_value_policy_pack& rvpp, \ - handle /* parent */) { \ - using ::clif::Clif_PyObjFrom; \ - return Clif_PyObjFrom(src, \ - clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ +#define CLIF_TYPE_CASTER_CAST \ + template (), int> = 0> \ + static handle cast(Type& src, const ::clif_pybind11::rvp_or_rvpp& rvpp, \ + handle /* parent */) { \ + using ::clif::Clif_PyObjFrom; \ + return Clif_PyObjFrom( \ + std::move(src), \ + clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ + } \ + \ + template (), int> = 0> \ + static handle cast(const Type& src, \ + const ::clif_pybind11::rvp_or_rvpp& rvpp, \ + handle /* parent */) { \ + using ::clif::Clif_PyObjFrom; \ + return Clif_PyObjFrom( \ + std::move(src), \ + clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ + } \ + \ + template (), int> = 0> \ + static handle cast(Type&& src, const ::clif_pybind11::rvp_or_rvpp& rvpp, \ + handle /* parent */) { \ + using ::clif::Clif_PyObjFrom; \ + return Clif_PyObjFrom( \ + std::move(src), \ + clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ + } \ + \ + template (), int> = 0> \ + static handle cast(Type* src, const ::clif_pybind11::rvp_or_rvpp& rvpp, \ + handle /* parent */) { \ + using ::clif::Clif_PyObjFrom; \ + return Clif_PyObjFrom( \ + src, clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ + } \ + \ + template (), int> = 0> \ + static handle cast(const Type* src, \ + const ::clif_pybind11::rvp_or_rvpp& rvpp, \ + handle /* parent */) { \ + using ::clif::Clif_PyObjFrom; \ + return Clif_PyObjFrom( \ + src, clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ + } \ + \ + template (), int> = 0> \ + static handle cast(Type** src, const ::clif_pybind11::rvp_or_rvpp& rvpp, \ + handle /* parent */) { \ + using ::clif::Clif_PyObjFrom; \ + return Clif_PyObjFrom( \ + src, clif_pybind11::PostConvFromReturnValuePolicyPack(rvpp)); \ } template struct clif_type_caster { public: +#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_PACK) PYBIND11_TYPE_CASTER_RVPP(Type, const_name()); +#else + PYBIND11_TYPE_CASTER(Type, const_name()); +#endif CLIF_TYPE_CASTER_CAST diff --git a/clif/pybind11/runtime.h b/clif/pybind11/runtime.h index 43bd0772..d2c294a7 100644 --- a/clif/pybind11/runtime.h +++ b/clif/pybind11/runtime.h @@ -54,6 +54,11 @@ struct type_caster> { value = clif::CapsuleWrapper(reinterpret_borrow(src.ptr())); return true; } +// This #if condition and the code below will need to be adapted, depending on +// the future of the PyCLIF-pybind11 unification. +#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC) + // The implementation was in smart_holder_type_casters.h before it was + // removed with https://github.com/pybind/pybind11/pull/5257 void* void_ptr_from_void_ptr_capsule = try_as_void_ptr_capsule_get_pointer(src, typeid(T).name()); if (void_ptr_from_void_ptr_capsule) { @@ -61,6 +66,7 @@ struct type_caster> { static_cast(void_ptr_from_void_ptr_capsule)); return true; } +#endif return false; } diff --git a/clif/pybind11/status_return_override.h b/clif/pybind11/status_return_override.h index f4b364a8..60bb0d7b 100644 --- a/clif/pybind11/status_return_override.h +++ b/clif/pybind11/status_return_override.h @@ -17,6 +17,16 @@ namespace clif_pybind11 { absl::Status StatusFromErrorAlreadySet(pybind11::error_already_set& e); +#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_PACK) +using rvp_or_rvpp = pybind11::return_value_policy_pack; +#define CLIF_PYBIND11_OVERLOAD_CALL_WITH_POLICIES(rvpp, argsddd) \ + overload.call_with_policies(rvpp, argsddd) +#else +using rvp_or_rvpp = pybind11::return_value_policy; +#define CLIF_PYBIND11_OVERLOAD_CALL_WITH_POLICIES(rvpp, argsddd) \ + overload(argsddd) +#endif + template pybind11::function GetOverload( const U* this_ptr, const std::string& function_name) { @@ -28,7 +38,7 @@ pybind11::function GetOverload( template absl::Status CatchErrorAlreadySetAndReturnStatus( const U* this_ptr, const std::string& function_name, - const pybind11::return_value_policy_pack rvpp, Ts... args) { + const rvp_or_rvpp& rvpp, Ts... args) { try { pybind11::function overload = GetOverload(this_ptr, function_name); if (!overload) { @@ -36,7 +46,8 @@ absl::Status CatchErrorAlreadySetAndReturnStatus( "No Python overload is defined for " + function_name + "."); } - overload.call_with_policies(rvpp, args...); /* Ignoring return value. */ + CLIF_PYBIND11_OVERLOAD_CALL_WITH_POLICIES( + rvpp, args...); /* Ignoring return value. */ return absl::OkStatus(); } catch (pybind11::error_already_set &e) { return StatusFromErrorAlreadySet(e); @@ -46,7 +57,7 @@ absl::Status CatchErrorAlreadySetAndReturnStatus( template StatusOrPayload CatchErrorAlreadySetAndReturnStatusOr( const U* this_ptr, const std::string& function_name, - const pybind11::return_value_policy_pack rvpp, Ts... args) { + const rvp_or_rvpp& rvpp, Ts... args) { try { pybind11::function overload = GetOverload(this_ptr, function_name); if (!overload) { @@ -54,7 +65,7 @@ StatusOrPayload CatchErrorAlreadySetAndReturnStatusOr( "No Python overload is defined for " + function_name + "."); } - auto o = overload.call_with_policies(rvpp, args...); + auto o = CLIF_PYBIND11_OVERLOAD_CALL_WITH_POLICIES(rvpp, args...); return o.template cast(); } catch (pybind11::error_already_set &e) { return StatusFromErrorAlreadySet(e);