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

This change builds on https://github.com/google/pywrapcc/pull/30022.

PiperOrigin-RevId: 597047935

pybind11_abseil/BUILD

@@ -84,6 +84,7 @@ pybind_library(
         ":no_throw_status",
         ":ok_status_singleton_lib",
         ":status_not_ok_exception",
+        "//pybind11_abseil/compat:status_from_py_exc",
         "//pybind11_abseil/cpp_capsule_tools:raw_ptr_from_capsule",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
@@ -97,6 +98,7 @@ pybind_library(
         ":check_status_module_imported",
         ":no_throw_status",
         ":status_caster",
+        "//pybind11_abseil/compat:status_from_py_exc",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
     ],

pybind11_abseil/CMakeLists.txt

@@ -91,6 +91,7 @@ target_link_libraries(
   INTERFACE check_status_module_imported
             no_throw_status
             ok_status_singleton_lib
+            status_from_py_exc
             status_not_ok_exception
             raw_ptr_from_capsule
             absl::status
@@ -105,8 +106,9 @@ target_include_directories(statusor_caster
                            INTERFACE $<BUILD_INTERFACE:${TOP_LEVEL_DIR}>)
 
 target_link_libraries(
-  statusor_caster INTERFACE check_status_module_imported no_throw_status
-                            status_caster absl::status absl::statusor)
+  statusor_caster
+  INTERFACE check_status_module_imported no_throw_status status_caster
+            status_from_py_exc absl::status absl::statusor)
 
 # init_from_tag ================================================================
 

pybind11_abseil/status_caster.h

@@ -5,6 +5,7 @@
 #ifndef PYBIND11_ABSEIL_STATUS_CASTER_H_
 #define PYBIND11_ABSEIL_STATUS_CASTER_H_
 
+#include <pybind11/functional.h>
 #include <pybind11/pybind11.h>
 
 #include <stdexcept>
@@ -13,6 +14,7 @@
 
 #include "absl/status/status.h"
 #include "pybind11_abseil/check_status_module_imported.h"
+#include "pybind11_abseil/compat/status_from_py_exc.h"
 #include "pybind11_abseil/cpp_capsule_tools/raw_ptr_from_capsule.h"
 #include "pybind11_abseil/no_throw_status.h"
 #include "pybind11_abseil/ok_status_singleton_lib.h"
@@ -118,6 +120,49 @@ struct type_caster<absl::Status> : public type_caster_base<absl::Status> {
   }
 };
 
+#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_PACK)
+
+// This code requires https://github.com/google/pywrapcc
+// IMPORTANT:
+//     KEEP
+//         type_caster<absl::Status>
+//         func_wrapper<absl::Status, Args...>
+//     IN THE SAME HEADER FILE
+//     to avoid surprising behavior differences and ODR violations.
+
+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;
+  // NOTE: `noexcept` to guarantee that no C++ exception will ever escape.
+  absl::Status operator()(Args... args) const noexcept {
+    gil_scoped_acquire acq;
+    try {
+      object py_result =
+          hfunc.f.call_with_policies(rvpp, std::forward<Args>(args)...);
+      try {
+        return py_result.template cast<absl::Status>();
+      } catch (cast_error& e) {
+        return absl::Status(absl::StatusCode::kInvalidArgument, e.what());
+      }
+    }
+    // All exceptions derived from std::exception are handled here:
+    // https://github.com/pybind/pybind11/blob/aec6cc5406edb076f5a489c2d7f84bb07052c4a3/include/pybind11/detail/internals.h#L363-L420
+    // Design choice for safety: Intentionally no `catch (...)`:
+    // Occurrence of such exceptions in this context is considered a bug in
+    // user code. The `noexcept` above will lead to process termination.
+    catch (error_already_set& e) {
+      e.restore();
+      return pybind11_abseil::compat::StatusFromPyExcGivenErrOccurred();
+    }
+  }
+};
+
+}  // namespace type_caster_std_function_specializations
+
+#endif
+
 }  // namespace detail
 }  // namespace pybind11
 

pybind11_abseil/statusor_caster.h

@@ -5,7 +5,9 @@
 #ifndef PYBIND11_ABSEIL_STATUSOR_CASTER_H_
 #define PYBIND11_ABSEIL_STATUSOR_CASTER_H_
 
+#include <pybind11/functional.h>
 #include <pybind11/pybind11.h>
+#include <pybind11/type_caster_pyobject_ptr.h>
 
 #include <stdexcept>
 #include <type_traits>
