From aa3b9c6f08eda39919b4c33d57bb867da9588c27 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Fri, 11 Mar 2022 08:50:28 -0600 Subject: [PATCH 1/7] PYBIND11_OBJECT_CVT should use namespace for error_already_set() This change makes the macro usable outside of pybind11 namespace. --- include/pybind11/pytypes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index dc753d32cd..e54941485b 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1038,12 +1038,12 @@ public: Name(const object &o) \ : Parent(check_(o) ? o.inc_ref().ptr() : ConvertFun(o.ptr()), stolen_t{}) { \ if (!m_ptr) \ - throw error_already_set(); \ + throw ::pybind11::error_already_set(); \ } \ /* NOLINTNEXTLINE(google-explicit-constructor) */ \ Name(object &&o) : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) { \ if (!m_ptr) \ - throw error_already_set(); \ + throw ::pybind11::error_already_set(); \ } #define PYBIND11_OBJECT_CVT_DEFAULT(Name, Parent, CheckFun, ConvertFun) \ From 263c88156b27f69d0edd876ccb6e0ee6d6262aef Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Fri, 11 Mar 2022 12:48:44 -0600 Subject: [PATCH 2/7] added test for use of PYBIND11_OBJECT_CVT for classes in external to pybind11 namespaces --- tests/test_pytypes.cpp | 34 ++++++++++++++++++++++++++++++++++ tests/test_pytypes.py | 7 +++++++ 2 files changed, 41 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 1ed237ea21..2f0228bc51 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -11,6 +11,38 @@ #include + + +namespace external { + namespace detail { + bool check(PyObject *o) { return PyFloat_Check(o) != 0; } + + PyObject *conv(PyObject *o) { + if (PyLong_Check(o)) { + double v = PyLong_AsDouble(o); + if (v == -1.0 && PyErr_Occurred()) { + return nullptr; + } + return PyFloat_FromDouble(v); + } else { + PyErr_SetString(PyExc_TypeError, "Unexpected type"); + return nullptr; + } + } + + PyObject *default_constructed() { + return PyFloat_FromDouble(0.0); + } + } + class float_ : public py::object { + PYBIND11_OBJECT_CVT(float_, py::object, ::detail::check, ::detail::conv) + + float_() : py::object(::detail::default_constructed(), borrowed_t{}) {} + + double get_value() const { return PyFloat_AsDouble(this->ptr()); } + } +} + TEST_SUBMODULE(pytypes, m) { // test_bool m.def("get_bool", [] { return py::bool_(false); }); @@ -545,4 +577,6 @@ TEST_SUBMODULE(pytypes, m) { py::detail::accessor_policies::tuple_item::set(o, (py::size_t) 0, s0); return o; }); + + m.def("square_float_", [](external::float_ x) -> double { double v = x.get_value(); return v*v; }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index becd1cc8ae..14d3bdfb2e 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -634,3 +634,10 @@ def test_implementation_details(): assert m.tuple_item_set_ssize_t() == ("emely", "edmond") assert m.tuple_item_get_size_t(tup) == 93 assert m.tuple_item_set_size_t() == ("candy", "cat") + + +def test_external_float_(): + r1 = m.square_float_(2.0) + assert r1 == 4.0 + r2 = m.square_float_(4) + assert r2 == 16.0 From e44607f25769b749529eeadc75208781d4db84b2 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Fri, 11 Mar 2022 13:00:43 -0600 Subject: [PATCH 3/7] Extended test_pytypes.cpp and test_pytest.py The added test defines a dummy function that takes a custom-defined class external::float_ that uses PYBIND11_OBJECT_CVT --- tests/test_pytypes.cpp | 6 +++--- tests/test_pytypes.py | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 2f0228bc51..58f447c26e 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -35,12 +35,12 @@ namespace external { } } class float_ : public py::object { - PYBIND11_OBJECT_CVT(float_, py::object, ::detail::check, ::detail::conv) + PYBIND11_OBJECT_CVT(float_, py::object, external::detail::check, external::detail::conv) - float_() : py::object(::detail::default_constructed(), borrowed_t{}) {} + float_() : py::object(external::detail::default_constructed(), borrowed_t{}) {} double get_value() const { return PyFloat_AsDouble(this->ptr()); } - } + }; } TEST_SUBMODULE(pytypes, m) { diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 14d3bdfb2e..85afb94232 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -639,5 +639,3 @@ def test_implementation_details(): def test_external_float_(): r1 = m.square_float_(2.0) assert r1 == 4.0 - r2 = m.square_float_(4) - assert r2 == 16.0 From bed59815163e30ca5c9387c3e6c2b654ced239c6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 11 Mar 2022 19:04:54 +0000 Subject: [PATCH 4/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_pytypes.cpp | 55 +++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 58f447c26e..8c62fb702d 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -11,37 +11,33 @@ #include - - namespace external { - namespace detail { - bool check(PyObject *o) { return PyFloat_Check(o) != 0; } - - PyObject *conv(PyObject *o) { - if (PyLong_Check(o)) { - double v = PyLong_AsDouble(o); - if (v == -1.0 && PyErr_Occurred()) { - return nullptr; - } - return PyFloat_FromDouble(v); - } else { - PyErr_SetString(PyExc_TypeError, "Unexpected type"); - return nullptr; - } - } - - PyObject *default_constructed() { - return PyFloat_FromDouble(0.0); - } +namespace detail { +bool check(PyObject *o) { return PyFloat_Check(o) != 0; } + +PyObject *conv(PyObject *o) { + if (PyLong_Check(o)) { + double v = PyLong_AsDouble(o); + if (v == -1.0 && PyErr_Occurred()) { + return nullptr; + } + return PyFloat_FromDouble(v); + } else { + PyErr_SetString(PyExc_TypeError, "Unexpected type"); + return nullptr; } - class float_ : public py::object { - PYBIND11_OBJECT_CVT(float_, py::object, external::detail::check, external::detail::conv) +} - float_() : py::object(external::detail::default_constructed(), borrowed_t{}) {} +PyObject *default_constructed() { return PyFloat_FromDouble(0.0); } +} // namespace detail +class float_ : public py::object { + PYBIND11_OBJECT_CVT(float_, py::object, external::detail::check, external::detail::conv) - double get_value() const { return PyFloat_AsDouble(this->ptr()); } - }; -} + float_() : py::object(external::detail::default_constructed(), borrowed_t{}) {} + + double get_value() const { return PyFloat_AsDouble(this->ptr()); } +}; +} // namespace external TEST_SUBMODULE(pytypes, m) { // test_bool @@ -578,5 +574,8 @@ TEST_SUBMODULE(pytypes, m) { return o; }); - m.def("square_float_", [](external::float_ x) -> double { double v = x.get_value(); return v*v; }); + m.def("square_float_", [](external::float_ x) -> double { + double v = x.get_value(); + return v * v; + }); } From 5b38d112cce83487ff575e31c7545fc90d668eef Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Fri, 11 Mar 2022 13:28:55 -0600 Subject: [PATCH 5/7] fixed issues pointed out by CI --- tests/test_pytypes.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 8c62fb702d..2dd6755614 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -16,16 +16,16 @@ namespace detail { bool check(PyObject *o) { return PyFloat_Check(o) != 0; } PyObject *conv(PyObject *o) { + PyObject *ret = nullptr; if (PyLong_Check(o)) { double v = PyLong_AsDouble(o); - if (v == -1.0 && PyErr_Occurred()) { - return nullptr; - } - return PyFloat_FromDouble(v); + if (!(v == -1.0 && PyErr_Occurred())) { + ret = PyFloat_FromDouble(v); + } } else { PyErr_SetString(PyExc_TypeError, "Unexpected type"); - return nullptr; } + return ret; } PyObject *default_constructed() { return PyFloat_FromDouble(0.0); } @@ -574,7 +574,7 @@ TEST_SUBMODULE(pytypes, m) { return o; }); - m.def("square_float_", [](external::float_ x) -> double { + m.def("square_float_", [](const external::float_ &x) -> double { double v = x.get_value(); return v * v; }); From ed90b83ec3da76e7aa04fcf55918f6bd653bf061 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 11 Mar 2022 19:33:12 +0000 Subject: [PATCH 6/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_pytypes.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 2dd6755614..9ed19c9b57 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -21,7 +21,7 @@ PyObject *conv(PyObject *o) { double v = PyLong_AsDouble(o); if (!(v == -1.0 && PyErr_Occurred())) { ret = PyFloat_FromDouble(v); - } + } } else { PyErr_SetString(PyExc_TypeError, "Unexpected type"); } From f7062b3ed1ff1503cd3567524c3e9d20823ac4ba Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Fri, 11 Mar 2022 13:35:45 -0600 Subject: [PATCH 7/7] fixed memory leak in default constructor --- tests/test_pytypes.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 9ed19c9b57..b859497b83 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -33,7 +33,7 @@ PyObject *default_constructed() { return PyFloat_FromDouble(0.0); } class float_ : public py::object { PYBIND11_OBJECT_CVT(float_, py::object, external::detail::check, external::detail::conv) - float_() : py::object(external::detail::default_constructed(), borrowed_t{}) {} + float_() : py::object(external::detail::default_constructed(), stolen_t{}) {} double get_value() const { return PyFloat_AsDouble(this->ptr()); } };