From f881e260b889e72cdcfa403957eb00025043044e Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 6 Feb 2017 13:14:48 -0500 Subject: [PATCH 1/3] Fix debugging output for nameless py::arg annotations This fixes a couple bugs with nameless py::arg() (introduced in #634) annotations: - the argument name was being used in debug mode without checking that it exists (which would result in the std::string construction throwing an exception for being invoked with a nullptr) - the error output says "keyword arguments", but py::arg_v() can now also be used for positional argument defaults. - the debugging output "in function named 'blah'" was overly verbose: changed it to just "in function 'blah'". --- include/pybind11/attr.h | 10 ++++++---- tests/test_issues.cpp | 19 ++++++++++++++++++- tests/test_issues.py | 24 ++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 129db05486..da3ac624f8 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -303,19 +303,21 @@ template <> struct process_attribute : process_attribute_default { if (!a.value) { #if !defined(NDEBUG) - auto descr = "'" + std::string(a.name) + ": " + a.type + "'"; + std::string descr("'"); + if (a.name) descr += std::string(a.name) + ": "; + descr += a.type + "'"; if (r->is_method) { if (r->name) descr += " in method '" + (std::string) str(r->scope) + "." + (std::string) r->name + "'"; else descr += " in method of '" + (std::string) str(r->scope) + "'"; } else if (r->name) { - descr += " in function named '" + (std::string) r->name + "'"; + descr += " in function '" + (std::string) r->name + "'"; } - pybind11_fail("arg(): could not convert default keyword argument " + pybind11_fail("arg(): could not convert default argument " + descr + " into a Python object (type not registered yet?)"); #else - pybind11_fail("arg(): could not convert default keyword argument " + pybind11_fail("arg(): could not convert default argument " "into a Python object (type not registered yet?). " "Compile in debug mode for more information."); #endif diff --git a/tests/test_issues.cpp b/tests/test_issues.cpp index 4c59a1b122..ebfd9f7751 100644 --- a/tests/test_issues.cpp +++ b/tests/test_issues.cpp @@ -74,6 +74,8 @@ namespace std { template <> struct hash { size_t operator()(const TplConstrClass &t) const { return std::hash()(t.str); } }; } +/// Issue/PR #648: bad arg default debugging output +class NotRegistered {}; void init_issues(py::module &m) { py::module m2 = m.def_submodule("issues"); @@ -395,7 +397,22 @@ void init_issues(py::module &m) { #elif defined(PYBIND11_HAS_EXP_OPTIONAL) m2.def("tpl_constr_optional", [](std::experimental::optional &) {}); #endif + + /// Issue/PR #648: bad arg default debugging output +#if !defined(NDEBUG) + m2.attr("debug_enabled") = true; +#else + m2.attr("debug_enabled") = false; +#endif + m2.def("bad_arg_def_named", []{ + auto m = py::module::import("pybind11_tests.issues"); + m.def("should_fail", [](int, NotRegistered) {}, py::arg(), py::arg("a") = NotRegistered()); + }); + m2.def("bad_arg_def_unnamed", []{ + auto m = py::module::import("pybind11_tests.issues"); + m.def("should_fail", [](int, NotRegistered) {}, py::arg(), py::arg() = NotRegistered()); + }); } -// MSVC workaround: trying to use a lambda here crashes MSCV +// MSVC workaround: trying to use a lambda here crashes MSVC test_initializer issues(&init_issues); diff --git a/tests/test_issues.py b/tests/test_issues.py index e60b5ca907..4fe0f19300 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -249,3 +249,27 @@ def test_inheritance_override_def_static(): assert isinstance(b, MyBase) assert isinstance(d1, MyDerived) assert isinstance(d2, MyDerived) + + +def test_bad_arg_default(msg): + from pybind11_tests.issues import debug_enabled, bad_arg_def_named, bad_arg_def_unnamed + + with pytest.raises(RuntimeError) as excinfo: + bad_arg_def_named() + assert msg(excinfo.value) == ( + "arg(): could not convert default argument 'a: NotRegistered' in function 'should_fail'" + "into a Python object (type not registered yet?)" + if debug_enabled else + "arg(): could not convert default argument into a Python object (type not registered " + "yet?). Compile in debug mode for more information." + ) + + with pytest.raises(RuntimeError) as excinfo: + bad_arg_def_unnamed() + assert msg(excinfo.value) == ( + "arg(): could not convert default argument 'NotRegistered' in function 'should_fail'" + "into a Python object (type not registered yet?)" + if debug_enabled else + "arg(): could not convert default argument into a Python object (type not registered " + "yet?). Compile in debug mode for more information." + ) From d6c0923e77f45cc6f653d248b9e37718b1ec4dcf Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 6 Feb 2017 20:15:06 -0500 Subject: [PATCH 2/3] Fix missing space in debug test string --- tests/test_issues.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_issues.py b/tests/test_issues.py index 4fe0f19300..f58cbfdd1b 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -257,7 +257,7 @@ def test_bad_arg_default(msg): with pytest.raises(RuntimeError) as excinfo: bad_arg_def_named() assert msg(excinfo.value) == ( - "arg(): could not convert default argument 'a: NotRegistered' in function 'should_fail'" + "arg(): could not convert default argument 'a: NotRegistered' in function 'should_fail' " "into a Python object (type not registered yet?)" if debug_enabled else "arg(): could not convert default argument into a Python object (type not registered " @@ -267,7 +267,7 @@ def test_bad_arg_default(msg): with pytest.raises(RuntimeError) as excinfo: bad_arg_def_unnamed() assert msg(excinfo.value) == ( - "arg(): could not convert default argument 'NotRegistered' in function 'should_fail'" + "arg(): could not convert default argument 'NotRegistered' in function 'should_fail' " "into a Python object (type not registered yet?)" if debug_enabled else "arg(): could not convert default argument into a Python object (type not registered " From 499caaf8aff42416f60a6c12f71b3b0ecfad2007 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Tue, 7 Feb 2017 11:49:33 -0500 Subject: [PATCH 3/3] Moved tests from issues to methods_and_attributes --- tests/test_issues.cpp | 18 ------------------ tests/test_issues.py | 24 ------------------------ tests/test_methods_and_attributes.cpp | 17 +++++++++++++++++ tests/test_methods_and_attributes.py | 24 ++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 42 deletions(-) diff --git a/tests/test_issues.cpp b/tests/test_issues.cpp index ebfd9f7751..7da3700456 100644 --- a/tests/test_issues.cpp +++ b/tests/test_issues.cpp @@ -74,9 +74,6 @@ namespace std { template <> struct hash { size_t operator()(const TplConstrClass &t) const { return std::hash()(t.str); } }; } -/// Issue/PR #648: bad arg default debugging output -class NotRegistered {}; - void init_issues(py::module &m) { py::module m2 = m.def_submodule("issues"); @@ -397,21 +394,6 @@ void init_issues(py::module &m) { #elif defined(PYBIND11_HAS_EXP_OPTIONAL) m2.def("tpl_constr_optional", [](std::experimental::optional &) {}); #endif - - /// Issue/PR #648: bad arg default debugging output -#if !defined(NDEBUG) - m2.attr("debug_enabled") = true; -#else - m2.attr("debug_enabled") = false; -#endif - m2.def("bad_arg_def_named", []{ - auto m = py::module::import("pybind11_tests.issues"); - m.def("should_fail", [](int, NotRegistered) {}, py::arg(), py::arg("a") = NotRegistered()); - }); - m2.def("bad_arg_def_unnamed", []{ - auto m = py::module::import("pybind11_tests.issues"); - m.def("should_fail", [](int, NotRegistered) {}, py::arg(), py::arg() = NotRegistered()); - }); } // MSVC workaround: trying to use a lambda here crashes MSVC diff --git a/tests/test_issues.py b/tests/test_issues.py index f58cbfdd1b..e60b5ca907 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -249,27 +249,3 @@ def test_inheritance_override_def_static(): assert isinstance(b, MyBase) assert isinstance(d1, MyDerived) assert isinstance(d2, MyDerived) - - -def test_bad_arg_default(msg): - from pybind11_tests.issues import debug_enabled, bad_arg_def_named, bad_arg_def_unnamed - - with pytest.raises(RuntimeError) as excinfo: - bad_arg_def_named() - assert msg(excinfo.value) == ( - "arg(): could not convert default argument 'a: NotRegistered' in function 'should_fail' " - "into a Python object (type not registered yet?)" - if debug_enabled else - "arg(): could not convert default argument into a Python object (type not registered " - "yet?). Compile in debug mode for more information." - ) - - with pytest.raises(RuntimeError) as excinfo: - bad_arg_def_unnamed() - assert msg(excinfo.value) == ( - "arg(): could not convert default argument 'NotRegistered' in function 'should_fail' " - "into a Python object (type not registered yet?)" - if debug_enabled else - "arg(): could not convert default argument into a Python object (type not registered " - "yet?). Compile in debug mode for more information." - ) diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp index e33e4ac397..5bccf49cf5 100644 --- a/tests/test_methods_and_attributes.cpp +++ b/tests/test_methods_and_attributes.cpp @@ -150,6 +150,9 @@ template <> struct type_caster { }; }} +/// Issue/PR #648: bad arg default debugging output +class NotRegistered {}; + test_initializer methods_and_attributes([](py::module &m) { py::class_(m, "ExampleMandA") .def(py::init<>()) @@ -270,4 +273,18 @@ test_initializer methods_and_attributes([](py::module &m) { m.def("floats_preferred", [](double f) { return 0.5 * f; }, py::arg("f")); m.def("floats_only", [](double f) { return 0.5 * f; }, py::arg("f").noconvert()); + /// Issue/PR #648: bad arg default debugging output +#if !defined(NDEBUG) + m.attr("debug_enabled") = true; +#else + m.attr("debug_enabled") = false; +#endif + m.def("bad_arg_def_named", []{ + auto m = py::module::import("pybind11_tests.issues"); + m.def("should_fail", [](int, NotRegistered) {}, py::arg(), py::arg("a") = NotRegistered()); + }); + m.def("bad_arg_def_unnamed", []{ + auto m = py::module::import("pybind11_tests.issues"); + m.def("should_fail", [](int, NotRegistered) {}, py::arg(), py::arg() = NotRegistered()); + }); }); diff --git a/tests/test_methods_and_attributes.py b/tests/test_methods_and_attributes.py index 3a5d2e1985..1ea669a430 100644 --- a/tests/test_methods_and_attributes.py +++ b/tests/test_methods_and_attributes.py @@ -255,3 +255,27 @@ def test_noconvert_args(msg): Invoked with: 4 """ + + +def test_bad_arg_default(msg): + from pybind11_tests import debug_enabled, bad_arg_def_named, bad_arg_def_unnamed + + with pytest.raises(RuntimeError) as excinfo: + bad_arg_def_named() + assert msg(excinfo.value) == ( + "arg(): could not convert default argument 'a: NotRegistered' in function 'should_fail' " + "into a Python object (type not registered yet?)" + if debug_enabled else + "arg(): could not convert default argument into a Python object (type not registered " + "yet?). Compile in debug mode for more information." + ) + + with pytest.raises(RuntimeError) as excinfo: + bad_arg_def_unnamed() + assert msg(excinfo.value) == ( + "arg(): could not convert default argument 'NotRegistered' in function 'should_fail' " + "into a Python object (type not registered yet?)" + if debug_enabled else + "arg(): could not convert default argument into a Python object (type not registered " + "yet?). Compile in debug mode for more information." + )