@@ -14,6 +16,7 @@
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "pybind11_abseil/check_status_module_imported.h"
+#include "pybind11_abseil/compat/status_from_py_exc.h"
 #include "pybind11_abseil/no_throw_status.h"
 #include "pybind11_abseil/status_caster.h"
 
@@ -121,6 +124,45 @@ struct type_caster<absl::StatusOr<PayloadType>> {
   }
 };
 
+#if defined(PYBIND11_HAS_RETURN_VALUE_POLICY_PACK)
+
+// This code requires https://github.com/google/pywrapcc
+// IMPORTANT:
+//     KEEP
+//         type_caster<absl::StatusOr<PayloadType>>
+//         func_wrapper<absl::StatusOr<PayloadType>, Args...>
+//     IN THE SAME HEADER FILE
+//     to avoid surprising behavior differences and ODR violations.
+
+namespace type_caster_std_function_specializations {
+
+template <typename PayloadType, typename... Args>
+struct func_wrapper<absl::StatusOr<PayloadType>, Args...> : func_wrapper_base {
+  using func_wrapper_base::func_wrapper_base;
+  // NOTE: `noexcept` to guarantee that no C++ exception will ever escape.
+  absl::StatusOr<PayloadType> operator()(Args... args) const noexcept {
+    gil_scoped_acquire acq;
+    try {
+      object py_result =
+          hfunc.f.call_with_policies(rvpp, std::forward<Args>(args)...);
+      try {
+        return py_result.template cast<absl::StatusOr<PayloadType>>();
+      } catch (cast_error& e) {
+        return absl::Status(absl::StatusCode::kInvalidArgument, e.what());
+      }
+    }
+    // See comment for the corresponding `catch` in status_caster.h.
+    catch (error_already_set& e) {
+      e.restore();
+      return pybind11_abseil::compat::StatusFromPyExcGivenErrOccurred();
+    }
+  }
+};
+
+}  // namespace type_caster_std_function_specializations
+
+#endif
+
 }  // namespace detail
 }  // namespace pybind11
 

pybind11_abseil/tests/BUILD

 # Tests and examples for pybind11_abseil.
 
-# LOAD(pytype_strict_contrib_test)
 load("@pybind11_bazel//:build_defs.bzl", "pybind_extension")
 
 licenses(["notice"])
@@ -24,6 +23,32 @@ py_test(
     srcs_version = "PY3",
 )
 
