Skip to content

Commit f687d1d

Browse files
committed
[CPyCppyy] Correctly propagate template argument name errors
In case the `Utility::ConstructTemplateArgs()` method fails to create the C++ template names, it sets a useful Python exception that is unfortunately not propagated, because the calling code in `TemplateProxy.cxx` is ignoring it. This is fixed in this commit. The place where the error is set is also moved from `Utility::ConstructTemplateArgs()` to the actual implementation: `AddTypeName()`. This means the error is set by the function that actually has the context of the error, which is conceptually cleaner and will allow us also to set more detailed errors for different failure modes of `AddTypeName()`. A unit test is also implemented. Demo code: ```python import cppyy cppyy.cppdef(""" template<class T> class Foo {}; template<class T, class Y=void> auto make_foo () { return Foo<T>{}; } template<class T, class Y=void> auto make_foo (T x) { return Foo<T>{}; } """) cppyy.gbl.make_foo[set(), list()]() ``` Before: ``` cppyy.gbl.make_foo[set(), list()]() ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^ TypeError: Could not find "make_foo" (set cppyy.set_debug() for C++ errors): Template method resolution failed: Failed to instantiate "make_foo()" Failed to instantiate "make_foo()" ``` After: ``` cppyy.gbl.make_foo[set(), list()]() ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^ TypeError: could not construct C++ name from template argument set() ``` Note: this work is orthogonal to the rebase of `cppyy` on CppInterOp.
1 parent 24fe57c commit f687d1d

File tree

3 files changed

+49
-3
lines changed

3 files changed

+49
-3
lines changed

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,13 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname,
173173

174174
Py_DECREF(pyargs);
175175
Py_DECREF(tpArgs);
176+
177+
// Propagate the error that occurs if we can't construct the C++ name
178+
// from the provided template argument
179+
if (PyErr_Occurred()) {
180+
return nullptr;
181+
}
182+
176183
if (name_v1.size())
177184
proto = name_v1.substr(1, name_v1.size()-2);
178185
}
@@ -720,6 +727,11 @@ static PyObject* tpp_subscript(TemplateProxy* pytmpl, PyObject* args)
720727
Py_XDECREF(typeBoundMethod->fTemplateArgs);
721728
typeBoundMethod->fTemplateArgs = CPyCppyy_PyText_FromString(
722729
Utility::ConstructTemplateArgs(nullptr, args).c_str());
730+
// Propagate the error that occurs if we can't construct the C++ name
731+
// from the provided template argument
732+
if (PyErr_Occurred()) {
733+
return nullptr;
734+
}
723735
return (PyObject*)typeBoundMethod;
724736
}
725737

@@ -815,6 +827,11 @@ static PyObject* tpp_overload(TemplateProxy* pytmpl, PyObject* args)
815827
if (ol) return ol;
816828

817829
proto = Utility::ConstructTemplateArgs(nullptr, args);
830+
// Propagate the error that occurs if we can't construct the C++ name
831+
// from the provided template argument
832+
if (PyErr_Occurred()) {
833+
return nullptr;
834+
}
818835

819836
scope = ((CPPClass*)pytmpl->fTI->fPyClass)->fCppType;
820837
cppmeth = Cppyy::GetMethodTemplate(

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -477,11 +477,17 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg,
477477
std::string subtype{"std::initializer_list<"};
478478
PyObject* item = PySequence_GetItem(arg, 0);
479479
ArgPreference subpref = pref == kValue ? kValue : kPointer;
480-
if (AddTypeName(subtype, (PyObject*)Py_TYPE(item), item, subpref)) {
480+
bool ret = AddTypeName(subtype, (PyObject*)Py_TYPE(item), item, subpref);
481+
if (ret) {
481482
tmpl_name.append(subtype);
482483
tmpl_name.append(">");
483484
}
484485
Py_DECREF(item);
486+
// Error occurred in inner call to AddTypeName, which means it has
487+
// also set a TypeError. We return and let the error propagate.
488+
if (!ret) {
489+
return false;
490+
}
485491
}
486492

487493
return true;
@@ -615,6 +621,15 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg,
615621
return true;
616622
}
617623

624+
// Give up with a TypeError
625+
626+
// Try to get a readable representation of the argument
627+
PyObject *repr = PyObject_Repr(tn);
628+
const char *repr_cstr = repr ? PyUnicode_AsUTF8(repr) : "<unprintable>";
629+
630+
PyErr_Format(PyExc_TypeError, "could not construct C++ name from template argument %s", repr_cstr);
631+
632+
Py_XDECREF(repr);
618633
return false;
619634
}
620635

@@ -644,8 +659,6 @@ std::string CPyCppyy::Utility::ConstructTemplateArgs(
644659
// __cpp_name__ and/or __name__ is rather expensive)
645660
} else {
646661
if (!AddTypeName(tmpl_name, tn, (args ? PyTuple_GET_ITEM(args, i) : nullptr), pref, pcnt)) {
647-
PyErr_SetString(PyExc_SyntaxError,
648-
"could not construct C++ name from provided template argument.");
649662
return "";
650663
}
651664
}

bindings/pyroot/cppyy/cppyy/test/test_templates.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,6 +1177,22 @@ def test34_cstring_template_argument(self):
11771177
assert ns.stringify["const char*"]("Aap") == "Aap "
11781178
assert ns.stringify(ctypes.c_char_p(bytes("Noot", "ascii"))) == "Noot "
11791179

1180+
def test35_no_possible_cpp_name(self):
1181+
"""Verify that we get a meaningful error if the C++ name can't be
1182+
constructed, e.g. because one attempts to use a Python object that
1183+
doens't map to a C++ type.
1184+
"""
1185+
1186+
import cppyy
1187+
1188+
cppyy.cppdef(r"""
1189+
template<class T, class Y=void>
1190+
void test35_func () { }
1191+
""")
1192+
1193+
with pytest.raises(TypeError, match=r"could not construct C\+\+ name"):
1194+
cppyy.gbl.test35_func[set(), list()]()
1195+
11801196

11811197
class TestTEMPLATED_TYPEDEFS:
11821198
def setup_class(cls):

0 commit comments

Comments
 (0)