Skip to content

Commit

Permalink
Rename we_love_dashes_module to we_love_dashes_cc_only_module
Browse files Browse the repository at this point in the history
Move copyable_holder_caster/copyable_holder_caster to proto_caster_impl.h

PiperOrigin-RevId: 627439261
  • Loading branch information
anandolee authored and copybara-github committed May 20, 2024
1 parent 2079e29 commit 31d5ff0
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 89 deletions.
82 changes: 6 additions & 76 deletions pybind11_protobuf/native_proto_caster.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProtoType *>(nullptr)))>> {
private:
using Base = type_caster<intrinsic_t<ProtoType>>;
static constexpr bool const_element =
std::is_const<typename HolderType::element_type>::value;

public:
static constexpr auto name = Base::name;

// C++->Python.
static handle cast(HolderType &&src, return_value_policy, handle p) {
auto *ptr = holder_helper<HolderType>::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 <typename T_>
using cast_op_type = pybind11::detail::movable_cast_op_type<T_>;

protected:
HolderType holder;
};
static_cast<ProtoType *>(nullptr)))>>
: public pybind11_protobuf::move_only_holder_caster_impl<ProtoType,
HolderType> {};

// copyable_holder_caster enables using copyable holder types such as
// std::shared_ptr. It uses type_caster<Proto> to manage the conversion
Expand All @@ -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<ProtoType *>(nullptr)))>> {
private:
using Base = type_caster<intrinsic_t<ProtoType>>;
static constexpr bool const_element =
std::is_const<typename HolderType::element_type>::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<HolderType>::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 <typename>
using cast_op_type = HolderType &;

protected:
HolderType holder;
};
static_cast<ProtoType *>(nullptr)))>>
: public pybind11_protobuf::copyable_holder_caster_impl<ProtoType,
HolderType> {};

// NOTE: We also need to add support and/or test classes:
//
Expand Down
99 changes: 99 additions & 0 deletions pybind11_protobuf/proto_caster_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,105 @@ struct proto_caster : public proto_caster_load_impl<ProtoType>,
// clang-format on
};

// move_only_holder_caster enables using move-only holder types such as
// std::unique_ptr. It uses type_caster<Proto> to manage the conversion
// and construct a holder type.
template <typename MessageType, typename HolderType>
struct move_only_holder_caster_impl {
private:
using Base =
pybind11::detail::type_caster<pybind11::detail::intrinsic_t<MessageType>>;
static constexpr bool const_element =
std::is_const<typename HolderType::element_type>::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<HolderType>::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 <typename T_>
using cast_op_type = pybind11::detail::movable_cast_op_type<T_>;

protected:
HolderType holder;
};

// copyable_holder_caster enables using copyable holder types such
// as std::shared_ptr. It uses type_caster<Proto> to manage the
// conversion and construct a copy of the proto, then returns the
// shared_ptr.
//
// NOTE: When using pybind11 bindings, std::shared_ptr<Proto> is
// almost never correct, as it always makes a copy. It's mostly
// useful for handling methods that return a shared_ptr<const T>,
// which the caller never intends to mutate and where copy semantics
// will work just as well.
//
template <typename MessageType, typename HolderType>
struct copyable_holder_caster_impl {
private:
using Base =
pybind11::detail::type_caster<pybind11::detail::intrinsic_t<MessageType>>;
static constexpr bool const_element =
std::is_const<typename HolderType::element_type>::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<HolderType>::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 <typename>
using cast_op_type = HolderType &;

protected:
HolderType holder;
};
} // namespace pybind11_protobuf

#endif // PYBIND11_PROTOBUF_PROTO_CASTER_IMPL_H_
11 changes: 6 additions & 5 deletions pybind11_protobuf/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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",
Expand All @@ -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"),
Expand All @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
# 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.


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)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions pybind11_protobuf/tests/we_love_dashes_cc_only_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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__':
Expand Down

0 comments on commit 31d5ff0

Please sign in to comment.