Skip to content

Commit

Permalink
Change pybind11 handling of callbacks with Status, StatusOr retur…
Browse files Browse the repository at this point in the history
…ns to capture C++ exceptions.

BEGIN_PUBLIC

Change pybind11 handling of callbacks with `Status`, `StatusOr` returns to capture C++ exceptions.

This change builds on google/pybind11clif#30022.

END_PUBLIC

The CL is mainly moving the `type_caster_std_function_specializations` code block from

* google3/third_party/clif/pybind11/type_casters.h;l=42-74;rcl=532883006

into google3/third_party/pybind11_abseil/status_caster.h & google3/third_party/pybind11_abseil/statusor_caster.h.

Additionally, the behavior for callbacks with a `Status` return is changed (`catch` `cast_error` in status_caster.h), to ensure that C++ exceptions do not unwind through C++ code built with exception handling disabled (see e.g. google3/third_party/pybind11_abseil/compat/README.md). Note that the added related test code (`testStatusWrongReturnType` in status_testing_no_cpp_eh_test_lib.py) exposed a behavior difference between PyCLIF-C-API and pybind11: returned objects are simply ignored by PyCLIF-C-API. Addressing this is left for later.

All other changes are mainly to reorganize and expand the test code. (Note that the extensive, multi-year refactoring work around `Status`, `StatusOr`, `StatusNotOk` has left the related test code in a fairly unorganized state.)

Notes for completeness:

* This is a fairly fundamental behavior change, but somewhat surprisingly cl/596532587 was the only adjustment needed in google3.

* TGP tested via child cl/578064081, which is only a very small additional production code change (fixing a very obvious bug), and will be submitted shortly after this CL.

PiperOrigin-RevId: 597047935
  • Loading branch information
rwgk committed Jan 22, 2024
1 parent b74c5c1 commit 58a5530
Showing 1 changed file with 0 additions and 35 deletions.
35 changes: 0 additions & 35 deletions clif/pybind11/type_casters.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,48 +31,13 @@
#include "absl/container/flat_hash_set.h"
#include "clif/python/types.h"
#include "third_party/pybind11/include/pybind11/detail/common.h"
#include "third_party/pybind11/include/pybind11/functional.h"
#include "third_party/pybind11_abseil/status_caster.h"
#include "third_party/pybind11_abseil/statusor_caster.h"
#include "util/task/python/clif/status_return_override.h"

namespace pybind11 {
namespace detail {

namespace type_caster_std_function_specializations {

template <typename... Args>
struct func_wrapper<absl::Status, Args...> : func_wrapper_base {
using func_wrapper_base::func_wrapper_base;
absl::Status operator()(Args... args) const {
gil_scoped_acquire acq;
try {
return hfunc.f.call_with_policies(rvpp, std::forward<Args>(args)...)
.template cast<absl::Status>();
} catch (error_already_set &e) {
return util_task_python_clif::StatusFromErrorAlreadySet(e);
}
}
};

template <typename PayloadType, typename... Args>
struct func_wrapper<absl::StatusOr<PayloadType>, Args...> : func_wrapper_base {
using func_wrapper_base::func_wrapper_base;
absl::StatusOr<PayloadType> operator()(Args... args) const {
gil_scoped_acquire acq;
try {
return hfunc.f.call_with_policies(rvpp, std::forward<Args>(args)...)
.template cast<absl::StatusOr<PayloadType>>();
} catch (error_already_set &e) {
return util_task_python_clif::StatusFromErrorAlreadySet(e);
} catch (cast_error & e) {
return absl::Status(absl::StatusCode::kInvalidArgument, e.what());
}
}
};

} // namespace type_caster_std_function_specializations

template <>
struct type_caster<absl::int128> {
public:
Expand Down

0 comments on commit 58a5530

Please sign in to comment.