Skip to content

Commit c7c7d20

Browse files
guitargeekguitargeek
committed
[CPyCppyy] Don't use deprecated PyErr_Fetch and PyErr_Restore API
Also, in some places the same logic could be implemented with just `PyErr_Clear()`.
1 parent 8637917 commit c7c7d20

File tree

8 files changed

+95
-67
lines changed

8 files changed

+95
-67
lines changed

‎bindings/pyroot/cppyy/CPyCppyy/src/CPPOperator.cxx‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include "CPyCppyy.h"
33
#include "CPPOperator.h"
44
#include "CPPInstance.h"
5+
#include "Utility.h"
56

67

78
//- constructor --------------------------------------------------------------
@@ -49,17 +50,14 @@ PyObject* CPyCppyy::CPPOperator::Call(CPPInstance*& self,
4950
return result;
5051
}
5152

52-
PyObject* pytype = 0, *pyvalue = 0, *pytrace = 0;
53-
PyErr_Fetch(&pytype, &pyvalue, &pytrace);
53+
// fetch the current error, resetting the error buffer
54+
auto error = CPyCppyy::Utility::FetchPyError();
5455

5556
result = fStub((PyObject*)self, CPyCppyy_PyArgs_GET_ITEM(args, idx_other));
5657

57-
if (!result)
58-
PyErr_Restore(pytype, pyvalue, pytrace);
59-
else {
60-
Py_XDECREF(pytype);
61-
Py_XDECREF(pyvalue);
62-
Py_XDECREF(pytrace);
58+
// if there was still a problem, restore the Python error buffer
59+
if (!result) {
60+
CPyCppyy::Utility::RestorePyError(error);
6361
}
6462

6563
return result;

‎bindings/pyroot/cppyy/CPyCppyy/src/CPPOverload.cxx‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -712,9 +712,6 @@ static PyObject* mp_call(CPPOverload* pymeth, PyObject* args, PyObject* kwds)
712712
}
713713
}
714714

715-
// clear collected errors
716-
if (!errors.empty())
717-
std::for_each(errors.begin(), errors.end(), Utility::PyError_t::Clear);
718715
return HandleReturn(pymeth, im_self, result);
719716
}
720717

@@ -758,7 +755,7 @@ static PyObject* mp_call(CPPOverload* pymeth, PyObject* args, PyObject* kwds)
758755
// first summarize, then add details
759756
PyObject* topmsg = CPyCppyy_PyText_FromFormat(
760757
"none of the %d overloaded methods succeeded. Full details:", (int)nMethods);
761-
SetDetailedException(errors, topmsg /* steals */, PyExc_TypeError /* default error */);
758+
SetDetailedException(std::move(errors), topmsg /* steals */, PyExc_TypeError /* default error */);
762759

763760
// report failure
764761
return nullptr;

‎bindings/pyroot/cppyy/CPyCppyy/src/CPPScope.cxx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,6 @@ static PyObject* meta_getattro(PyObject* pyclass, PyObject* pyname)
504504
}
505505

506506
if (attr) {
507-
std::for_each(errors.begin(), errors.end(), Utility::PyError_t::Clear);
508507
PyErr_Clear();
509508
} else {
510509
// not found: prepare a full error report
@@ -518,7 +517,7 @@ static PyObject* meta_getattro(PyObject* pyclass, PyObject* pyname)
518517
topmsg = CPyCppyy_PyText_FromFormat("no such attribute \'%s\'. Full details:",
519518
CPyCppyy_PyText_AsString(pyname));
520519
}
521-
SetDetailedException(errors, topmsg /* steals */, PyExc_AttributeError /* default error */);
520+
SetDetailedException(std::move(errors), topmsg /* steals */, PyExc_AttributeError /* default error */);
522521
}
523522

524523
return attr;

