Skip to content

Commit

Permalink
Make //third_party/clif/pybind11 independent from `//util/task/pyth…
Browse files Browse the repository at this point in the history
…on`.

Straightforward untangling of dependencies. The current situation arose from very large-scale, multi-year work on the Python bindings for `absl::Status` and `absl::StatusOr` across PyCLIF and pybind11.

Trigger for this cleanup: Support experimental work on going back to deploying pybind11 from the smart_holder branch. However, this cleanup is useful regardless.

For easy future reference, here is a snapshot of the fig graph from which this CL is submitted: https://rwgk.users.x20web.corp.google.com/hg_xl_archive/hgx_back2sh_fig_as_of_2024-06-16%2B2357.txt

PiperOrigin-RevId: 643912636
  • Loading branch information
rwgk committed Aug 27, 2024
1 parent 8c3715f commit fccb788
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 2 deletions.
4 changes: 3 additions & 1 deletion clif/pybind11/runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
#ifndef CLIF_PYBIND11_RUNTIME_H_
#define CLIF_PYBIND11_RUNTIME_H_

// pybind11 includes have to be at the very top, even before Python.h
#include "third_party/pybind11/include/pybind11/smart_holder.h"
#include "third_party/pybind11/include/pybind11/type_caster_pyobject_ptr.h"

// Must be after pybind11 include.
#include "clif/pybind11/status_return_override.h"
#include "clif/python/stltypes.h"
#include "third_party/pybind11_abseil/absl_casters.h"
#include "util/task/python/clif/status_return_override.h"

namespace clif {

Expand Down
16 changes: 16 additions & 0 deletions clif/pybind11/status_return_override.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#include "clif/pybind11/status_return_override.h"

#include "absl/log/check.h"
#include "absl/status/status.h"
#include "third_party/pybind11/include/pybind11/pybind11.h"
#include "third_party/pybind11_abseil/compat/status_from_py_exc.h"

namespace clif_pybind11 {

absl::Status StatusFromErrorAlreadySet(pybind11::error_already_set& e) {
CHECK(PyGILState_Check());
e.restore();
return pybind11_abseil::compat::StatusFromPyExcGivenErrOccurred();
}

} // namespace clif_pybind11
97 changes: 97 additions & 0 deletions clif/pybind11/status_return_override.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#ifndef CLIF_PYBIND11_STATUS_RETURN_OVERRIDE_H_
#define CLIF_PYBIND11_STATUS_RETURN_OVERRIDE_H_

// pybind11 includes have to be at the very top, even before Python.h
#include "third_party/pybind11/include/pybind11/detail/common.h"
#include "third_party/pybind11/include/pybind11/gil.h"
#include "third_party/pybind11/include/pybind11/pybind11.h" // NOLINT(build/include_order)

// Must be after pybind11 include.
#include <string> // NOLINT(build/include_order)

#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "third_party/pybind11/include/pybind11/pytypes.h"

namespace clif_pybind11 {

absl::Status StatusFromErrorAlreadySet(pybind11::error_already_set& e);

template<typename U>
pybind11::function GetOverload(
const U* this_ptr, const std::string& function_name) {
pybind11::gil_scoped_acquire gil;
return pybind11::get_overload(static_cast<const U *>(this_ptr),
function_name.c_str());
}

template <typename U, typename... Ts>
absl::Status CatchErrorAlreadySetAndReturnStatus(
const U* this_ptr, const std::string& function_name,
const pybind11::return_value_policy_pack rvpp, Ts... args) {
try {
pybind11::function overload = GetOverload(this_ptr, function_name);
if (!overload) {
return absl::Status(absl::StatusCode::kUnimplemented,
"No Python overload is defined for " + function_name +
".");
}
overload.call_with_policies(rvpp, args...); /* Ignoring return value. */
return absl::OkStatus();
} catch (pybind11::error_already_set &e) {
return StatusFromErrorAlreadySet(e);
}
}

template <typename StatusOrPayload, typename U, typename... Ts>
StatusOrPayload CatchErrorAlreadySetAndReturnStatusOr(
const U* this_ptr, const std::string& function_name,
const pybind11::return_value_policy_pack rvpp, Ts... args) {
try {
pybind11::function overload = GetOverload(this_ptr, function_name);
if (!overload) {
return absl::Status(absl::StatusCode::kUnimplemented,
"No Python overload is defined for " + function_name +
".");
}
auto o = overload.call_with_policies(rvpp, args...);
return o.template cast<StatusOrPayload>();
} catch (pybind11::error_already_set &e) {
return StatusFromErrorAlreadySet(e);
}
}

} // namespace clif_pybind11

// Similar with macro `PYBIND11_OVERLOAD_PURE` defined in pybind11/pybind11.h,
// but catches all exceptions derived from pybind11::error_already_set and
// converts the exception to a StatusNotOk return.
#define PYBIND11_OVERRIDE_PURE_STATUS_RETURN(cname, name, fn, rvpp, ...) \
return ::clif_pybind11::CatchErrorAlreadySetAndReturnStatus( \
this, name, rvpp, ##__VA_ARGS__);

#define PYBIND11_OVERRIDE_PURE_STATUSOR_RETURN(statusor_payload_type, cname, \
name, fn, rvpp, ...) \
return ::clif_pybind11::CatchErrorAlreadySetAndReturnStatusOr< \
statusor_payload_type>(this, name, rvpp, ##__VA_ARGS__);

#define PYBIND11_OVERRIDE_STATUS_RETURN(cname, name, fn, rvpp, ...) \
pybind11::function overload = ::clif_pybind11::GetOverload(this, name); \
if (overload) { \
return ::clif_pybind11::CatchErrorAlreadySetAndReturnStatus( \
this, name, rvpp, ##__VA_ARGS__); \
} else { \
return cname::fn(__VA_ARGS__); \
}

#define PYBIND11_OVERRIDE_STATUSOR_RETURN(statusor_payload_type, cname, name, \
fn, rvpp, ...) \
pybind11::function overload = ::clif_pybind11::GetOverload(this, name); \
if (overload) { \
return ::clif_pybind11::CatchErrorAlreadySetAndReturnStatusOr< \
statusor_payload_type>(this, name, rvpp, ##__VA_ARGS__); \
} else { \
return cname::fn(__VA_ARGS__); \
}

#endif // CLIF_PYBIND11_STATUS_RETURN_OVERRIDE_H_
2 changes: 1 addition & 1 deletion clif/pybind11/type_casters.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@

#include "absl/container/flat_hash_map.h"
#include "absl/container/flat_hash_set.h"
#include "clif/pybind11/status_return_override.h"
#include "clif/python/types.h"
#include "third_party/pybind11/include/pybind11/detail/common.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 {
Expand Down

0 comments on commit fccb788

Please sign in to comment.