Skip to content
This repository was archived by the owner on Nov 11, 2025. It is now read-only.

Commit fd93e12

Browse files
CLIF Teamrwgk
authored andcommitted
Remove exception handling of PyObject* return values in CLIF callbacks
If an exception has occurred in the py-callback (-> `PyObject* = nullptr` is returned) the caller must handle the exception. This change removes the exception handling from `ReturnValue<PyObject*>` and offloads this responsibility on the caller. Previously, the exception signal was cleared in the call to `HandlePyExc()` which prohibits the user from handling the exception, while still returning `nullptr` and implicitly signalling that there was an error. Tested: `blaze test third_party/clif/... devtools/clif/...` and `tap_presubmit -p all --train -c 493233664`: https://test.corp.google.com/OCL:493233664:BASE:493182079:1670319490310:bf766ad9 + rerun https://test.corp.google.com/OCL:493233664:BASE:493182079:1670337211526:88c1b8f1 PiperOrigin-RevId: 493571121
1 parent cbadf2a commit fd93e12

File tree

4 files changed

+20
-3
lines changed

4 files changed

+20
-3
lines changed

clif/python/stltypes.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,9 +379,7 @@ template <>
379379
class ReturnValue<PyObject*> {
380380
public:
381381
PyObject* FromPyValue(PyObject* result) {
382-
if (PyErr_Occurred()) {
383-
HandlePyExc();
384-
}
382+
// Return as-is and offload exception handling to the caller.
385383
return result;
386384
}
387385
};

clif/testing/callback.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
#ifndef CLIF_TESTING_CALLBACK_H_
1717
#define CLIF_TESTING_CALLBACK_H_
1818

19+
#include <Python.h>
20+
1921
#include <functional>
2022
#include <string>
2123
#include <vector>
@@ -82,4 +84,6 @@ inline std::string LambdaCallback(std::function<std::string()> f) {
8284
return f();
8385
}
8486

87+
inline PyObject* PyObjectCallback(std::function<PyObject*()> f) { return f(); }
88+
8589
#endif // CLIF_TESTING_CALLBACK_H_

clif/testing/python/callback.clif

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,5 @@ from "clif/testing/callback.h":
3131

3232
# Testing pybind11 code generator generating lambda expressions with callbacks
3333
def LambdaCallback(cb: () -> str) -> bytes
34+
35+
def PyObjectCallback(cb: () -> object) -> object

clif/testing/python/callback_test.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ def StringCallback():
3939
return 'abcd'
4040

4141

42+
def RaisingCallback():
43+
raise ValueError('raised a value error')
44+
45+
4246
class CallbackTest(absltest.TestCase):
4347

4448
def CallbackMethod(self, data):
@@ -90,6 +94,15 @@ def testBytesCallback(self):
9094
self.assertIsInstance(res, bytes)
9195
self.assertEqual(res, b'abcd')
9296

97+
def testPyObjectCallback(self):
98+
res = callback.PyObjectCallback(StringCallback)
99+
self.assertIsInstance(res, str)
100+
self.assertEqual(res, 'abcd')
101+
102+
def testPyObjectCallbackRaises(self):
103+
with self.assertRaisesWithLiteralMatch(ValueError, 'raised a value error'):
104+
callback.PyObjectCallback(RaisingCallback)
105+
93106

94107
if __name__ == '__main__':
95108
absltest.main()

0 commit comments

Comments
 (0)