Skip to content

Commit b6875e0

Browse files
committed
[CPyCppyy] Fix improper warning handling and support -W error
Replace deprecated [1] `PyErr_Warn()` with `PyErr_WarnEx()` and correctly handle its return value throughout the CPyCppyy codebase. Previously, warnings were emitted without checking for failure. When Python is run with `"-W error"` (or equivalent filters), warnings are promoted to exceptions, causing `PyErr_Warn*()` to return an error while leaving an exception set. The code would then continue execution and return a valid value, triggering errors such as: ``` SystemError: <blah ...> returned a result with an exception set ``` This change ensures that all warning calls: - use the non-deprecated `PyErr_WarnEx()` - properly propagate errors when warnings are turned into exceptions Additional fixes: - Update tests to explicitly control warning behavior using `warnings.catch_warnings()`, avoiding unintended interaction with global `"-W error"` settings - Adjust tests to expect exceptions when warnings are promoted to errors This makes CPyCppyy more compliant with CPython C API requirements and robust under strict warning configurations. [1] https://www.cs.auckland.ac.nz/references/python/2.7.3-docs/c-api/exceptions.html?utm_source=chatgpt.com#PyErr_Warn
1 parent c5377c7 commit b6875e0

7 files changed

Lines changed: 50 additions & 28 deletions

File tree

