From 31d5ff03ca856b544912da72c16ecc541d8d6f62 Mon Sep 17 00:00:00 2001 From: Jie Luo Date: Tue, 23 Apr 2024 10:50:58 -0700 Subject: [PATCH] Rename we_love_dashes_module to we_love_dashes_cc_only_module Move copyable_holder_caster/copyable_holder_caster to proto_caster_impl.h PiperOrigin-RevId: 627439261 --- pybind11_protobuf/native_proto_caster.h | 82 ++------------- pybind11_protobuf/proto_caster_impl.h | 99 +++++++++++++++++++ pybind11_protobuf/tests/BUILD | 11 ++- .../we_love_dashes_cc_and_py_in_deps_test.py | 6 +- ...le.cc => we_love_dashes_cc_only_module.cc} | 2 +- .../tests/we_love_dashes_cc_only_test.py | 8 +- 6 files changed, 119 insertions(+), 89 deletions(-) rename pybind11_protobuf/tests/{we_love_dashes_module.cc => we_love_dashes_cc_only_module.cc} (92%) diff --git a/pybind11_protobuf/native_proto_caster.h b/pybind11_protobuf/native_proto_caster.h index 32afb50..e9f742f 100644 --- a/pybind11_protobuf/native_proto_caster.h +++ b/pybind11_protobuf/native_proto_caster.h @@ -109,43 +109,9 @@ struct move_only_holder_caster< ProtoType, HolderType, std::enable_if_t<(std::is_base_of<::google::protobuf::Message, ProtoType>::value && pybind11_protobuf_enable_type_caster( - static_cast(nullptr)))>> { - private: - using Base = type_caster>; - static constexpr bool const_element = - std::is_const::value; - - public: - static constexpr auto name = Base::name; - - // C++->Python. - static handle cast(HolderType &&src, return_value_policy, handle p) { - auto *ptr = holder_helper::get(src); - if (!ptr) return none().release(); - return Base::cast(std::move(*ptr), return_value_policy::move, p); - } - - // Convert Python->C++. - bool load(handle src, bool convert) { - Base base; - if (!base.load(src, convert)) { - return false; - } - holder = base.as_unique_ptr(); - return true; - } - - // PYBIND11_TYPE_CASTER - explicit operator HolderType *() { return &holder; } - explicit operator HolderType &() { return holder; } - explicit operator HolderType &&() && { return std::move(holder); } - - template - using cast_op_type = pybind11::detail::movable_cast_op_type; - - protected: - HolderType holder; -}; + static_cast(nullptr)))>> + : public pybind11_protobuf::move_only_holder_caster_impl {}; // copyable_holder_caster enables using copyable holder types such as // std::shared_ptr. It uses type_caster to manage the conversion @@ -161,45 +127,9 @@ struct copyable_holder_caster< ProtoType, HolderType, std::enable_if_t<(std::is_base_of<::google::protobuf::Message, ProtoType>::value && pybind11_protobuf_enable_type_caster( - static_cast(nullptr)))>> { - private: - using Base = type_caster>; - static constexpr bool const_element = - std::is_const::value; - - public: - static constexpr auto name = Base::name; - - // C++->Python. - static handle cast(const HolderType &src, return_value_policy, handle p) { - // The default path calls into cast_holder so that the holder/deleter - // gets added to the proto. Here we just make a copy - const auto *ptr = holder_helper::get(src); - if (!ptr) return none().release(); - return Base::cast(*ptr, return_value_policy::copy, p); - } - - // Convert Python->C++. - bool load(handle src, bool convert) { - Base base; - if (!base.load(src, convert)) { - return false; - } - // This always makes a copy, but it could, in some cases, grab a reference - // and construct a shared_ptr, since the intention is clearly to mutate - // the existing object... - holder = base.as_unique_ptr(); - return true; - } - - explicit operator HolderType &() { return holder; } - - template - using cast_op_type = HolderType &; - - protected: - HolderType holder; -}; + static_cast(nullptr)))>> + : public pybind11_protobuf::copyable_holder_caster_impl {}; // NOTE: We also need to add support and/or test classes: // diff --git a/pybind11_protobuf/proto_caster_impl.h b/pybind11_protobuf/proto_caster_impl.h index 50eb3bc..e10c0d1 100644 --- a/pybind11_protobuf/proto_caster_impl.h +++ b/pybind11_protobuf/proto_caster_impl.h @@ -315,6 +315,105 @@ struct proto_caster : public proto_caster_load_impl, // clang-format on }; +// move_only_holder_caster enables using move-only holder types such as +// std::unique_ptr. It uses type_caster to manage the conversion +// and construct a holder type. +template +struct move_only_holder_caster_impl { + private: + using Base = + pybind11::detail::type_caster>; + static constexpr bool const_element = + std::is_const::value; + + public: + static constexpr auto name = Base::name; + + // C++->Python. + static pybind11::handle cast(HolderType &&src, pybind11::return_value_policy, + pybind11::handle p) { + auto *ptr = pybind11::detail::holder_helper::get(src); + if (!ptr) return pybind11::none().release(); + return Base::cast(std::move(*ptr), pybind11::return_value_policy::move, p); + } + + // Convert Python->C++. + bool load(pybind11::handle src, bool convert) { + Base base; + if (!base.load(src, convert)) { + return false; + } + holder = base.as_unique_ptr(); + return true; + } + + // PYBIND11_TYPE_CASTER + explicit operator HolderType *() { return &holder; } + explicit operator HolderType &() { return holder; } + explicit operator HolderType &&() && { return std::move(holder); } + + template + using cast_op_type = pybind11::detail::movable_cast_op_type; + + protected: + HolderType holder; +}; + +// copyable_holder_caster enables using copyable holder types such +// as std::shared_ptr. It uses type_caster to manage the +// conversion and construct a copy of the proto, then returns the +// shared_ptr. +// +// NOTE: When using pybind11 bindings, std::shared_ptr is +// almost never correct, as it always makes a copy. It's mostly +// useful for handling methods that return a shared_ptr, +// which the caller never intends to mutate and where copy semantics +// will work just as well. +// +template +struct copyable_holder_caster_impl { + private: + using Base = + pybind11::detail::type_caster>; + static constexpr bool const_element = + std::is_const::value; + + public: + static constexpr auto name = Base::name; + + // C++->Python. + static pybind11::handle cast(const HolderType &src, + pybind11::return_value_policy, + pybind11::handle p) { + // The default path calls into cast_holder so that the + // holder/deleter gets added to the proto. Here we just make a + // copy + const auto *ptr = pybind11::detail::holder_helper::get(src); + if (!ptr) return pybind11::none().release(); + return Base::cast(*ptr, pybind11::return_value_policy::copy, p); + } + + // Convert Python->C++. + bool load(pybind11::handle src, bool convert) { + Base base; + if (!base.load(src, convert)) { + return false; + } + // This always makes a copy, but it could, in some cases, grab a + // reference and construct a shared_ptr, since the intention is + // clearly to mutate the existing object... + holder = base.as_unique_ptr(); + return true; + } + + explicit operator HolderType &() { return holder; } + + template + using cast_op_type = HolderType &; + + protected: + HolderType holder; +}; } // namespace pybind11_protobuf #endif // PYBIND11_PROTOBUF_PROTO_CASTER_IMPL_H_ diff --git a/pybind11_protobuf/tests/BUILD b/pybind11_protobuf/tests/BUILD index dce695c..7ae0fbd 100644 --- a/pybind11_protobuf/tests/BUILD +++ b/pybind11_protobuf/tests/BUILD @@ -5,6 +5,7 @@ load("@com_github_grpc_grpc//bazel:python_rules.bzl", "py_proto_library") # Placeholder: load py_proto_library # Placeholder: load py_library load("@pybind11_bazel//:build_defs.bzl", "pybind_extension") + # [internal] load cc_proto_library.bzl load("@pypi//:requirements.bzl", "requirement") @@ -359,8 +360,8 @@ py_test( ) pybind_extension( - name = "we_love_dashes_module", - srcs = ["we_love_dashes_module.cc"], + name = "we_love_dashes_cc_only_module", + srcs = ["we_love_dashes_cc_only_module.cc"], deps = [ ":we_love_dashes_cc_proto", "//pybind11_protobuf:native_proto_caster", @@ -371,7 +372,7 @@ py_test( name = "we_love_dashes_cc_only_test", srcs = ["we_love_dashes_cc_only_test.py"], deps = [ - ":we_love_dashes_module", + ":we_love_dashes_cc_only_module", "@com_google_absl_py//absl/testing:absltest", "@com_google_protobuf//:protobuf_python", requirement("absl_py"), @@ -382,8 +383,8 @@ py_test( name = "we_love_dashes_cc_and_py_in_deps_test", srcs = ["we_love_dashes_cc_and_py_in_deps_test.py"], deps = [ - ":we_love_dashes_module", - ":we-love-dashes_py_pb2", # fixdeps: keep + ":we_love_dashes_cc_only_module", + ":we-love-dashes_py_pb2", "@com_google_absl_py//absl/testing:absltest", "@com_google_protobuf//:protobuf_python", requirement("absl_py"), diff --git a/pybind11_protobuf/tests/we_love_dashes_cc_and_py_in_deps_test.py b/pybind11_protobuf/tests/we_love_dashes_cc_and_py_in_deps_test.py index 89e71b7..d634dc0 100644 --- a/pybind11_protobuf/tests/we_love_dashes_cc_and_py_in_deps_test.py +++ b/pybind11_protobuf/tests/we_love_dashes_cc_and_py_in_deps_test.py @@ -4,7 +4,7 @@ # BSD-style license that can be found in the LICENSE file. from absl.testing import absltest -from pybind11_protobuf.tests import we_love_dashes_module +from pybind11_protobuf.tests import we_love_dashes_cc_only_module # NOTE: ":we-love-dashes_py_pb2" is in deps but intentionally not imported here. @@ -12,8 +12,8 @@ class MessageTest(absltest.TestCase): def test_return_then_pass(self): - msg = we_love_dashes_module.return_token_effort(234) - score = we_love_dashes_module.pass_token_effort(msg) + msg = we_love_dashes_cc_only_module.return_token_effort(234) + score = we_love_dashes_cc_only_module.pass_token_effort(msg) self.assertEqual(score, 234) diff --git a/pybind11_protobuf/tests/we_love_dashes_module.cc b/pybind11_protobuf/tests/we_love_dashes_cc_only_module.cc similarity index 92% rename from pybind11_protobuf/tests/we_love_dashes_module.cc rename to pybind11_protobuf/tests/we_love_dashes_cc_only_module.cc index 87dbebf..8db09ad 100644 --- a/pybind11_protobuf/tests/we_love_dashes_module.cc +++ b/pybind11_protobuf/tests/we_love_dashes_cc_only_module.cc @@ -10,7 +10,7 @@ namespace { -PYBIND11_MODULE(we_love_dashes_module, m) { +PYBIND11_MODULE(we_love_dashes_cc_only_module, m) { pybind11_protobuf::ImportNativeProtoCasters(); m.def("return_token_effort", [](int score) { diff --git a/pybind11_protobuf/tests/we_love_dashes_cc_only_test.py b/pybind11_protobuf/tests/we_love_dashes_cc_only_test.py index 563ba74..c1cd7c8 100644 --- a/pybind11_protobuf/tests/we_love_dashes_cc_only_test.py +++ b/pybind11_protobuf/tests/we_love_dashes_cc_only_test.py @@ -5,15 +5,15 @@ from absl.testing import absltest from google.protobuf.internal import api_implementation -from pybind11_protobuf.tests import we_love_dashes_module +from pybind11_protobuf.tests import we_love_dashes_cc_only_module class MessageTest(absltest.TestCase): def test_return_then_pass(self): if api_implementation.Type() == 'cpp': - msg = we_love_dashes_module.return_token_effort(234) - score = we_love_dashes_module.pass_token_effort(msg) + msg = we_love_dashes_cc_only_module.return_token_effort(234) + score = we_love_dashes_cc_only_module.pass_token_effort(msg) self.assertEqual(score, 234) else: with self.assertRaisesRegex( @@ -22,7 +22,7 @@ def test_return_then_pass(self): r' pybind11\.test\.TokenEffort in python\.' r' .*pybind11_protobuf\.tests\.we_love_dashes_pb2\?$', ): - we_love_dashes_module.return_token_effort(0) + we_love_dashes_cc_only_module.return_token_effort(0) if __name__ == '__main__':