+cc_library(
+    name = "status_testing_no_cpp_eh_lib",
+    hdrs = ["status_testing_no_cpp_eh_lib.h"],
+    deps = [
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
+        "@local_config_python//:python_headers",  # buildcleaner: keep
+    ],
+)
+
+pybind_extension(
+    name = "status_testing_no_cpp_eh_pybind",
+    srcs = ["status_testing_no_cpp_eh_pybind.cc"],
+    deps = [
+        ":status_testing_no_cpp_eh_lib",
+        "//pybind11_abseil:import_status_module",
+        "//pybind11_abseil:status_caster",
+        "//pybind11_abseil:statusor_caster",
+    ],
+)
+
+py_library(
+    name = "status_testing_no_cpp_eh_test_lib",
+    srcs = ["status_testing_no_cpp_eh_test_lib.py"],
+)
+
 pybind_extension(
     name = "absl_example",
     srcs = ["absl_example.cc"],

pybind11_abseil/tests/status_testing_no_cpp_eh_lib.h

+// This code is meant to be built with C++ exception handling disabled:
+// the whole point of absl::Status, absl::StatusOr is to provide an alternative
+// to C++ exception handling.
+
+#ifndef PYBIND11_ABSEIL_TESTS_STATUS_TESTING_NO_CPP_EH_LIB_H_
+#define PYBIND11_ABSEIL_TESTS_STATUS_TESTING_NO_CPP_EH_LIB_H_
+
+#include <Python.h>
+
+#include <functional>
+#include <string>
+
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
+
+namespace pybind11_abseil_tests {
+namespace status_testing_no_cpp_eh {
+
+inline std::string CallCallbackWithStatusReturn(
+    const std::function<absl::Status()> &cb) {
+  absl::Status cb_return_value = cb();
+  return cb_return_value.ToString();
+}
+
+inline std::string CallCallbackWithStatusOrIntReturn(
+    const std::function<absl::StatusOr<int>()> &cb) {
+  absl::StatusOr<int> cb_return_value = cb();
+  if (cb_return_value.ok()) {
+    return std::to_string(cb_return_value.value());
+  }
+  return cb_return_value.status().ToString();
+}
+
+inline PyObject *CallCallbackWithStatusOrObjectReturn(
+    const std::function<absl::StatusOr<PyObject *>()> &cb) {
+  absl::StatusOr<PyObject*> cb_return_value = cb();
+  if (cb_return_value.ok()) {
+    return cb_return_value.value();
+  }
+  return PyUnicode_FromString(cb_return_value.status().ToString().c_str());
+}
+
+inline absl::Status GenerateErrorStatusNotOk() {
+  return absl::AlreadyExistsError("Something went wrong, again.");
+}
+
+}  // namespace status_testing_no_cpp_eh
+}  // namespace pybind11_abseil_tests
+
+#endif  // PYBIND11_ABSEIL_TESTS_STATUS_TESTING_NO_CPP_EH_LIB_H_

pybind11_abseil/tests/status_testing_no_cpp_eh_pybind.cc

+#if true  // go/pybind11_include_order
+#include <pybind11/pybind11.h>
+#endif
+
+#include <pybind11/functional.h>
+#include <pybind11/type_caster_pyobject_ptr.h>
+
+#include "pybind11_abseil/import_status_module.h"
+#include "pybind11_abseil/status_caster.h"
+#include "pybind11_abseil/statusor_caster.h"
+#include "pybind11_abseil/tests/status_testing_no_cpp_eh_lib.h"
+
+namespace pybind11_abseil_tests {
+namespace status_testing_no_cpp_eh {
+
+PYBIND11_MODULE(status_testing_no_cpp_eh_pybind, m) {
+  pybind11::google::ImportStatusModule();
+
+  m.def("CallCallbackWithStatusReturn", &CallCallbackWithStatusReturn);
+  m.def("CallCallbackWithStatusOrIntReturn",
+        &CallCallbackWithStatusOrIntReturn);
+  m.def("CallCallbackWithStatusOrObjectReturn",
+        &CallCallbackWithStatusOrObjectReturn,
+        pybind11::return_value_policy::take_ownership);
+  m.def("GenerateErrorStatusNotOk", &GenerateErrorStatusNotOk);
+}
+
+}  // namespace status_testing_no_cpp_eh
+}  // namespace pybind11_abseil_tests

pybind11_abseil/tests/status_testing_no_cpp_eh_test.py

+from absl.testing import absltest
+
+from pybind11_abseil.tests import status_testing_no_cpp_eh_pybind
+from pybind11_abseil.tests import status_testing_no_cpp_eh_test_lib as test_lib
+
+
+class _TestModuleMixin:
+
+  def getTestModule(self):  # pylint: disable=invalid-name
+    return status_testing_no_cpp_eh_pybind
+
+
+class StatusReturnTest(test_lib.StatusReturnTest, _TestModuleMixin):
+  pass
+
+
+class StatusOrReturnTest(test_lib.StatusOrReturnTest, _TestModuleMixin):
+  pass
+
+
+class StatusOrPyObjectPtrTest(
+    test_lib.StatusOrPyObjectPtrTest, _TestModuleMixin
+):
+  pass
+
+
+if __name__ == '__main__':
+  absltest.main()

pybind11_abseil/tests/status_testing_no_cpp_eh_test_lib.py

+"""This is a py_library to enable testing with pybind11 & PyCLIF."""
+
+# This is the default for py_test code:
+# pylint: disable=missing-class-docstring
+# pylint: disable=missing-function-docstring
+
+from absl.testing import absltest
+from absl.testing import parameterized
+
+# Exercises status_from_core_py_exc.cc:StatusFromFetchedExc()
+TAB_StatusFromFetchedExc = (
+    (MemoryError, 'RESOURCE_EXHAUSTED: MemoryError'),
+    (NotImplementedError, 'UNIMPLEMENTED: NotImplementedError'),
+    (KeyboardInterrupt, 'ABORTED: KeyboardInterrupt'),
+    (SystemError, 'INTERNAL: SystemError'),
+    (SyntaxError, 'INTERNAL: SyntaxError'),
+    (TypeError, 'INVALID_ARGUMENT: TypeError'),
+    (ValueError, 'OUT_OF_RANGE: ValueError'),
+    (LookupError, 'NOT_FOUND: LookupError'),
+    (RuntimeError, 'UNKNOWN: RuntimeError'),
+)
+
+
+class StatusReturnTest(parameterized.TestCase):
+
+  def setUp(self):
+    super().setUp()
+    self.tm = self.getTestModule()  # pytype: disable=attribute-error
+
+  def testStatusOk(self):  # pylint: disable=invalid-name
+    def cb():
+      pass
+
+    self.assertEqual(self.tm.CallCallbackWithStatusReturn(cb), 'OK')
+
+  @parameterized.parameters(*TAB_StatusFromFetchedExc)
+  def testStatusFromFetchedExc(self, etype, expected):  # pylint: disable=invalid-name
+
+    def cb():
+      raise etype('Msg.')
+
+    self.assertEqual(
+        self.tm.CallCallbackWithStatusReturn(cb), expected + ': Msg.'
+    )
+
+  def testStatusWrongReturnType(self):  # pylint: disable=invalid-name
+    def cb():
+      return ['something']
+
+    if getattr(self.tm, '__pyclif_codegen_mode__', None) == 'c_api':
+      expected = 'OK'
+    else:
+      expected = (
+          'INVALID_ARGUMENT: Unable to cast Python instance of type <class'
+          " 'list'> to C++ type 'absl::Status'"
+      )
+    self.assertEqual(self.tm.CallCallbackWithStatusReturn(cb), expected)
+
+  def testAssertionErrorBare(self):  # pylint: disable=invalid-name
+
+    def cb():
+      assert False
+
+    self.assertEqual(
+        self.tm.CallCallbackWithStatusReturn(cb), 'UNKNOWN: AssertionError: '
+    )
+
+  def testAssertionErrorWithValue(self):  # pylint: disable=invalid-name
+
+    def cb():
+      assert False, 'Unexpected'
+
+    self.assertEqual(
+        self.tm.CallCallbackWithStatusReturn(cb),
+        'UNKNOWN: AssertionError: Unexpected',
+    )
+
+  def testErrorStatusNotOkRoundTrip(self):  # pylint: disable=invalid-name
+
+    def cb():
+      self.tm.GenerateErrorStatusNotOk()
+
+    self.assertEqual(
+        self.tm.CallCallbackWithStatusReturn(cb),
+        'ALREADY_EXISTS: Something went wrong, again.',
+    )
+
+
+class StatusOrReturnTest(parameterized.TestCase):
+
+  def setUp(self):
+    super().setUp()
+    self.tm = self.getTestModule()  # pytype: disable=attribute-error
+
+  def testStatusOrIntOk(self):  # pylint: disable=invalid-name
+    def cb():
+      return 5
+
+    self.assertEqual(self.tm.CallCallbackWithStatusOrIntReturn(cb), '5')
+
+  @parameterized.parameters(*TAB_StatusFromFetchedExc)
+  def testStatusOrIntFromFetchedExc(self, etype, expected):  # pylint: disable=invalid-name
+    def cb():
+      raise etype('Msg.')
+
+    self.assertEqual(
+        self.tm.CallCallbackWithStatusOrIntReturn(cb), expected + ': Msg.'
+    )
+
+  def testStatusOrIntWrongReturnType(self):  # pylint: disable=invalid-name
+    def cb():
+      return '5'
+
+    if getattr(self.tm, '__pyclif_codegen_mode__', None) == 'c_api':
+      expected = 'INVALID_ARGUMENT: TypeError: expecting int'
+    else:
+      expected = (
+          'INVALID_ARGUMENT: Unable to cast Python instance of type <class'
+          " 'str'> to C++ type 'absl::StatusOr<int>'"
+      )
+    self.assertEqual(self.tm.CallCallbackWithStatusOrIntReturn(cb), expected)
+
+
+class StatusOrPyObjectPtrTest(absltest.TestCase):
+
+  def setUp(self):
+    super().setUp()
+    self.tm = self.getTestModule()  # pytype: disable=attribute-error
+
+  def testStatusOrObject(self):  # pylint: disable=invalid-name
+    if getattr(self.tm, '__pyclif_codegen_mode__', None) != 'c_api':
+      self.skipTest('TODO(cl/578064081)')
+    # No leak (manually verified under cl/485274434).
+    while True:
+      lst = [1, 2, 3, 4]
+
+      def cb():
+        return lst
+
+      # call many times to be sure that object reference is not being removed
+      for _ in range(10):
+        res = self.tm.CallCallbackWithStatusOrObjectReturn(cb)
+        self.assertListEqual(res, lst)
+        self.assertIs(res, lst)
+      return  # Comment out for manual leak checking (use `top` command).