bindings/pyroot/cppyy/CPyCppyy/src/CPyCppyyModule.cxx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,8 @@ static PyObject* SetCppLazyLookup(PyObject*, PyObject* args)
439439
#else
440440
// As of py3.11, there is no longer a lookup function pointer in the dict object
441441
// to replace. Since this feature is not widely advertised, it's simply dropped
442-
PyErr_Warn(PyExc_RuntimeWarning, (char*)"lazy lookup is no longer supported");
442+
if (PyErr_WarnEx(PyExc_RuntimeWarning, (char*)"lazy lookup is no longer supported", 1) < 0)
443+
return nullptr;
443444
(void)args; // avoid warning about unused parameter
444445
#endif
445446

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,7 +1304,8 @@ bool CPyCppyy::CStringConverter::SetArg(
13041304

13051305
// verify (too long string will cause truncation, no crash)
13061306
if (fMaxSize != std::string::npos && fMaxSize < fBuffer.size())
1307-
PyErr_Warn(PyExc_RuntimeWarning, (char*)"string too long for char array (truncated)");
1307+
if (PyErr_WarnEx(PyExc_RuntimeWarning, (char*)"string too long for char array (truncated)", 1) < 0)
1308+
return false;
13081309

13091310
if (!ctxt->fPyContext) {
13101311
// use internal buffer as workaround
@@ -1349,7 +1350,8 @@ bool CPyCppyy::CStringConverter::ToMemory(PyObject* value, void* address, PyObje
13491350

13501351
// verify (too long string will cause truncation, no crash)
13511352
if (fMaxSize != std::string::npos && fMaxSize < (std::string::size_type)len)
1352-
PyErr_Warn(PyExc_RuntimeWarning, (char*)"string too long for char array (truncated)");
1353+
if (PyErr_WarnEx(PyExc_RuntimeWarning, (char*)"string too long for char array (truncated)", 1) < 0)
1354+
return false;
13531355

13541356
// if address is available, and it wasn't set by this converter, assume a byte-wise copy;
13551357
// otherwise assume a pointer copy (this relies on the converter to be used for properties,
@@ -1426,7 +1428,8 @@ bool CPyCppyy::WCStringConverter::ToMemory(PyObject* value, void* address, PyObj
14261428

14271429
// verify (too long string will cause truncation, no crash)
14281430
if (fMaxSize != std::wstring::npos && fMaxSize < (std::wstring::size_type)len)
1429-
PyErr_Warn(PyExc_RuntimeWarning, (char*)"string too long for wchar_t array (truncated)");
1431+
if (PyErr_WarnEx(PyExc_RuntimeWarning, (char*)"string too long for wchar_t array (truncated)", 1) < 0)
1432+
return false;
14301433

14311434
Py_ssize_t res = -1;
14321435
if (fMaxSize != std::wstring::npos)
@@ -1485,7 +1488,10 @@ bool CPyCppyy::name##Converter::ToMemory(PyObject* value, void* address, PyObjec
14851488
\
14861489
/* verify (too long string will cause truncation, no crash) */ \
14871490
if (fMaxSize != std::wstring::npos && maxbytes < len) { \
1488-
PyErr_Warn(PyExc_RuntimeWarning, (char*)"string too long for "#type" array (truncated)");\
1491+
if (PyErr_WarnEx(PyExc_RuntimeWarning, (char*)"string too long for "#type" array (truncated)", 1) < 0) { \
1492+
Py_DECREF(bstr); \
1493+
return false; \
1494+
} \
14891495
len = maxbytes; \
14901496
} \
14911497
\

bindings/pyroot/cppyy/CPyCppyy/src/Dispatcher.cxx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ bool CPyCppyy::InsertDispatcher(CPPScope* klass, PyObject* bases, PyObject* dct,
203203

204204
if (!Cppyy::HasVirtualDestructor(basetype)) {
205205
const std::string& bname = Cppyy::GetScopedFinalName(basetype);
206-
PyErr_Warn(PyExc_RuntimeWarning, (char*)("class \""+bname+"\" has no virtual destructor").c_str());
206+
if (PyErr_WarnEx(PyExc_RuntimeWarning, (char*)("class \""+bname+"\" has no virtual destructor").c_str(), 1) < 0)
207+
return false;
207208
}
208209

209210
base_infos.emplace_back(

bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1703,7 +1703,7 @@ ptrdiff_t Cppyy::GetBaseOffset(TCppType_t derived, TCppType_t base,
17031703
std::ostringstream msg;
17041704
msg << "failed offset calculation between " << cb->GetName() << " and " << cd->GetName();
17051705
// TODO: propagate this warning to caller w/o use of Python C-API
1706-
// PyErr_Warn(PyExc_RuntimeWarning, const_cast<char*>(msg.str().c_str()));
1706+
// PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(msg.str().c_str()), 1);
17071707
std::cerr << "Warning: " << msg.str() << '\n';
17081708
}
17091709

bindings/pyroot/cppyy/cppyy/test/test_crossinheritance.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,12 @@ class MyClass4 {
448448
# as the C++ side now carries the type of the dispatcher, not the type of
449449
# the direct base class
450450
with warnings.catch_warnings(record=True) as w:
451+
# ensure warnings are not turned into errors, even if we run python -W error
452+
# The reason why we don't turn warnings into erros instead an just catch the exception:
453+
# The error would be changed into a more uninformative
454+
# "TypeError: no python-side overrides supported ()"
455+
warnings.simplefilter("default")
456+
451457
class MyPyDerived1(VD.MyClass1):
452458
pass # TODO: verify warning is given
453459
assert len(w) == 1
@@ -1139,7 +1145,7 @@ def __init__(self):
11391145
def test26_no_default_ctor(self):
11401146
"""Make sure no default ctor is created if not viable"""
11411147

1142-
import cppyy, warnings
1148+
import cppyy
11431149

11441150
cppyy.cppdef("""namespace no_default_ctor {
11451151
struct NoDefCtor1 {
@@ -1158,7 +1164,11 @@ class NoDefCtor3 {
11581164
virtual ~NoDefCtor3() {}
11591165
};
11601166
1161-
class Simple {}; }""")
1167+
class Simple {
1168+
public:
1169+
virtual ~Simple() {}
1170+
};
1171+
}""")
11621172

11631173
ns = cppyy.gbl.no_default_ctor
11641174

@@ -1170,18 +1180,16 @@ def __init__(self):
11701180
with raises(TypeError):
11711181
PyDerived()
11721182

1173-
with warnings.catch_warnings(record=True) as w:
1174-
class PyDerived(cppyy.multi(kls, ns.Simple)):
1175-
def __init__(self):
1176-
super(PyDerived, self).__init__()
1183+
class PyDerived(cppyy.multi(kls, ns.Simple)):
1184+
def __init__(self):
1185+
super(PyDerived, self).__init__()
11771186

11781187
with raises(TypeError):
11791188
PyDerived()
11801189

1181-
with warnings.catch_warnings(record=True) as w:
1182-
class PyDerived(cppyy.multi(ns.Simple, kls)):
1183-
def __init__(self):
1184-
super(PyDerived, self).__init__()
1190+
class PyDerived(cppyy.multi(ns.Simple, kls)):
1191+
def __init__(self):
1192+
super(PyDerived, self).__init__()
11851193

11861194
with raises(TypeError):
11871195
PyDerived()

bindings/pyroot/cppyy/cppyy/test/test_fragile.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -604,16 +604,18 @@ def test25_cppdef_error_reporting(self):
604604
int add42(int i) { return i + 42; }
605605
}""")
606606

607-
with warnings.catch_warnings(record=True) as w:
607+
# isolate the warning configuration
608+
with warnings.catch_warnings():
609+
warnings.simplefilter("error") # turn warnings into errors
610+
608611
# missing return statement
609-
cppyy.cppdef("""\
610-
namespace fragile {
611-
double add42d(double d) { d + 42.; return d; }
612-
}""")
612+
with pytest.raises(SyntaxWarning) as exc:
613+
cppyy.cppdef("""\
614+
namespace fragile {
615+
double add42d(double d) { d + 42.; return d; }
616+
}""")
613617

614-
assert len(w) == 1
615-
assert issubclass(w[-1].category, SyntaxWarning)
616-
assert "return" in str(w[-1].message)
618+
assert "return" in str(exc.value)
617619

618620
# mix of error and warning
619621
with raises(SyntaxError):

bindings/pyroot/cppyy/cppyy/test/test_regression.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,9 +1114,13 @@ def test38_char16_arrays(self):
11141114
len(ai.name) == 6
11151115
assert ai.name[:len(s)] == s
11161116

1117-
with warnings.catch_warnings(record=True) as w:
1118-
ai.name = u'hellowd'
1119-
assert 'too long' in str(w[-1].message)
1117+
# isolate the warning configuration
1118+
with warnings.catch_warnings():
1119+
warnings.simplefilter("error") # turn warnings into errors
1120+
1121+
with pytest.raises(RuntimeWarning) as exc:
1122+
ai.name = u'hellowd'
1123+
assert 'too long' in str(exc.value)
11201124

11211125
# vector of objects
11221126
va = cppyy.gbl.std.vector[ns.AxisInformation](N)

0 commit comments

Comments
 (0)