Skip to content

Commit

Permalink
type_caster<absl::Cord>: return Python str when given `return_val…
Browse files Browse the repository at this point in the history
…ue_policy::_clif_automatic`.

Compared to other string-kind `type_caster`s in pybind11, `type_caster<absl::Cord>` is unusual in that it returns `bytes`. All other string-kind `type_caster`s return `str` and can be directed to use `bytes` instead via `return_value_policy::_return_as_bytes`. There is no `return_value_policy::return_as_str` that we could use here. Introducing such a policy just for `type_caster<absl::Cord>` seems heavy-handed. The existing `return_value_policy::_clif_automatic` fits this niche case organically and is fully sufficient for the purposes of PyCLIF-pybind11.

PiperOrigin-RevId: 620916112
  • Loading branch information
Ralf W. Grosse-Kunstleve authored and copybara-github committed Apr 1, 2024
1 parent 0f8a997 commit 01171e9
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 8 deletions.
7 changes: 6 additions & 1 deletion pybind11_abseil/absl_casters.h
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,13 @@ struct type_caster<absl::Cord> {
}

// Conversion part 2 (C++ -> Python)
static handle cast(const absl::Cord& src, return_value_policy /*policy*/,
static handle cast(const absl::Cord& src, return_value_policy policy,
handle /*parent*/) {
#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC)
if (policy == return_value_policy::_clif_automatic) {
return str(std::string(src)).release();
}
#endif
return bytes(std::string(src)).release();
}
};
Expand Down
12 changes: 6 additions & 6 deletions pybind11_abseil/requirements/BUILD
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
load("@python//3.10:defs.bzl", compile_pip_requirements_3_10 = "compile_pip_requirements")
load("@python//3.11:defs.bzl", compile_pip_requirements_3_11 = "compile_pip_requirements")
load("@python//3.12:defs.bzl", compile_pip_requirements_3_12 = "compile_pip_requirements")
load("@python//3.8:defs.bzl", compile_pip_requirements_3_8 = "compile_pip_requirements")
load("@python//3.9:defs.bzl", compile_pip_requirements_3_9 = "compile_pip_requirements")

package(
default_visibility = ["//visibility:private"],
)

load("@python//3.12:defs.bzl", compile_pip_requirements_3_12 = "compile_pip_requirements")
load("@python//3.11:defs.bzl", compile_pip_requirements_3_11 = "compile_pip_requirements")
load("@python//3.10:defs.bzl", compile_pip_requirements_3_10 = "compile_pip_requirements")
load("@python//3.9:defs.bzl", compile_pip_requirements_3_9 = "compile_pip_requirements")
load("@python//3.8:defs.bzl", compile_pip_requirements_3_8 = "compile_pip_requirements")

compile_pip_requirements_3_12(
name = "requirements_3_12",
src = "requirements.in",
Expand Down
12 changes: 12 additions & 0 deletions pybind11_abseil/tests/absl_example.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,13 @@ static_assert(
!std::is_const<const int*>::value); // int is const, pointer is not.

PYBIND11_MODULE(absl_example, m) {
m.attr("PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC") =
#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC)
true;
#else
false;
#endif

// absl::Time/Duration bindings.
m.def("make_duration", &MakeDuration, arg("secs"));
m.def("make_infinite_duration", &MakeInfiniteDuration);
Expand Down Expand Up @@ -440,6 +447,11 @@ PYBIND11_MODULE(absl_example, m) {
// absl::Cord bindings.
m.def("check_absl_cord", &CheckAbslCord, arg("view"), arg("values"));
m.def("return_absl_cord", &ReturnAbslCord, arg("values"));
#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC)
m.def("return_absl_cord_clif_automatic", [](const std::string& values) {
return cast(ReturnAbslCord(values), return_value_policy::_clif_automatic);
});
#endif

// absl::optional bindings.
m.def("check_optional", &CheckOptional, arg("optional") = absl::nullopt,
Expand Down
14 changes: 13 additions & 1 deletion pybind11_abseil/tests/absl_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,12 +487,24 @@ class AbslCordTest(absltest.TestCase):
TEST_STRING = 'absl_Cord'
TEST_BYTES = b'absl_Cord'

def test_return_absl_cord(self):
def test_return_absl_cord_rvp_not_specified(self):
self.assertSequenceEqual(
absl_example.return_absl_cord(self.TEST_STRING), self.TEST_BYTES)
self.assertSequenceEqual(
absl_example.return_absl_cord(self.TEST_BYTES), self.TEST_BYTES)

def test_return_absl_cord_rvp_clif_automatic(self):
if not absl_example.PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC:
self.skipTest('return_value_policy::_clif_automatic not available')
self.assertSequenceEqual(
absl_example.return_absl_cord_clif_automatic(self.TEST_STRING),
self.TEST_STRING,
)
self.assertSequenceEqual(
absl_example.return_absl_cord_clif_automatic(self.TEST_BYTES),
self.TEST_STRING,
)

def test_pass_absl_cord(self):
self.assertTrue(
absl_example.check_absl_cord(self.TEST_STRING, self.TEST_STRING))
Expand Down

0 comments on commit 01171e9

Please sign in to comment.