‎bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -535,10 +535,9 @@ bool CPyCppyy::Converter::ToMemory(PyObject*, void*, PyObject* /* ctxt */)
535535
if (val == (type)-1 && PyErr_Occurred()) { \
536536
static PyTypeObject* ctypes_type = nullptr; \
537537
if (!ctypes_type) { \
538-
PyObject* pytype = 0, *pyvalue = 0, *pytrace = 0; \
539-
PyErr_Fetch(&pytype, &pyvalue, &pytrace); \
538+
auto error = CPyCppyy::Utility::FetchPyError(); \
540539
ctypes_type = GetCTypesType(ct_##ctype); \
541-
PyErr_Restore(pytype, pyvalue, pytrace); \
540+
CPyCppyy::Utility::RestorePyError(error); \
542541
} \
543542
if (Py_TYPE(pyobject) == ctypes_type) { \
544543
PyErr_Clear(); \
@@ -1258,16 +1257,14 @@ bool CPyCppyy::CStringConverter::SetArg(
12581257
const char* cstr = CPyCppyy_PyText_AsStringAndSize(pyobject, &len);
12591258
if (!cstr) {
12601259
// special case: allow ctypes c_char_p
1261-
PyObject* pytype = 0, *pyvalue = 0, *pytrace = 0;
1262-
PyErr_Fetch(&pytype, &pyvalue, &pytrace);
1260+
auto error = CPyCppyy::Utility::FetchPyError();
12631261
if (Py_TYPE(pyobject) == GetCTypesType(ct_c_char_p)) {
12641262
SetLifeLine(ctxt->fPyContext, pyobject, (intptr_t)this);
12651263
para.fValue.fVoidp = (void*)((CPyCppyy_tagCDataObject*)pyobject)->b_ptr;
12661264
para.fTypeCode = 'V';
1267-
Py_XDECREF(pytype); Py_XDECREF(pyvalue); Py_XDECREF(pytrace);
12681265
return true;
12691266
}
1270-
PyErr_Restore(pytype, pyvalue, pytrace);
1267+
CPyCppyy::Utility::RestorePyError(error);
12711268
return false;
12721269
}
12731270

‎bindings/pyroot/cppyy/CPyCppyy/src/PyException.cxx‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,17 @@ CPyCppyy::PyException::PyException()
2828
PyGILState_STATE state = PyGILState_Ensure();
2929
#endif
3030

31+
#if PY_VERSION_HEX >= 0x030c0000
32+
PyObject *pyvalue = PyErr_GetRaisedException();
33+
PyObject *pytype = pyvalue ? (PyObject *)Py_TYPE(pyvalue) : nullptr;
34+
PyObject* traceback = pyvalue ? PyException_GetTraceback(pyvalue) : nullptr;
35+
#else
3136
PyObject* pytype = nullptr, *pyvalue = nullptr, *pytrace = nullptr;
3237
PyErr_Fetch(&pytype, &pyvalue, &pytrace);
38+
PyObject* traceback = pytrace; // to keep the original unchanged
39+
Py_XINCREF(traceback);
40+
#endif
41+
3342
if (pytype && pyvalue) {
3443
const char* tname = PyExceptionClass_Name(pytype);
3544
if (tname) {
@@ -46,9 +55,6 @@ CPyCppyy::PyException::PyException()
4655
}
4756
}
4857

49-
PyObject* traceback = pytrace; // to keep the original unchanged
50-
Py_XINCREF(traceback);
51-
5258
std::string locName;
5359
std::string locFile;
5460
int locLine = 0;
@@ -88,7 +94,11 @@ CPyCppyy::PyException::PyException()
8894

8995
Py_XDECREF(traceback);
9096

97+
#if PY_VERSION_HEX >= 0x030c0000
98+
PyErr_SetRaisedException(pyvalue);
99+
#else
91100
PyErr_Restore(pytype, pyvalue, pytrace);
101+
#endif
92102

93103
if (fMsg.empty())
94104
fMsg = "python exception";

‎bindings/pyroot/cppyy/CPyCppyy/src/TemplateProxy.cxx‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,7 @@ static int tpp_doc_set(TemplateProxy* pytmpl, PyObject *val, void *)
437437
//= CPyCppyy template proxy callable behavior ================================
438438

439439
#define TPPCALL_RETURN \
440-
{ if (!errors.empty()) \
441-
std::for_each(errors.begin(), errors.end(), Utility::PyError_t::Clear);\
440+
{ errors.clear(); \
442441
return result; }
443442

444443
static inline std::string targs2str(TemplateProxy* pytmpl)
@@ -645,7 +644,7 @@ static PyObject* tpp_call(TemplateProxy* pytmpl, PyObject* args, PyObject* kwds)
645644
PyObject* topmsg = CPyCppyy_PyText_FromFormat(
646645
"Could not find \"%s\" (set cppyy.set_debug() for C++ errors):", CPyCppyy_PyText_AsString(pyfullname));
647646
Py_DECREF(pyfullname);
648-
Utility::SetDetailedException(errors, topmsg /* steals */, PyExc_TypeError /* default error */);
647+
Utility::SetDetailedException(std::move(errors), topmsg /* steals */, PyExc_TypeError /* default error */);
649648

650649
return nullptr;
651650
}
@@ -686,7 +685,7 @@ static PyObject* tpp_call(TemplateProxy* pytmpl, PyObject* args, PyObject* kwds)
686685
// error reporting is fraud, given the numerous steps taken, but more details seems better
687686
if (!errors.empty()) {
688687
PyObject* topmsg = CPyCppyy_PyText_FromString("Template method resolution failed:");
689-
Utility::SetDetailedException(errors, topmsg /* steals */, PyExc_TypeError /* default error */);
688+
Utility::SetDetailedException(std::move(errors), topmsg /* steals */, PyExc_TypeError /* default error */);
690689
} else {
691690
PyErr_Format(PyExc_TypeError, "cannot resolve method template call for \'%s\'",
692691
pytmpl->fTI->fCppName.c_str());
@@ -853,17 +852,11 @@ static PyObject* tpp_overload(TemplateProxy* pytmpl, PyObject* args)
853852
}
854853

855854
// else attempt instantiation
856-
PyObject* pytype = 0, *pyvalue = 0, *pytrace = 0;
857-
PyErr_Fetch(&pytype, &pyvalue, &pytrace);
858-
859855
if (!cppmeth) {
860-
PyErr_Restore(pytype, pyvalue, pytrace);
861856
return nullptr;
862857
}
863858

864-
Py_XDECREF(pytype);
865-
Py_XDECREF(pyvalue);
866-
Py_XDECREF(pytrace);
859+
PyErr_Clear();
867860

868861
// TODO: the next step should be consolidated with Instantiate()
869862
PyCallable* meth = nullptr;

‎bindings/pyroot/cppyy/CPyCppyy/src/Utility.cxx‎

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -914,15 +914,14 @@ Py_ssize_t CPyCppyy::Utility::GetBuffer(PyObject* pyobject, char tc, int size, v
914914
buf = 0; // not compatible
915915

916916
// clarify error message
917-
PyObject* pytype = 0, *pyvalue = 0, *pytrace = 0;
918-
PyErr_Fetch(&pytype, &pyvalue, &pytrace);
917+
auto error = FetchPyError();
919918
PyObject* pyvalue2 = CPyCppyy_PyText_FromFormat(
920919
(char*)"%s and given element size (%ld) do not match needed (%d)",
921-
CPyCppyy_PyText_AsString(pyvalue),
920+
CPyCppyy_PyText_AsString(error.fValue.get()),
922921
seqmeths->sq_length ? (long)(buflen/(*(seqmeths->sq_length))(pyobject)) : (long)buflen,
923922
size);
924-
Py_DECREF(pyvalue);
925-
PyErr_Restore(pytype, pyvalue2, pytrace);
923+
error.fValue.reset(pyvalue2);
924+
RestorePyError(error);
926925
}
927926
}
928927

@@ -1079,20 +1078,50 @@ PyObject* CPyCppyy::Utility::PyErr_Occurred_WithGIL()
10791078
}
10801079

10811080

1081+
//----------------------------------------------------------------------------
1082+
CPyCppyy::Utility::PyError_t CPyCppyy::Utility::FetchPyError()
1083+
{
1084+
// create a PyError_t RAII object that will capture and store the exception data
1085+
CPyCppyy::Utility::PyError_t error{};
1086+
#if PY_VERSION_HEX >= 0x030c0000
1087+
error.fValue.reset(PyErr_GetRaisedException());
1088+
#else
1089+
PyObject *pytype = nullptr;
1090+
PyObject *pyvalue = nullptr;
1091+
PyObject *pytrace = nullptr;
1092+
PyErr_Fetch(&pytype, &pyvalue, &pytrace);
1093+
error.fType.reset(pytype);
1094+
error.fValue.reset(pyvalue);
1095+
error.fTrace.reset(pytrace);
1096+
#endif
1097+
return error;
1098+
}
1099+
1100+
1101+
//----------------------------------------------------------------------------
1102+
void CPyCppyy::Utility::RestorePyError(CPyCppyy::Utility::PyError_t &error)
1103+
{
1104+
#if PY_VERSION_HEX >= 0x030c0000
1105+
PyErr_SetRaisedException(error.fValue.release());
1106+
#else
1107+
PyErr_Restore(error.fType.release(), error.fValue.release(), error.fTrace.release());
1108+
#endif
1109+
}
1110+
1111+
10821112
//----------------------------------------------------------------------------
10831113
size_t CPyCppyy::Utility::FetchError(std::vector<PyError_t>& errors, bool is_cpp)
10841114
{
10851115
// Fetch the current python error, if any, and store it for future use.
10861116
if (PyErr_Occurred()) {
1087-
PyError_t e{is_cpp};
1088-
PyErr_Fetch(&e.fType, &e.fValue, &e.fTrace);
1089-
errors.push_back(e);
1117+
errors.emplace_back(FetchPyError());
1118+
errors.back().fIsCpp = is_cpp;
10901119
}
10911120
return errors.size();
10921121
}
10931122

10941123
//----------------------------------------------------------------------------
1095-
void CPyCppyy::Utility::SetDetailedException(std::vector<PyError_t>& errors, PyObject* topmsg, PyObject* defexc)
1124+
void CPyCppyy::Utility::SetDetailedException(std::vector<PyError_t>&& errors, PyObject* topmsg, PyObject* defexc)
10961125
{
10971126
// Use the collected exceptions to build up a detailed error log.
10981127
if (errors.empty()) {
@@ -1123,14 +1152,18 @@ void CPyCppyy::Utility::SetDetailedException(std::vector<PyError_t>& errors, PyO
11231152

11241153
// bind the original C++ object, rather than constructing from topmsg, as it
11251154
// is expected to have informative state
1126-
Py_INCREF(unique_from_cpp->fType); Py_INCREF(unique_from_cpp->fValue); Py_XINCREF(unique_from_cpp->fTrace);
1127-
PyErr_Restore(unique_from_cpp->fType, unique_from_cpp->fValue, unique_from_cpp->fTrace);
1155+
RestorePyError(*unique_from_cpp);
11281156
} else {
11291157
// try to consolidate Python exceptions, otherwise select default
11301158
PyObject* exc_type = nullptr;
11311159
for (auto& e : errors) {
1132-
if (!exc_type) exc_type = e.fType;
1133-
else if (exc_type != e.fType) {
1160+
#if PY_VERSION_HEX >= 0x030c0000
1161+
PyObject* pytype = (PyObject*)Py_TYPE(e.fValue.get());
1162+
#else
1163+
PyObject* pytype = e.fType.get();
1164+
#endif
1165+
if (!exc_type) exc_type = pytype;
1166+
else if (exc_type != pytype) {
11341167
exc_type = defexc;
11351168
break;
11361169
}
@@ -1139,14 +1172,15 @@ void CPyCppyy::Utility::SetDetailedException(std::vector<PyError_t>& errors, PyO
11391172
// add the details to the topmsg
11401173
PyObject* separator = CPyCppyy_PyText_FromString("\n ");
11411174
for (auto& e : errors) {
1175+
PyObject *pyvalue = e.fValue.get();
11421176
CPyCppyy_PyText_Append(&topmsg, separator);
1143-
if (CPyCppyy_PyText_Check(e.fValue)) {
1144-
CPyCppyy_PyText_Append(&topmsg, e.fValue);
1145-
} else if (e.fValue) {
1146-
PyObject* excstr = PyObject_Str(e.fValue);
1177+
if (CPyCppyy_PyText_Check(pyvalue)) {
1178+
CPyCppyy_PyText_Append(&topmsg, pyvalue);
1179+
} else if (pyvalue) {
1180+
PyObject* excstr = PyObject_Str(pyvalue);
11471181
if (!excstr) {
11481182
PyErr_Clear();
1149-
excstr = PyObject_Str((PyObject*)Py_TYPE(e.fValue));
1183+
excstr = PyObject_Str((PyObject*)Py_TYPE(pyvalue));
11501184
}
11511185
CPyCppyy_PyText_AppendAndDel(&topmsg, excstr);
11521186
} else {
@@ -1161,8 +1195,6 @@ void CPyCppyy::Utility::SetDetailedException(std::vector<PyError_t>& errors, PyO
11611195
PyErr_SetString(exc_type, CPyCppyy_PyText_AsString(topmsg));
11621196
}
11631197

1164-
// cleanup stored errors and done with topmsg (whether used or not)
1165-
std::for_each(errors.begin(), errors.end(), PyError_t::Clear);
11661198
Py_DECREF(topmsg);
11671199
}
11681200

‎bindings/pyroot/cppyy/CPyCppyy/src/Utility.h‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
// Standard
55
#include <map>
6+
#include <memory>
67
#include <string>
78
#include <vector>
89

@@ -85,22 +86,23 @@ PyObject* PyErr_Occurred_WithGIL();
8586

8687
// helpers for collecting/maintaining python exception data
8788
struct PyError_t {
88-
PyError_t(bool is_cpp = false) : fIsCpp(is_cpp) { fType = fValue = fTrace = 0; }
89-
90-
static void Clear(PyError_t& e)
91-
{
92-
// Remove exception information.
93-
Py_XDECREF(e.fType); Py_XDECREF(e.fValue); Py_XDECREF(e.fTrace);
94-
e.fType = e.fValue = e.fTrace = 0;
95-
}
96-
97-
PyObject *fType, *fValue, *fTrace;
98-
bool fIsCpp;
89+
struct PyObjectDeleter {
90+
void operator()(PyObject *obj) { Py_XDECREF(obj); }
91+
};
92+
#if PY_VERSION_HEX < 0x030c0000
93+
std::unique_ptr<PyObject, PyObjectDeleter> fType;
94+
std::unique_ptr<PyObject, PyObjectDeleter> fTrace;
95+
#endif
96+
std::unique_ptr<PyObject, PyObjectDeleter> fValue;
97+
bool fIsCpp = false;
9998
};
10099

100+
PyError_t FetchPyError();
101+
void RestorePyError(PyError_t &error);
102+
101103
size_t FetchError(std::vector<PyError_t>&, bool is_cpp = false);
102104
void SetDetailedException(
103-
std::vector<PyError_t>& errors /* clears */, PyObject* topmsg /* steals ref */, PyObject* defexc);
105+
std::vector<PyError_t>&& errors /* clears */, PyObject* topmsg /* steals ref */, PyObject* defexc);
104106

105107
// setup Python API for callbacks
106108
bool IncludePython();

0 commit comments

Comments
 (0)