From d2a36d91d109758cf37e05690ef5f4f1904e5d37 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 29 May 2023 12:47:15 -0700 Subject: [PATCH 01/39] Add `test_pass_std_vector_int()`, `test_pass_std_set_int()` in test_stl --- tests/test_stl.cpp | 3 +++ tests/test_stl.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index d45465d681..483e394bea 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -548,4 +548,7 @@ TEST_SUBMODULE(stl, m) { []() { return new std::vector(4513); }, // Without explicitly specifying `take_ownership`, this function leaks. py::return_value_policy::take_ownership); + + m.def("pass_std_vector_int", [](const std::vector &vec_int) { return vec_int.size(); }); + m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 8a614f8b87..f5e3f6c957 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -379,3 +379,32 @@ def test_return_vector_bool_raw_ptr(): v = m.return_vector_bool_raw_ptr() assert isinstance(v, list) assert len(v) == 4513 + + +def test_pass_std_vector_int(): + fn = m.pass_std_vector_int + assert fn([1, 2]) == 2 + assert fn((1, 2)) == 2 + with pytest.raises(TypeError): + fn(set()) + with pytest.raises(TypeError): + fn({}) + with pytest.raises(TypeError): + fn({}.keys()) + with pytest.raises(TypeError): + fn(i for i in range(3)) + + +def test_pass_std_set_int(): + fn = m.pass_std_set_int + assert fn({1, 2}) == 2 + with pytest.raises(TypeError): + fn([1, 2]) + with pytest.raises(TypeError): + fn((1, 2)) + with pytest.raises(TypeError): + fn({}) + with pytest.raises(TypeError): + fn({}.keys()) + with pytest.raises(TypeError): + fn(i for i in range(3)) From 7d30378bdffa54dc05f8166adf8dcf045a57d526 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 30 May 2023 08:46:15 -0700 Subject: [PATCH 02/39] Change `list_caster` to also accept generator objects (`PyGen_Check(src.ptr()`). Note for completeness: This is a more conservative change than https://github.com/google/pywrapcc/pull/30042 --- include/pybind11/stl.h | 38 ++++++++++++++++++++++++++++---------- tests/test_stl.py | 13 +++++++++++-- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index f39f44f7c9..cad9bc66fc 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -166,13 +166,38 @@ struct list_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - if (!isinstance(src) || isinstance(src) || isinstance(src)) { + if (isinstance(src) || isinstance(src)) { return false; } - auto s = reinterpret_borrow(src); + if (isinstance(src)) { + return convert_elements(src, convert); + } + if (!convert) { + return false; + } + if (PyGen_Check(src.ptr())) { + // Designed to be behavior-equivalent to passing tuple(src) from Python: + // The conversion to a tuple will first exhaust the generator object, to ensure that + // the generator is not left in an unpredictable (to the caller) partially-consumed + // state. + assert(isinstance(src)); + return convert_elements(tuple(reinterpret_borrow(src)), convert); + } + return false; + } + +private: + template ::value, int> = 0> + void reserve_maybe(const sequence &s, Type *) { + value.reserve(s.size()); + } + void reserve_maybe(const sequence &, void *) {} + + bool convert_elements(handle seq, bool convert) { + auto s = reinterpret_borrow(seq); value.clear(); reserve_maybe(s, &value); - for (auto it : s) { + for (auto it : seq) { value_conv conv; if (!conv.load(it, convert)) { return false; @@ -182,13 +207,6 @@ struct list_caster { return true; } -private: - template ::value, int> = 0> - void reserve_maybe(const sequence &s, Type *) { - value.reserve(s.size()); - } - void reserve_maybe(const sequence &, void *) {} - public: template static handle cast(T &&src, return_value_policy policy, handle parent) { diff --git a/tests/test_stl.py b/tests/test_stl.py index f5e3f6c957..9e5e84e163 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -385,14 +385,13 @@ def test_pass_std_vector_int(): fn = m.pass_std_vector_int assert fn([1, 2]) == 2 assert fn((1, 2)) == 2 + assert fn(i for i in range(3)) == 3 with pytest.raises(TypeError): fn(set()) with pytest.raises(TypeError): fn({}) with pytest.raises(TypeError): fn({}.keys()) - with pytest.raises(TypeError): - fn(i for i in range(3)) def test_pass_std_set_int(): @@ -408,3 +407,13 @@ def test_pass_std_set_int(): fn({}.keys()) with pytest.raises(TypeError): fn(i for i in range(3)) + + +def test_list_caster_fully_consumes_generator_object(): + def gen_mix(): + yield from [1, 2.0, 3] + + gen_obj = gen_mix() + with pytest.raises(TypeError): + m.pass_std_vector_int(gen_obj) + assert not tuple(gen_obj) From 039d1b7c819b37df18d845b0a1f6a0e04147c9db Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 12:33:30 -0700 Subject: [PATCH 03/39] Drop in (currently unpublished) PyCLIF code, use in `list_caster`, adjust tests. --- include/pybind11/stl.h | 38 +++++++++++++++++++++++++++++++++++++- tests/test_stl.cpp | 3 +++ tests/test_stl.py | 14 ++++++++++---- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index cad9bc66fc..a040a3fe7d 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -35,6 +35,42 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) +// +// Begin: Code developed under https://github.com/google/clif/blob/main/clif/python/runtime.cc +// (currently unpublished) +// + +inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, + std::initializer_list tp_names) { + if (PyType_Check(obj)) { + return false; + } + const char *obj_tp_name = Py_TYPE(obj)->tp_name; + for (auto tp_name : tp_names) { + if (strcmp(obj_tp_name, tp_name) == 0) + return true; + } + return false; +} + +inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { + return PySequence_Check(obj) || PyGen_Check(obj) || PyAnySet_Check(obj) + || PyObjectIsInstanceWithOneOfTpNames( + obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); +} + +inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { + return PyAnySet_Check(obj) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); +} + +inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { + return PyDict_Check(obj) || (PyMapping_Check(obj) && PyObject_HasAttrString(obj, "items")); +} + +// +// End: Code developed under https://github.com/google/clif/blob/main/clif/python/runtime.cc +// + /// Extracts an const lvalue reference or rvalue reference for U based on the type of T (e.g. for /// forwarding a container element). Typically used indirect via forwarded_type(), below. template @@ -175,7 +211,7 @@ struct list_caster { if (!convert) { return false; } - if (PyGen_Check(src.ptr())) { + if (PyObjectTypeIsConvertibleToStdVector(src.ptr())) { // Designed to be behavior-equivalent to passing tuple(src) from Python: // The conversion to a tuple will first exhaust the generator object, to ensure that // the generator is not left in an unpredictable (to the caller) partially-consumed diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 483e394bea..29e9c62327 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -550,5 +550,8 @@ TEST_SUBMODULE(stl, m) { py::return_value_policy::take_ownership); m.def("pass_std_vector_int", [](const std::vector &vec_int) { return vec_int.size(); }); + m.def("pass_std_vector_pair_int", [](const std::vector> &vec_pair_int) { + return vec_pair_int.size(); + }); m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 9e5e84e163..f25c1b7cd8 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -385,13 +385,19 @@ def test_pass_std_vector_int(): fn = m.pass_std_vector_int assert fn([1, 2]) == 2 assert fn((1, 2)) == 2 + assert fn({1, 2}) == 2 + assert fn({"x": 1, "y": 2}.values()) == 2 + assert fn({1: None, 2: None}.keys()) == 2 assert fn(i for i in range(3)) == 3 - with pytest.raises(TypeError): - fn(set()) + assert fn(map(lambda i: i, range(4))) == 4 # noqa: C417 with pytest.raises(TypeError): fn({}) - with pytest.raises(TypeError): - fn({}.keys()) + + +def test_pass_std_vector_pair_int(): + fn = m.pass_std_vector_pair_int + assert fn({1: 2, 3: 4}.items()) == 2 + assert fn(zip([1, 2], [3, 4])) == 2 def test_pass_std_set_int(): From bdba8570f92c848ea968606c70b4538c6088ffca Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 13:04:10 -0700 Subject: [PATCH 04/39] Use `PyObjectTypeIsConvertibleToStdSet()` in `set_caster`, adjust tests. --- include/pybind11/stl.h | 35 +++++++++++++++++++++++++---------- tests/test_stl.py | 3 +-- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index a040a3fe7d..6feac126f7 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -97,22 +97,18 @@ struct set_caster { private: template ::value, int> = 0> - void reserve_maybe(const anyset &s, Type *) { + void reserve_maybe(const sequence &s, Type *) { value.reserve(s.size()); } - void reserve_maybe(const anyset &, void *) {} + void reserve_maybe(const sequence &, void *) {} -public: - bool load(handle src, bool convert) { - if (!isinstance(src)) { - return false; - } - auto s = reinterpret_borrow(src); + bool convert_elements(handle seq, bool convert) { + auto s = reinterpret_borrow(seq); value.clear(); reserve_maybe(s, &value); - for (auto entry : s) { + for (auto it : seq) { key_conv conv; - if (!conv.load(entry, convert)) { + if (!conv.load(it, convert)) { return false; } value.insert(cast_op(std::move(conv))); @@ -120,6 +116,25 @@ struct set_caster { return true; } +public: + bool load(handle src, bool convert) { + if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) { + return false; + } + if (isinstance(src)) { + return convert_elements(src, convert); + } + if (!convert) { + return false; + } + // Designed to be behavior-equivalent to passing tuple(src) from Python: + // The conversion to a tuple will first exhaust the iterator object, to ensure that + // the iterator is not left in an unpredictable (to the caller) partially-consumed + // state. + assert(isinstance(src)); + return convert_elements(tuple(reinterpret_borrow(src)), convert); + } + template static handle cast(T &&src, return_value_policy policy, handle parent) { if (!std::is_lvalue_reference::value) { diff --git a/tests/test_stl.py b/tests/test_stl.py index f25c1b7cd8..b1bf4f3249 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -403,14 +403,13 @@ def test_pass_std_vector_pair_int(): def test_pass_std_set_int(): fn = m.pass_std_set_int assert fn({1, 2}) == 2 + assert fn({1: None, 2: None}.keys()) == 2 with pytest.raises(TypeError): fn([1, 2]) with pytest.raises(TypeError): fn((1, 2)) with pytest.raises(TypeError): fn({}) - with pytest.raises(TypeError): - fn({}.keys()) with pytest.raises(TypeError): fn(i for i in range(3)) From ec50ca0630bee3a20da1ef088f2f2931b47d08c8 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 13:10:48 -0700 Subject: [PATCH 05/39] Use `PyObjectTypeIsConvertibleToStdMap()` in `map_caster`, add tests. --- include/pybind11/stl.h | 28 ++++++++++++++++++++++------ tests/test_stl.cpp | 1 + tests/test_stl.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 6feac126f7..cad842ccfd 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -166,12 +166,7 @@ struct map_caster { } void reserve_maybe(const dict &, void *) {} -public: - bool load(handle src, bool convert) { - if (!isinstance(src)) { - return false; - } - auto d = reinterpret_borrow(src); + bool convert_elements(const dict &d, bool convert) { value.clear(); reserve_maybe(d, &value); for (auto it : d) { @@ -185,6 +180,27 @@ struct map_caster { return true; } +public: + bool load(handle src, bool convert) { + if (!PyObjectTypeIsConvertibleToStdMap(src.ptr())) { + return false; + } + if (isinstance(src)) { + return convert_elements(reinterpret_borrow(src), convert); + } + if (!convert) { + return false; + } + // Designed to be behavior-equivalent to passing dict(src.items()) from Python: + // The conversion to a dict will first exhaust the iterator object, to ensure that + // the iterator is not left in an unpredictable (to the caller) partially-consumed + // state. + auto items = src.attr("items")(); + assert(isinstance(items)); + return convert_elements(dict(reinterpret_borrow(items)), convert); + return false; + } + template static handle cast(T &&src, return_value_policy policy, handle parent) { dict d; diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 29e9c62327..997af69909 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -554,4 +554,5 @@ TEST_SUBMODULE(stl, m) { return vec_pair_int.size(); }); m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); + m.def("pass_std_map_int", [](const std::map &map_int) { return map_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index b1bf4f3249..72eda87a22 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -422,3 +422,34 @@ def gen_mix(): with pytest.raises(TypeError): m.pass_std_vector_int(gen_obj) assert not tuple(gen_obj) + + +class FakePyMappingMissingItems: + def __getitem__(self, _): + raise RuntimeError("Not expected to be called.") + + +class FakePyMappingWithItems(FakePyMappingMissingItems): + def items(self): + return ((1, 2), (3, 4)) + + +class FakePyMappingBadItems(FakePyMappingMissingItems): + def items(self): + return ((1, 2), (3, "x")) + + +def test_pass_std_map_int(): + fn = m.pass_std_map_int + assert fn({1: 2, 3: 4}) == 2 + assert fn(FakePyMappingWithItems()) == 2 + with pytest.raises(TypeError): + fn(FakePyMappingMissingItems()) + with pytest.raises(TypeError): + fn(FakePyMappingBadItems()) + with pytest.raises(TypeError): + fn([]) + + +# TODO(rwgk): test_set_caster_fully_consumes_iterator_object +# TODO(rwgk): test_map_caster_fully_consumes_iterator_object From eeb59afc787ff89451dc327e4773046463580274 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 15:22:54 -0700 Subject: [PATCH 06/39] Simplify `list_caster` `load()` implementation, push str/bytes check into `PyObjectTypeIsConvertibleToStdVector()`. --- include/pybind11/stl.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index cad842ccfd..ab8370bbf5 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -54,7 +54,10 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, } inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { - return PySequence_Check(obj) || PyGen_Check(obj) || PyAnySet_Check(obj) + if (PySequence_Check(obj)) { + return !PyUnicode_Check(obj) && !PyBytes_Check(obj); + } + return PyGen_Check(obj) || PyAnySet_Check(obj) || PyObjectIsInstanceWithOneOfTpNames( obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); } @@ -233,7 +236,7 @@ struct list_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - if (isinstance(src) || isinstance(src)) { + if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { return false; } if (isinstance(src)) { @@ -242,15 +245,12 @@ struct list_caster { if (!convert) { return false; } - if (PyObjectTypeIsConvertibleToStdVector(src.ptr())) { - // Designed to be behavior-equivalent to passing tuple(src) from Python: - // The conversion to a tuple will first exhaust the generator object, to ensure that - // the generator is not left in an unpredictable (to the caller) partially-consumed - // state. - assert(isinstance(src)); - return convert_elements(tuple(reinterpret_borrow(src)), convert); - } - return false; + // Designed to be behavior-equivalent to passing tuple(src) from Python: + // The conversion to a tuple will first exhaust the generator object, to ensure that + // the generator is not left in an unpredictable (to the caller) partially-consumed + // state. + assert(isinstance(src)); + return convert_elements(tuple(reinterpret_borrow(src)), convert); } private: From 3631886d337e128c2f64634bea343fd405d5aa09 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 16:03:18 -0700 Subject: [PATCH 07/39] clang-tidy cleanup with a few extra `(... != 0)` to be more consistent. --- include/pybind11/stl.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index ab8370bbf5..1f453f54fd 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -46,28 +46,30 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, return false; } const char *obj_tp_name = Py_TYPE(obj)->tp_name; - for (auto tp_name : tp_names) { - if (strcmp(obj_tp_name, tp_name) == 0) + for (const auto *tp_name : tp_names) { + if (strcmp(obj_tp_name, tp_name) == 0) { return true; + } } return false; } inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { - if (PySequence_Check(obj)) { + if (PySequence_Check(obj) != 0) { return !PyUnicode_Check(obj) && !PyBytes_Check(obj); } - return PyGen_Check(obj) || PyAnySet_Check(obj) + return (PyGen_Check(obj) != 0) || (PyAnySet_Check(obj) != 0) || PyObjectIsInstanceWithOneOfTpNames( obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); } inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { - return PyAnySet_Check(obj) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); + return (PyAnySet_Check(obj) != 0) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); } inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { - return PyDict_Check(obj) || (PyMapping_Check(obj) && PyObject_HasAttrString(obj, "items")); + return (PyDict_Check(obj) != 0) + || ((PyMapping_Check(obj) != 0) && (PyObject_HasAttrString(obj, "items") != 0)); } // From b4f767bfb2de96d52c52ce1a6a406299a044b529 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 6 Jun 2023 18:06:06 -0700 Subject: [PATCH 08/39] Also use `PyObjectTypeIsConvertibleToStdVector()` in `array_caster`. --- include/pybind11/stl.h | 27 +++++++++++++++++++++------ tests/test_stl.cpp | 2 ++ tests/test_stl.py | 10 ++++++---- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 1f453f54fd..cba1ca6660 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -324,12 +324,8 @@ struct array_caster { return size == Size; } -public: - bool load(handle src, bool convert) { - if (!isinstance(src)) { - return false; - } - auto l = reinterpret_borrow(src); + bool convert_elements(handle seq, bool convert) { + auto l = reinterpret_borrow(seq); if (!require_size(l.size())) { return false; } @@ -344,6 +340,25 @@ struct array_caster { return true; } +public: + bool load(handle src, bool convert) { + if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { + return false; + } + if (isinstance(src)) { + return convert_elements(src, convert); + } + if (!convert) { + return false; + } + // Designed to be behavior-equivalent to passing tuple(src) from Python: + // The conversion to a tuple will first exhaust the generator object, to ensure that + // the generator is not left in an unpredictable (to the caller) partially-consumed + // state. + assert(isinstance(src)); + return convert_elements(tuple(reinterpret_borrow(src)), convert); + } + template static handle cast(T &&src, return_value_policy policy, handle parent) { list l(src.size()); diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 997af69909..610e7f9f66 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -553,6 +553,8 @@ TEST_SUBMODULE(stl, m) { m.def("pass_std_vector_pair_int", [](const std::vector> &vec_pair_int) { return vec_pair_int.size(); }); + m.def("pass_std_array_int_2", + [](const std::array &arr_int) { return arr_int.size(); }); m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); m.def("pass_std_map_int", [](const std::map &map_int) { return map_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 72eda87a22..3039c407d7 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -381,15 +381,17 @@ def test_return_vector_bool_raw_ptr(): assert len(v) == 4513 -def test_pass_std_vector_int(): - fn = m.pass_std_vector_int +@pytest.mark.parametrize("fn", [m.pass_std_vector_int, m.pass_std_array_int_2]) +def test_pass_std_vector_int(fn): assert fn([1, 2]) == 2 assert fn((1, 2)) == 2 assert fn({1, 2}) == 2 assert fn({"x": 1, "y": 2}.values()) == 2 assert fn({1: None, 2: None}.keys()) == 2 - assert fn(i for i in range(3)) == 3 - assert fn(map(lambda i: i, range(4))) == 4 # noqa: C417 + assert fn(i for i in range(2)) == 2 + assert fn(map(lambda i: i, range(2))) == 2 # noqa: C417 + with pytest.raises(TypeError): + fn({1: 2, 3: 4}) with pytest.raises(TypeError): fn({}) From 3cc034b2c1cdbd4436ac90737fc0828dee6f25b5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 14 Jul 2023 12:53:42 -0700 Subject: [PATCH 09/39] Update comment pointing to clif/python/runtime.cc (code is unchanged). --- include/pybind11/stl.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index cba1ca6660..80fa9c9ab9 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -13,6 +13,7 @@ #include "detail/common.h" #include +#include #include #include #include @@ -36,8 +37,8 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) // -// Begin: Code developed under https://github.com/google/clif/blob/main/clif/python/runtime.cc -// (currently unpublished) +// Begin: Equivalent of +// https://github.com/google/clif/blob/337b2a106c0552ad85d40cfc3797465756840ea0/clif/python/runtime.cc#L388-L424 // inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, @@ -73,7 +74,7 @@ inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { } // -// End: Code developed under https://github.com/google/clif/blob/main/clif/python/runtime.cc +// End: Equivalent of clif/python/runtime.cc // /// Extracts an const lvalue reference or rvalue reference for U based on the type of T (e.g. for From e7330f2dcfbf982556778af55ea9c9c266b28ab8 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 14 Jul 2023 15:36:38 -0700 Subject: [PATCH 10/39] Comprehensive test coverage, enhanced set_caster load implementation. --- include/pybind11/stl.h | 30 ++++++------- tests/test_stl.cpp | 39 +++++++++++++---- tests/test_stl.py | 95 +++++++++++++++++++++++++++++------------- 3 files changed, 112 insertions(+), 52 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 80fa9c9ab9..8ee9ae2037 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -103,16 +103,13 @@ struct set_caster { private: template ::value, int> = 0> - void reserve_maybe(const sequence &s, Type *) { + void reserve_maybe(const anyset &s, Type *) { value.reserve(s.size()); } - void reserve_maybe(const sequence &, void *) {} + void reserve_maybe(const anyset &, void *) {} - bool convert_elements(handle seq, bool convert) { - auto s = reinterpret_borrow(seq); - value.clear(); - reserve_maybe(s, &value); - for (auto it : seq) { + bool convert_iterable(iterable itbl, bool convert) { + for (auto it : itbl) { key_conv conv; if (!conv.load(it, convert)) { return false; @@ -122,23 +119,27 @@ struct set_caster { return true; } + bool convert_anyset(anyset s, bool convert) { + value.clear(); + reserve_maybe(s, &value); + return convert_iterable(s, convert); + } + public: bool load(handle src, bool convert) { if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) { return false; } - if (isinstance(src)) { - return convert_elements(src, convert); + if (isinstance(src)) { + value.clear(); + return convert_anyset(reinterpret_borrow(src), convert); } if (!convert) { return false; } - // Designed to be behavior-equivalent to passing tuple(src) from Python: - // The conversion to a tuple will first exhaust the iterator object, to ensure that - // the iterator is not left in an unpredictable (to the caller) partially-consumed - // state. assert(isinstance(src)); - return convert_elements(tuple(reinterpret_borrow(src)), convert); + value.clear(); + return convert_iterable(reinterpret_borrow(src), convert); } template @@ -204,7 +205,6 @@ struct map_caster { auto items = src.attr("items")(); assert(isinstance(items)); return convert_elements(dict(reinterpret_borrow(items)), convert); - return false; } template diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 610e7f9f66..22806ac02b 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -167,6 +167,14 @@ struct type_caster> } // namespace detail } // namespace PYBIND11_NAMESPACE +int pass_std_vector_int(const std::vector &v) { + int zum = 100; + for (const int i : v) { + zum += 2 * i; + } + return zum; +} + TEST_SUBMODULE(stl, m) { // test_vector m.def("cast_vector", []() { return std::vector{1}; }); @@ -549,12 +557,29 @@ TEST_SUBMODULE(stl, m) { // Without explicitly specifying `take_ownership`, this function leaks. py::return_value_policy::take_ownership); - m.def("pass_std_vector_int", [](const std::vector &vec_int) { return vec_int.size(); }); - m.def("pass_std_vector_pair_int", [](const std::vector> &vec_pair_int) { - return vec_pair_int.size(); + m.def("pass_std_vector_int", pass_std_vector_int); + m.def("pass_std_vector_pair_int", [](const std::vector> &v) { + int zum = 0; + for (const auto &ij : v) { + zum += ij.first * 100 + ij.second; + } + return zum; + }); + m.def("pass_std_array_int_2", [](const std::array &a) { + return pass_std_vector_int(std::vector(a.begin(), a.end())) + 1; + }); + m.def("pass_std_set_int", [](const std::set &s) { + int zum = 200; + for (const int i : s) { + zum += 3 * i; + } + return zum; + }); + m.def("pass_std_map_int", [](const std::map &m) { + int zum = 500; + for (const auto &p : m) { + zum += p.first * 1000 + p.second; + } + return zum; }); - m.def("pass_std_array_int_2", - [](const std::array &arr_int) { return arr_int.size(); }); - m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); - m.def("pass_std_map_int", [](const std::map &map_int) { return map_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 3039c407d7..baf841b855 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -381,49 +381,62 @@ def test_return_vector_bool_raw_ptr(): assert len(v) == 4513 -@pytest.mark.parametrize("fn", [m.pass_std_vector_int, m.pass_std_array_int_2]) -def test_pass_std_vector_int(fn): - assert fn([1, 2]) == 2 - assert fn((1, 2)) == 2 - assert fn({1, 2}) == 2 - assert fn({"x": 1, "y": 2}.values()) == 2 - assert fn({1: None, 2: None}.keys()) == 2 - assert fn(i for i in range(2)) == 2 - assert fn(map(lambda i: i, range(2))) == 2 # noqa: C417 +@pytest.mark.parametrize( + ("fn", "offset"), [(m.pass_std_vector_int, 0), (m.pass_std_array_int_2, 1)] +) +def test_pass_std_vector_int(fn, offset): + assert fn([7, 13]) == 140 + offset + assert fn({6, 2}) == 116 + offset + assert fn({"x": 8, "y": 11}.values()) == 138 + offset + assert fn({3: None, 9: None}.keys()) == 124 + offset + assert fn(i for i in [4, 17]) == 142 + offset + assert fn(map(lambda i: i * 3, [8, 7])) == 190 + offset # noqa: C417 with pytest.raises(TypeError): - fn({1: 2, 3: 4}) + fn({"x": 0, "y": 1}) with pytest.raises(TypeError): fn({}) def test_pass_std_vector_pair_int(): fn = m.pass_std_vector_pair_int - assert fn({1: 2, 3: 4}.items()) == 2 - assert fn(zip([1, 2], [3, 4])) == 2 + assert fn({1: 2, 3: 4}.items()) == 406 + assert fn(zip([5, 17], [13, 9])) == 2222 + + +def test_list_caster_fully_consumes_generator_object(): + def gen_mix(): + yield from [1, 2.0, 3] + + gen_obj = gen_mix() + with pytest.raises(TypeError): + m.pass_std_vector_int(gen_obj) + assert not tuple(gen_obj) def test_pass_std_set_int(): fn = m.pass_std_set_int - assert fn({1, 2}) == 2 - assert fn({1: None, 2: None}.keys()) == 2 - with pytest.raises(TypeError): - fn([1, 2]) + assert fn({3, 15}) == 254 + assert fn({5: None, 12: None}.keys()) == 251 with pytest.raises(TypeError): - fn((1, 2)) + fn([]) with pytest.raises(TypeError): fn({}) with pytest.raises(TypeError): - fn(i for i in range(3)) - + fn({}.values()) + with pytest.raises(TypeError): + fn(i for i in []) -def test_list_caster_fully_consumes_generator_object(): - def gen_mix(): - yield from [1, 2.0, 3] - gen_obj = gen_mix() +def test_set_caster_dict_keys_failure(): + dict_keys = {1: None, 2.0: None, 3: None}.keys() + # The asserts does not really exercise anything in pybind11, but if one of + # them fails in some future version of Python, the set_caster load + # implementation may need to be revisited. + assert tuple(dict_keys) == (1, 2.0, 3) + assert tuple(dict_keys) == (1, 2.0, 3) with pytest.raises(TypeError): - m.pass_std_vector_int(gen_obj) - assert not tuple(gen_obj) + m.pass_std_set_int(dict_keys) + assert tuple(dict_keys) == (1, 2.0, 3) class FakePyMappingMissingItems: @@ -433,7 +446,7 @@ def __getitem__(self, _): class FakePyMappingWithItems(FakePyMappingMissingItems): def items(self): - return ((1, 2), (3, 4)) + return ((1, 3), (2, 4)) class FakePyMappingBadItems(FakePyMappingMissingItems): @@ -441,10 +454,19 @@ def items(self): return ((1, 2), (3, "x")) +class FakePyMappingGenObj(FakePyMappingMissingItems): + def __init__(self, gen_obj): + super().__init__() + self.gen_obj = gen_obj + + def items(self): + yield from self.gen_obj + + def test_pass_std_map_int(): fn = m.pass_std_map_int - assert fn({1: 2, 3: 4}) == 2 - assert fn(FakePyMappingWithItems()) == 2 + assert fn({1: 2, 3: 4}) == 4506 + assert fn(FakePyMappingWithItems()) == 3507 with pytest.raises(TypeError): fn(FakePyMappingMissingItems()) with pytest.raises(TypeError): @@ -453,5 +475,18 @@ def test_pass_std_map_int(): fn([]) -# TODO(rwgk): test_set_caster_fully_consumes_iterator_object -# TODO(rwgk): test_map_caster_fully_consumes_iterator_object +@pytest.mark.parametrize( + ("items", "expected_exception"), + [ + (((1, 2), (3, "x"), (4, 5)), TypeError), + (((1, 2), (3, 4, 5), (6, 7)), ValueError), + ], +) +def test_map_caster_fully_consumes_generator_object(items, expected_exception): + def gen_mix(): + yield from items + + gen_obj = gen_mix() + with pytest.raises(expected_exception): + m.pass_std_map_int(FakePyMappingGenObj(gen_obj)) + assert not tuple(gen_obj) From da29bf574c777b171aedb4c3ab1539d03c915306 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 14 Jul 2023 17:45:58 -0700 Subject: [PATCH 11/39] Resolve clang-tidy eror. --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 8ee9ae2037..acd940ef14 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -108,7 +108,7 @@ struct set_caster { } void reserve_maybe(const anyset &, void *) {} - bool convert_iterable(iterable itbl, bool convert) { + bool convert_iterable(const iterable &itbl, bool convert) { for (auto it : itbl) { key_conv conv; if (!conv.load(it, convert)) { From 1fa338e803e5d641c77d03f1f6a201b9baea4c9a Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 14 Jul 2023 22:46:36 -0700 Subject: [PATCH 12/39] Add a long C++ comment explaining what led to the `PyObjectTypeIsConvertibleTo*()` implementations. --- include/pybind11/stl.h | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index acd940ef14..d8750a7a94 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -39,7 +39,34 @@ PYBIND11_NAMESPACE_BEGIN(detail) // // Begin: Equivalent of // https://github.com/google/clif/blob/337b2a106c0552ad85d40cfc3797465756840ea0/clif/python/runtime.cc#L388-L424 -// +/* +The three `PyObjectTypeIsConvertibleTo*()` functions below are +the result of converging the behaviors of pybind11 and PyCLIF +(http://github.com/google/clif). + +Originally PyCLIF was extremely far on the permissive side of the spectrum, +while pybind11 was very far on the strict side. Originally PyCLIF accepted any +Python iterable as input for a C++ `vector`/`set`/`map` argument, as long as +the elements were convertible. The obvious (in hindsight) problem was that +any empty Python iterable could be passed to any of these C++ types, e.g. `{}` +was accpeted for C++ `vector`/`set` arguments, or `[]` for C++ `map` arguments. + +The functions below strike a practical permissive-vs-strict compromise, +informed by tens of thousands of use cases in the wild. A main objective is +to prevent accidents and improve readability: + +- Python literals must match the C++ types. + +- For C++ `set`: The potentially reducing conversion from a Python sequence + (e.g. Python `list` or `tuple`) to a C++ `set` must be explicit, by going + through a Python `set`. + +- However, a Python `set` can still be passed to a C++ `vector`. The rationale + is that this conversion is not reducing. Implicit conversions of this kind + are also fairly commonly used, therefore enforcing explicit conversions + would have an unfavorable cost : benefit ratio; more sloppily speaking, + such an enforcement would be more annyoing than helpful. +*/ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, std::initializer_list tp_names) { From 6b5c780ace53eb78deb1273043610be59bb571b9 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 15 Jul 2023 01:48:15 -0700 Subject: [PATCH 13/39] Minor function name change in test. --- tests/test_stl.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_stl.py b/tests/test_stl.py index baf841b855..43615843a4 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -404,10 +404,10 @@ def test_pass_std_vector_pair_int(): def test_list_caster_fully_consumes_generator_object(): - def gen_mix(): + def gen_invalid(): yield from [1, 2.0, 3] - gen_obj = gen_mix() + gen_obj = gen_invalid() with pytest.raises(TypeError): m.pass_std_vector_int(gen_obj) assert not tuple(gen_obj) @@ -483,10 +483,10 @@ def test_pass_std_map_int(): ], ) def test_map_caster_fully_consumes_generator_object(items, expected_exception): - def gen_mix(): + def gen_invalid(): yield from items - gen_obj = gen_mix() + gen_obj = gen_invalid() with pytest.raises(expected_exception): m.pass_std_map_int(FakePyMappingGenObj(gen_obj)) assert not tuple(gen_obj) From 4b25f7469fa5cce867fb099d455b58ea4c684530 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 00:27:42 -0700 Subject: [PATCH 14/39] strcmp -> std::strcmp (thanks @Skylion007 for catching this) --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index d8750a7a94..743d9f166d 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -75,7 +75,7 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, } const char *obj_tp_name = Py_TYPE(obj)->tp_name; for (const auto *tp_name : tp_names) { - if (strcmp(obj_tp_name, tp_name) == 0) { + if (std::strcmp(obj_tp_name, tp_name) == 0) { return true; } } From 3cfc95b4dfa07a1bedee37c290dd003a4f077091 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 01:04:20 -0700 Subject: [PATCH 15/39] Add `PyCallable_Check(items)` in `PyObjectTypeIsConvertibleToStdMap()` --- include/pybind11/stl.h | 16 ++++++++++++++-- tests/test_stl.py | 17 ++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 743d9f166d..09b9466ca1 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -96,8 +96,20 @@ inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { } inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { - return (PyDict_Check(obj) != 0) - || ((PyMapping_Check(obj) != 0) && (PyObject_HasAttrString(obj, "items") != 0)); + if (PyDict_Check(obj)) { + return true; + } + if (!PyMapping_Check(obj)) { + return false; + } + PyObject *items = PyObject_GetAttrString(obj, "items"); + if (items == nullptr) { + PyErr_Clear(); + return false; + } + bool is_convertible = (PyCallable_Check(items) != 0); + Py_DECREF(items); + return is_convertible; } // diff --git a/tests/test_stl.py b/tests/test_stl.py index 43615843a4..1c6f093567 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -454,6 +454,17 @@ def items(self): return ((1, 2), (3, "x")) +class FakePyMappingItemsNotCallable(FakePyMappingMissingItems): + @property + def items(self): + return ((1, 2), (3, 4)) + + +class FakePyMappingItemsWithArg(FakePyMappingMissingItems): + def items(self, _): + return ((1, 2), (3, 4)) + + class FakePyMappingGenObj(FakePyMappingMissingItems): def __init__(self, gen_obj): super().__init__() @@ -466,13 +477,17 @@ def items(self): def test_pass_std_map_int(): fn = m.pass_std_map_int assert fn({1: 2, 3: 4}) == 4506 + with pytest.raises(TypeError): + fn([]) assert fn(FakePyMappingWithItems()) == 3507 with pytest.raises(TypeError): fn(FakePyMappingMissingItems()) with pytest.raises(TypeError): fn(FakePyMappingBadItems()) with pytest.raises(TypeError): - fn([]) + fn(FakePyMappingItemsNotCallable()) + with pytest.raises(TypeError): + fn(FakePyMappingItemsWithArg()) @pytest.mark.parametrize( From 59509f2c4b6c0f132ec24c1618beece0472e42de Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 01:29:45 -0700 Subject: [PATCH 16/39] Resolve clang-tidy error --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 09b9466ca1..02e4b7d14b 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -99,7 +99,7 @@ inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { if (PyDict_Check(obj)) { return true; } - if (!PyMapping_Check(obj)) { + if (PyMapping_Check(obj) == 0) { return false; } PyObject *items = PyObject_GetAttrString(obj, "items"); From 4ab40d74cb4d612d37ef196c70721df92a0be665 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 12:45:39 -0700 Subject: [PATCH 17/39] Use `PyMapping_Items()` instead of `src.attr("items")()`, to be internally consistent with `PyMapping_Check()` --- include/pybind11/stl.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 02e4b7d14b..7129c79c3e 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -99,6 +99,9 @@ inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { if (PyDict_Check(obj)) { return true; } + // Implicit requirement in the conditions below: + // A type with `.__getitem__()` & `.items()` methods must implement these + // to be compatible with https://docs.python.org/3/c-api/mapping.html if (PyMapping_Check(obj) == 0) { return false; } @@ -237,11 +240,10 @@ struct map_caster { if (!convert) { return false; } - // Designed to be behavior-equivalent to passing dict(src.items()) from Python: - // The conversion to a dict will first exhaust the iterator object, to ensure that - // the iterator is not left in an unpredictable (to the caller) partially-consumed - // state. - auto items = src.attr("items")(); + auto items = reinterpret_steal(PyMapping_Items(src.ptr())); + if (!items) { + throw error_already_set(); + } assert(isinstance(items)); return convert_elements(dict(reinterpret_borrow(items)), convert); } From 5f7c9d4b30da0be792ac7043a240da0475c1fbcc Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 14:12:52 -0700 Subject: [PATCH 18/39] Update link to PyCLIF sources. --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 7129c79c3e..7805ad2910 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -38,7 +38,7 @@ PYBIND11_NAMESPACE_BEGIN(detail) // // Begin: Equivalent of -// https://github.com/google/clif/blob/337b2a106c0552ad85d40cfc3797465756840ea0/clif/python/runtime.cc#L388-L424 +// https://github.com/google/clif/blob/ae4eee1de07cdf115c0c9bf9fec9ff28efce6f6c/clif/python/runtime.cc#L388-L438 /* The three `PyObjectTypeIsConvertibleTo*()` functions below are the result of converging the behaviors of pybind11 and PyCLIF From ccfeb02728e9b707b63e0d56ef57257fa3338019 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 15:02:04 -0700 Subject: [PATCH 19/39] Fix typo (thanks @wangxf123456 for catching this) --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 7805ad2910..ddfc78911d 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -65,7 +65,7 @@ to prevent accidents and improve readability: is that this conversion is not reducing. Implicit conversions of this kind are also fairly commonly used, therefore enforcing explicit conversions would have an unfavorable cost : benefit ratio; more sloppily speaking, - such an enforcement would be more annyoing than helpful. + such an enforcement would be more annoying than helpful. */ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, From dacb8272ab64aeab28847cfaa26ed8a21c988b6d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 29 May 2023 12:47:15 -0700 Subject: [PATCH 20/39] Add `test_pass_std_vector_int()`, `test_pass_std_set_int()` in test_stl --- tests/test_stl.cpp | 3 +++ tests/test_stl.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index d45465d681..483e394bea 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -548,4 +548,7 @@ TEST_SUBMODULE(stl, m) { []() { return new std::vector(4513); }, // Without explicitly specifying `take_ownership`, this function leaks. py::return_value_policy::take_ownership); + + m.def("pass_std_vector_int", [](const std::vector &vec_int) { return vec_int.size(); }); + m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 8a614f8b87..f5e3f6c957 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -379,3 +379,32 @@ def test_return_vector_bool_raw_ptr(): v = m.return_vector_bool_raw_ptr() assert isinstance(v, list) assert len(v) == 4513 + + +def test_pass_std_vector_int(): + fn = m.pass_std_vector_int + assert fn([1, 2]) == 2 + assert fn((1, 2)) == 2 + with pytest.raises(TypeError): + fn(set()) + with pytest.raises(TypeError): + fn({}) + with pytest.raises(TypeError): + fn({}.keys()) + with pytest.raises(TypeError): + fn(i for i in range(3)) + + +def test_pass_std_set_int(): + fn = m.pass_std_set_int + assert fn({1, 2}) == 2 + with pytest.raises(TypeError): + fn([1, 2]) + with pytest.raises(TypeError): + fn((1, 2)) + with pytest.raises(TypeError): + fn({}) + with pytest.raises(TypeError): + fn({}.keys()) + with pytest.raises(TypeError): + fn(i for i in range(3)) From 3fc2d2afafc70b2aad6e157600dbd04345843dbd Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 30 May 2023 08:46:15 -0700 Subject: [PATCH 21/39] Change `list_caster` to also accept generator objects (`PyGen_Check(src.ptr()`). Note for completeness: This is a more conservative change than https://github.com/google/pywrapcc/pull/30042 --- include/pybind11/stl.h | 38 ++++++++++++++++++++++++++++---------- tests/test_stl.py | 13 +++++++++++-- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index f39f44f7c9..cad9bc66fc 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -166,13 +166,38 @@ struct list_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - if (!isinstance(src) || isinstance(src) || isinstance(src)) { + if (isinstance(src) || isinstance(src)) { return false; } - auto s = reinterpret_borrow(src); + if (isinstance(src)) { + return convert_elements(src, convert); + } + if (!convert) { + return false; + } + if (PyGen_Check(src.ptr())) { + // Designed to be behavior-equivalent to passing tuple(src) from Python: + // The conversion to a tuple will first exhaust the generator object, to ensure that + // the generator is not left in an unpredictable (to the caller) partially-consumed + // state. + assert(isinstance(src)); + return convert_elements(tuple(reinterpret_borrow(src)), convert); + } + return false; + } + +private: + template ::value, int> = 0> + void reserve_maybe(const sequence &s, Type *) { + value.reserve(s.size()); + } + void reserve_maybe(const sequence &, void *) {} + + bool convert_elements(handle seq, bool convert) { + auto s = reinterpret_borrow(seq); value.clear(); reserve_maybe(s, &value); - for (auto it : s) { + for (auto it : seq) { value_conv conv; if (!conv.load(it, convert)) { return false; @@ -182,13 +207,6 @@ struct list_caster { return true; } -private: - template ::value, int> = 0> - void reserve_maybe(const sequence &s, Type *) { - value.reserve(s.size()); - } - void reserve_maybe(const sequence &, void *) {} - public: template static handle cast(T &&src, return_value_policy policy, handle parent) { diff --git a/tests/test_stl.py b/tests/test_stl.py index f5e3f6c957..9e5e84e163 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -385,14 +385,13 @@ def test_pass_std_vector_int(): fn = m.pass_std_vector_int assert fn([1, 2]) == 2 assert fn((1, 2)) == 2 + assert fn(i for i in range(3)) == 3 with pytest.raises(TypeError): fn(set()) with pytest.raises(TypeError): fn({}) with pytest.raises(TypeError): fn({}.keys()) - with pytest.raises(TypeError): - fn(i for i in range(3)) def test_pass_std_set_int(): @@ -408,3 +407,13 @@ def test_pass_std_set_int(): fn({}.keys()) with pytest.raises(TypeError): fn(i for i in range(3)) + + +def test_list_caster_fully_consumes_generator_object(): + def gen_mix(): + yield from [1, 2.0, 3] + + gen_obj = gen_mix() + with pytest.raises(TypeError): + m.pass_std_vector_int(gen_obj) + assert not tuple(gen_obj) From ff0724ff1d54a47dc78cca178398919ae8152c42 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 12:33:30 -0700 Subject: [PATCH 22/39] Drop in (currently unpublished) PyCLIF code, use in `list_caster`, adjust tests. --- include/pybind11/stl.h | 38 +++++++++++++++++++++++++++++++++++++- tests/test_stl.cpp | 3 +++ tests/test_stl.py | 14 ++++++++++---- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index cad9bc66fc..a040a3fe7d 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -35,6 +35,42 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) +// +// Begin: Code developed under https://github.com/google/clif/blob/main/clif/python/runtime.cc +// (currently unpublished) +// + +inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, + std::initializer_list tp_names) { + if (PyType_Check(obj)) { + return false; + } + const char *obj_tp_name = Py_TYPE(obj)->tp_name; + for (auto tp_name : tp_names) { + if (strcmp(obj_tp_name, tp_name) == 0) + return true; + } + return false; +} + +inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { + return PySequence_Check(obj) || PyGen_Check(obj) || PyAnySet_Check(obj) + || PyObjectIsInstanceWithOneOfTpNames( + obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); +} + +inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { + return PyAnySet_Check(obj) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); +} + +inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { + return PyDict_Check(obj) || (PyMapping_Check(obj) && PyObject_HasAttrString(obj, "items")); +} + +// +// End: Code developed under https://github.com/google/clif/blob/main/clif/python/runtime.cc +// + /// Extracts an const lvalue reference or rvalue reference for U based on the type of T (e.g. for /// forwarding a container element). Typically used indirect via forwarded_type(), below. template @@ -175,7 +211,7 @@ struct list_caster { if (!convert) { return false; } - if (PyGen_Check(src.ptr())) { + if (PyObjectTypeIsConvertibleToStdVector(src.ptr())) { // Designed to be behavior-equivalent to passing tuple(src) from Python: // The conversion to a tuple will first exhaust the generator object, to ensure that // the generator is not left in an unpredictable (to the caller) partially-consumed diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 483e394bea..29e9c62327 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -550,5 +550,8 @@ TEST_SUBMODULE(stl, m) { py::return_value_policy::take_ownership); m.def("pass_std_vector_int", [](const std::vector &vec_int) { return vec_int.size(); }); + m.def("pass_std_vector_pair_int", [](const std::vector> &vec_pair_int) { + return vec_pair_int.size(); + }); m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 9e5e84e163..f25c1b7cd8 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -385,13 +385,19 @@ def test_pass_std_vector_int(): fn = m.pass_std_vector_int assert fn([1, 2]) == 2 assert fn((1, 2)) == 2 + assert fn({1, 2}) == 2 + assert fn({"x": 1, "y": 2}.values()) == 2 + assert fn({1: None, 2: None}.keys()) == 2 assert fn(i for i in range(3)) == 3 - with pytest.raises(TypeError): - fn(set()) + assert fn(map(lambda i: i, range(4))) == 4 # noqa: C417 with pytest.raises(TypeError): fn({}) - with pytest.raises(TypeError): - fn({}.keys()) + + +def test_pass_std_vector_pair_int(): + fn = m.pass_std_vector_pair_int + assert fn({1: 2, 3: 4}.items()) == 2 + assert fn(zip([1, 2], [3, 4])) == 2 def test_pass_std_set_int(): From be022911e1ee04aede7145129c3cc9a03d80877b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 13:04:10 -0700 Subject: [PATCH 23/39] Use `PyObjectTypeIsConvertibleToStdSet()` in `set_caster`, adjust tests. --- include/pybind11/stl.h | 35 +++++++++++++++++++++++++---------- tests/test_stl.py | 3 +-- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index a040a3fe7d..6feac126f7 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -97,22 +97,18 @@ struct set_caster { private: template ::value, int> = 0> - void reserve_maybe(const anyset &s, Type *) { + void reserve_maybe(const sequence &s, Type *) { value.reserve(s.size()); } - void reserve_maybe(const anyset &, void *) {} + void reserve_maybe(const sequence &, void *) {} -public: - bool load(handle src, bool convert) { - if (!isinstance(src)) { - return false; - } - auto s = reinterpret_borrow(src); + bool convert_elements(handle seq, bool convert) { + auto s = reinterpret_borrow(seq); value.clear(); reserve_maybe(s, &value); - for (auto entry : s) { + for (auto it : seq) { key_conv conv; - if (!conv.load(entry, convert)) { + if (!conv.load(it, convert)) { return false; } value.insert(cast_op(std::move(conv))); @@ -120,6 +116,25 @@ struct set_caster { return true; } +public: + bool load(handle src, bool convert) { + if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) { + return false; + } + if (isinstance(src)) { + return convert_elements(src, convert); + } + if (!convert) { + return false; + } + // Designed to be behavior-equivalent to passing tuple(src) from Python: + // The conversion to a tuple will first exhaust the iterator object, to ensure that + // the iterator is not left in an unpredictable (to the caller) partially-consumed + // state. + assert(isinstance(src)); + return convert_elements(tuple(reinterpret_borrow(src)), convert); + } + template static handle cast(T &&src, return_value_policy policy, handle parent) { if (!std::is_lvalue_reference::value) { diff --git a/tests/test_stl.py b/tests/test_stl.py index f25c1b7cd8..b1bf4f3249 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -403,14 +403,13 @@ def test_pass_std_vector_pair_int(): def test_pass_std_set_int(): fn = m.pass_std_set_int assert fn({1, 2}) == 2 + assert fn({1: None, 2: None}.keys()) == 2 with pytest.raises(TypeError): fn([1, 2]) with pytest.raises(TypeError): fn((1, 2)) with pytest.raises(TypeError): fn({}) - with pytest.raises(TypeError): - fn({}.keys()) with pytest.raises(TypeError): fn(i for i in range(3)) From 0a79f55aaa214e1dd14644c516472099d342e8db Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 13:10:48 -0700 Subject: [PATCH 24/39] Use `PyObjectTypeIsConvertibleToStdMap()` in `map_caster`, add tests. --- include/pybind11/stl.h | 28 ++++++++++++++++++++++------ tests/test_stl.cpp | 1 + tests/test_stl.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 6feac126f7..cad842ccfd 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -166,12 +166,7 @@ struct map_caster { } void reserve_maybe(const dict &, void *) {} -public: - bool load(handle src, bool convert) { - if (!isinstance(src)) { - return false; - } - auto d = reinterpret_borrow(src); + bool convert_elements(const dict &d, bool convert) { value.clear(); reserve_maybe(d, &value); for (auto it : d) { @@ -185,6 +180,27 @@ struct map_caster { return true; } +public: + bool load(handle src, bool convert) { + if (!PyObjectTypeIsConvertibleToStdMap(src.ptr())) { + return false; + } + if (isinstance(src)) { + return convert_elements(reinterpret_borrow(src), convert); + } + if (!convert) { + return false; + } + // Designed to be behavior-equivalent to passing dict(src.items()) from Python: + // The conversion to a dict will first exhaust the iterator object, to ensure that + // the iterator is not left in an unpredictable (to the caller) partially-consumed + // state. + auto items = src.attr("items")(); + assert(isinstance(items)); + return convert_elements(dict(reinterpret_borrow(items)), convert); + return false; + } + template static handle cast(T &&src, return_value_policy policy, handle parent) { dict d; diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 29e9c62327..997af69909 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -554,4 +554,5 @@ TEST_SUBMODULE(stl, m) { return vec_pair_int.size(); }); m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); + m.def("pass_std_map_int", [](const std::map &map_int) { return map_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index b1bf4f3249..72eda87a22 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -422,3 +422,34 @@ def gen_mix(): with pytest.raises(TypeError): m.pass_std_vector_int(gen_obj) assert not tuple(gen_obj) + + +class FakePyMappingMissingItems: + def __getitem__(self, _): + raise RuntimeError("Not expected to be called.") + + +class FakePyMappingWithItems(FakePyMappingMissingItems): + def items(self): + return ((1, 2), (3, 4)) + + +class FakePyMappingBadItems(FakePyMappingMissingItems): + def items(self): + return ((1, 2), (3, "x")) + + +def test_pass_std_map_int(): + fn = m.pass_std_map_int + assert fn({1: 2, 3: 4}) == 2 + assert fn(FakePyMappingWithItems()) == 2 + with pytest.raises(TypeError): + fn(FakePyMappingMissingItems()) + with pytest.raises(TypeError): + fn(FakePyMappingBadItems()) + with pytest.raises(TypeError): + fn([]) + + +# TODO(rwgk): test_set_caster_fully_consumes_iterator_object +# TODO(rwgk): test_map_caster_fully_consumes_iterator_object From ae954246876274535cfb6cd833d1b2fd82c5e727 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 15:22:54 -0700 Subject: [PATCH 25/39] Simplify `list_caster` `load()` implementation, push str/bytes check into `PyObjectTypeIsConvertibleToStdVector()`. --- include/pybind11/stl.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index cad842ccfd..ab8370bbf5 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -54,7 +54,10 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, } inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { - return PySequence_Check(obj) || PyGen_Check(obj) || PyAnySet_Check(obj) + if (PySequence_Check(obj)) { + return !PyUnicode_Check(obj) && !PyBytes_Check(obj); + } + return PyGen_Check(obj) || PyAnySet_Check(obj) || PyObjectIsInstanceWithOneOfTpNames( obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); } @@ -233,7 +236,7 @@ struct list_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - if (isinstance(src) || isinstance(src)) { + if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { return false; } if (isinstance(src)) { @@ -242,15 +245,12 @@ struct list_caster { if (!convert) { return false; } - if (PyObjectTypeIsConvertibleToStdVector(src.ptr())) { - // Designed to be behavior-equivalent to passing tuple(src) from Python: - // The conversion to a tuple will first exhaust the generator object, to ensure that - // the generator is not left in an unpredictable (to the caller) partially-consumed - // state. - assert(isinstance(src)); - return convert_elements(tuple(reinterpret_borrow(src)), convert); - } - return false; + // Designed to be behavior-equivalent to passing tuple(src) from Python: + // The conversion to a tuple will first exhaust the generator object, to ensure that + // the generator is not left in an unpredictable (to the caller) partially-consumed + // state. + assert(isinstance(src)); + return convert_elements(tuple(reinterpret_borrow(src)), convert); } private: From 8aa81d7dc504c8d7d03c526fa9b78d77fd8fa77b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 5 Jun 2023 16:03:18 -0700 Subject: [PATCH 26/39] clang-tidy cleanup with a few extra `(... != 0)` to be more consistent. --- include/pybind11/stl.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index ab8370bbf5..1f453f54fd 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -46,28 +46,30 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, return false; } const char *obj_tp_name = Py_TYPE(obj)->tp_name; - for (auto tp_name : tp_names) { - if (strcmp(obj_tp_name, tp_name) == 0) + for (const auto *tp_name : tp_names) { + if (strcmp(obj_tp_name, tp_name) == 0) { return true; + } } return false; } inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { - if (PySequence_Check(obj)) { + if (PySequence_Check(obj) != 0) { return !PyUnicode_Check(obj) && !PyBytes_Check(obj); } - return PyGen_Check(obj) || PyAnySet_Check(obj) + return (PyGen_Check(obj) != 0) || (PyAnySet_Check(obj) != 0) || PyObjectIsInstanceWithOneOfTpNames( obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); } inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { - return PyAnySet_Check(obj) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); + return (PyAnySet_Check(obj) != 0) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); } inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { - return PyDict_Check(obj) || (PyMapping_Check(obj) && PyObject_HasAttrString(obj, "items")); + return (PyDict_Check(obj) != 0) + || ((PyMapping_Check(obj) != 0) && (PyObject_HasAttrString(obj, "items") != 0)); } // From 4435ed0f4464e738388047b0104482274b47c64a Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 6 Jun 2023 18:06:06 -0700 Subject: [PATCH 27/39] Also use `PyObjectTypeIsConvertibleToStdVector()` in `array_caster`. --- include/pybind11/stl.h | 27 +++++++++++++++++++++------ tests/test_stl.cpp | 2 ++ tests/test_stl.py | 10 ++++++---- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 1f453f54fd..cba1ca6660 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -324,12 +324,8 @@ struct array_caster { return size == Size; } -public: - bool load(handle src, bool convert) { - if (!isinstance(src)) { - return false; - } - auto l = reinterpret_borrow(src); + bool convert_elements(handle seq, bool convert) { + auto l = reinterpret_borrow(seq); if (!require_size(l.size())) { return false; } @@ -344,6 +340,25 @@ struct array_caster { return true; } +public: + bool load(handle src, bool convert) { + if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { + return false; + } + if (isinstance(src)) { + return convert_elements(src, convert); + } + if (!convert) { + return false; + } + // Designed to be behavior-equivalent to passing tuple(src) from Python: + // The conversion to a tuple will first exhaust the generator object, to ensure that + // the generator is not left in an unpredictable (to the caller) partially-consumed + // state. + assert(isinstance(src)); + return convert_elements(tuple(reinterpret_borrow(src)), convert); + } + template static handle cast(T &&src, return_value_policy policy, handle parent) { list l(src.size()); diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 997af69909..610e7f9f66 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -553,6 +553,8 @@ TEST_SUBMODULE(stl, m) { m.def("pass_std_vector_pair_int", [](const std::vector> &vec_pair_int) { return vec_pair_int.size(); }); + m.def("pass_std_array_int_2", + [](const std::array &arr_int) { return arr_int.size(); }); m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); m.def("pass_std_map_int", [](const std::map &map_int) { return map_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 72eda87a22..3039c407d7 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -381,15 +381,17 @@ def test_return_vector_bool_raw_ptr(): assert len(v) == 4513 -def test_pass_std_vector_int(): - fn = m.pass_std_vector_int +@pytest.mark.parametrize("fn", [m.pass_std_vector_int, m.pass_std_array_int_2]) +def test_pass_std_vector_int(fn): assert fn([1, 2]) == 2 assert fn((1, 2)) == 2 assert fn({1, 2}) == 2 assert fn({"x": 1, "y": 2}.values()) == 2 assert fn({1: None, 2: None}.keys()) == 2 - assert fn(i for i in range(3)) == 3 - assert fn(map(lambda i: i, range(4))) == 4 # noqa: C417 + assert fn(i for i in range(2)) == 2 + assert fn(map(lambda i: i, range(2))) == 2 # noqa: C417 + with pytest.raises(TypeError): + fn({1: 2, 3: 4}) with pytest.raises(TypeError): fn({}) From 9aca5e8363c47a678e9674a37f1e1694ffbbfacd Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 14 Jul 2023 12:53:42 -0700 Subject: [PATCH 28/39] Update comment pointing to clif/python/runtime.cc (code is unchanged). --- include/pybind11/stl.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index cba1ca6660..80fa9c9ab9 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -13,6 +13,7 @@ #include "detail/common.h" #include +#include #include #include #include @@ -36,8 +37,8 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(detail) // -// Begin: Code developed under https://github.com/google/clif/blob/main/clif/python/runtime.cc -// (currently unpublished) +// Begin: Equivalent of +// https://github.com/google/clif/blob/337b2a106c0552ad85d40cfc3797465756840ea0/clif/python/runtime.cc#L388-L424 // inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, @@ -73,7 +74,7 @@ inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { } // -// End: Code developed under https://github.com/google/clif/blob/main/clif/python/runtime.cc +// End: Equivalent of clif/python/runtime.cc // /// Extracts an const lvalue reference or rvalue reference for U based on the type of T (e.g. for From 9dc7d7c9935e57c41cd7ec77946d195dfcad96e8 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 14 Jul 2023 15:36:38 -0700 Subject: [PATCH 29/39] Comprehensive test coverage, enhanced set_caster load implementation. --- include/pybind11/stl.h | 30 ++++++------- tests/test_stl.cpp | 39 +++++++++++++---- tests/test_stl.py | 95 +++++++++++++++++++++++++++++------------- 3 files changed, 112 insertions(+), 52 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 80fa9c9ab9..8ee9ae2037 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -103,16 +103,13 @@ struct set_caster { private: template ::value, int> = 0> - void reserve_maybe(const sequence &s, Type *) { + void reserve_maybe(const anyset &s, Type *) { value.reserve(s.size()); } - void reserve_maybe(const sequence &, void *) {} + void reserve_maybe(const anyset &, void *) {} - bool convert_elements(handle seq, bool convert) { - auto s = reinterpret_borrow(seq); - value.clear(); - reserve_maybe(s, &value); - for (auto it : seq) { + bool convert_iterable(iterable itbl, bool convert) { + for (auto it : itbl) { key_conv conv; if (!conv.load(it, convert)) { return false; @@ -122,23 +119,27 @@ struct set_caster { return true; } + bool convert_anyset(anyset s, bool convert) { + value.clear(); + reserve_maybe(s, &value); + return convert_iterable(s, convert); + } + public: bool load(handle src, bool convert) { if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) { return false; } - if (isinstance(src)) { - return convert_elements(src, convert); + if (isinstance(src)) { + value.clear(); + return convert_anyset(reinterpret_borrow(src), convert); } if (!convert) { return false; } - // Designed to be behavior-equivalent to passing tuple(src) from Python: - // The conversion to a tuple will first exhaust the iterator object, to ensure that - // the iterator is not left in an unpredictable (to the caller) partially-consumed - // state. assert(isinstance(src)); - return convert_elements(tuple(reinterpret_borrow(src)), convert); + value.clear(); + return convert_iterable(reinterpret_borrow(src), convert); } template @@ -204,7 +205,6 @@ struct map_caster { auto items = src.attr("items")(); assert(isinstance(items)); return convert_elements(dict(reinterpret_borrow(items)), convert); - return false; } template diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 610e7f9f66..22806ac02b 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -167,6 +167,14 @@ struct type_caster> } // namespace detail } // namespace PYBIND11_NAMESPACE +int pass_std_vector_int(const std::vector &v) { + int zum = 100; + for (const int i : v) { + zum += 2 * i; + } + return zum; +} + TEST_SUBMODULE(stl, m) { // test_vector m.def("cast_vector", []() { return std::vector{1}; }); @@ -549,12 +557,29 @@ TEST_SUBMODULE(stl, m) { // Without explicitly specifying `take_ownership`, this function leaks. py::return_value_policy::take_ownership); - m.def("pass_std_vector_int", [](const std::vector &vec_int) { return vec_int.size(); }); - m.def("pass_std_vector_pair_int", [](const std::vector> &vec_pair_int) { - return vec_pair_int.size(); + m.def("pass_std_vector_int", pass_std_vector_int); + m.def("pass_std_vector_pair_int", [](const std::vector> &v) { + int zum = 0; + for (const auto &ij : v) { + zum += ij.first * 100 + ij.second; + } + return zum; + }); + m.def("pass_std_array_int_2", [](const std::array &a) { + return pass_std_vector_int(std::vector(a.begin(), a.end())) + 1; + }); + m.def("pass_std_set_int", [](const std::set &s) { + int zum = 200; + for (const int i : s) { + zum += 3 * i; + } + return zum; + }); + m.def("pass_std_map_int", [](const std::map &m) { + int zum = 500; + for (const auto &p : m) { + zum += p.first * 1000 + p.second; + } + return zum; }); - m.def("pass_std_array_int_2", - [](const std::array &arr_int) { return arr_int.size(); }); - m.def("pass_std_set_int", [](const std::set &set_int) { return set_int.size(); }); - m.def("pass_std_map_int", [](const std::map &map_int) { return map_int.size(); }); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 3039c407d7..baf841b855 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -381,49 +381,62 @@ def test_return_vector_bool_raw_ptr(): assert len(v) == 4513 -@pytest.mark.parametrize("fn", [m.pass_std_vector_int, m.pass_std_array_int_2]) -def test_pass_std_vector_int(fn): - assert fn([1, 2]) == 2 - assert fn((1, 2)) == 2 - assert fn({1, 2}) == 2 - assert fn({"x": 1, "y": 2}.values()) == 2 - assert fn({1: None, 2: None}.keys()) == 2 - assert fn(i for i in range(2)) == 2 - assert fn(map(lambda i: i, range(2))) == 2 # noqa: C417 +@pytest.mark.parametrize( + ("fn", "offset"), [(m.pass_std_vector_int, 0), (m.pass_std_array_int_2, 1)] +) +def test_pass_std_vector_int(fn, offset): + assert fn([7, 13]) == 140 + offset + assert fn({6, 2}) == 116 + offset + assert fn({"x": 8, "y": 11}.values()) == 138 + offset + assert fn({3: None, 9: None}.keys()) == 124 + offset + assert fn(i for i in [4, 17]) == 142 + offset + assert fn(map(lambda i: i * 3, [8, 7])) == 190 + offset # noqa: C417 with pytest.raises(TypeError): - fn({1: 2, 3: 4}) + fn({"x": 0, "y": 1}) with pytest.raises(TypeError): fn({}) def test_pass_std_vector_pair_int(): fn = m.pass_std_vector_pair_int - assert fn({1: 2, 3: 4}.items()) == 2 - assert fn(zip([1, 2], [3, 4])) == 2 + assert fn({1: 2, 3: 4}.items()) == 406 + assert fn(zip([5, 17], [13, 9])) == 2222 + + +def test_list_caster_fully_consumes_generator_object(): + def gen_mix(): + yield from [1, 2.0, 3] + + gen_obj = gen_mix() + with pytest.raises(TypeError): + m.pass_std_vector_int(gen_obj) + assert not tuple(gen_obj) def test_pass_std_set_int(): fn = m.pass_std_set_int - assert fn({1, 2}) == 2 - assert fn({1: None, 2: None}.keys()) == 2 - with pytest.raises(TypeError): - fn([1, 2]) + assert fn({3, 15}) == 254 + assert fn({5: None, 12: None}.keys()) == 251 with pytest.raises(TypeError): - fn((1, 2)) + fn([]) with pytest.raises(TypeError): fn({}) with pytest.raises(TypeError): - fn(i for i in range(3)) - + fn({}.values()) + with pytest.raises(TypeError): + fn(i for i in []) -def test_list_caster_fully_consumes_generator_object(): - def gen_mix(): - yield from [1, 2.0, 3] - gen_obj = gen_mix() +def test_set_caster_dict_keys_failure(): + dict_keys = {1: None, 2.0: None, 3: None}.keys() + # The asserts does not really exercise anything in pybind11, but if one of + # them fails in some future version of Python, the set_caster load + # implementation may need to be revisited. + assert tuple(dict_keys) == (1, 2.0, 3) + assert tuple(dict_keys) == (1, 2.0, 3) with pytest.raises(TypeError): - m.pass_std_vector_int(gen_obj) - assert not tuple(gen_obj) + m.pass_std_set_int(dict_keys) + assert tuple(dict_keys) == (1, 2.0, 3) class FakePyMappingMissingItems: @@ -433,7 +446,7 @@ def __getitem__(self, _): class FakePyMappingWithItems(FakePyMappingMissingItems): def items(self): - return ((1, 2), (3, 4)) + return ((1, 3), (2, 4)) class FakePyMappingBadItems(FakePyMappingMissingItems): @@ -441,10 +454,19 @@ def items(self): return ((1, 2), (3, "x")) +class FakePyMappingGenObj(FakePyMappingMissingItems): + def __init__(self, gen_obj): + super().__init__() + self.gen_obj = gen_obj + + def items(self): + yield from self.gen_obj + + def test_pass_std_map_int(): fn = m.pass_std_map_int - assert fn({1: 2, 3: 4}) == 2 - assert fn(FakePyMappingWithItems()) == 2 + assert fn({1: 2, 3: 4}) == 4506 + assert fn(FakePyMappingWithItems()) == 3507 with pytest.raises(TypeError): fn(FakePyMappingMissingItems()) with pytest.raises(TypeError): @@ -453,5 +475,18 @@ def test_pass_std_map_int(): fn([]) -# TODO(rwgk): test_set_caster_fully_consumes_iterator_object -# TODO(rwgk): test_map_caster_fully_consumes_iterator_object +@pytest.mark.parametrize( + ("items", "expected_exception"), + [ + (((1, 2), (3, "x"), (4, 5)), TypeError), + (((1, 2), (3, 4, 5), (6, 7)), ValueError), + ], +) +def test_map_caster_fully_consumes_generator_object(items, expected_exception): + def gen_mix(): + yield from items + + gen_obj = gen_mix() + with pytest.raises(expected_exception): + m.pass_std_map_int(FakePyMappingGenObj(gen_obj)) + assert not tuple(gen_obj) From d9eeea897820e726ba7e1bd0fbaff8ae0def1aa1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 14 Jul 2023 17:45:58 -0700 Subject: [PATCH 30/39] Resolve clang-tidy eror. --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 8ee9ae2037..acd940ef14 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -108,7 +108,7 @@ struct set_caster { } void reserve_maybe(const anyset &, void *) {} - bool convert_iterable(iterable itbl, bool convert) { + bool convert_iterable(const iterable &itbl, bool convert) { for (auto it : itbl) { key_conv conv; if (!conv.load(it, convert)) { From 9ac319b42e904d9a0d00a43e3d04c53c4e748f0b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 14 Jul 2023 22:46:36 -0700 Subject: [PATCH 31/39] Add a long C++ comment explaining what led to the `PyObjectTypeIsConvertibleTo*()` implementations. --- include/pybind11/stl.h | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index acd940ef14..d8750a7a94 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -39,7 +39,34 @@ PYBIND11_NAMESPACE_BEGIN(detail) // // Begin: Equivalent of // https://github.com/google/clif/blob/337b2a106c0552ad85d40cfc3797465756840ea0/clif/python/runtime.cc#L388-L424 -// +/* +The three `PyObjectTypeIsConvertibleTo*()` functions below are +the result of converging the behaviors of pybind11 and PyCLIF +(http://github.com/google/clif). + +Originally PyCLIF was extremely far on the permissive side of the spectrum, +while pybind11 was very far on the strict side. Originally PyCLIF accepted any +Python iterable as input for a C++ `vector`/`set`/`map` argument, as long as +the elements were convertible. The obvious (in hindsight) problem was that +any empty Python iterable could be passed to any of these C++ types, e.g. `{}` +was accpeted for C++ `vector`/`set` arguments, or `[]` for C++ `map` arguments. + +The functions below strike a practical permissive-vs-strict compromise, +informed by tens of thousands of use cases in the wild. A main objective is +to prevent accidents and improve readability: + +- Python literals must match the C++ types. + +- For C++ `set`: The potentially reducing conversion from a Python sequence + (e.g. Python `list` or `tuple`) to a C++ `set` must be explicit, by going + through a Python `set`. + +- However, a Python `set` can still be passed to a C++ `vector`. The rationale + is that this conversion is not reducing. Implicit conversions of this kind + are also fairly commonly used, therefore enforcing explicit conversions + would have an unfavorable cost : benefit ratio; more sloppily speaking, + such an enforcement would be more annyoing than helpful. +*/ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, std::initializer_list tp_names) { From b834767ba5bc179707c745cdf43270c612f08efe Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 15 Jul 2023 01:48:15 -0700 Subject: [PATCH 32/39] Minor function name change in test. --- tests/test_stl.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_stl.py b/tests/test_stl.py index baf841b855..43615843a4 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -404,10 +404,10 @@ def test_pass_std_vector_pair_int(): def test_list_caster_fully_consumes_generator_object(): - def gen_mix(): + def gen_invalid(): yield from [1, 2.0, 3] - gen_obj = gen_mix() + gen_obj = gen_invalid() with pytest.raises(TypeError): m.pass_std_vector_int(gen_obj) assert not tuple(gen_obj) @@ -483,10 +483,10 @@ def test_pass_std_map_int(): ], ) def test_map_caster_fully_consumes_generator_object(items, expected_exception): - def gen_mix(): + def gen_invalid(): yield from items - gen_obj = gen_mix() + gen_obj = gen_invalid() with pytest.raises(expected_exception): m.pass_std_map_int(FakePyMappingGenObj(gen_obj)) assert not tuple(gen_obj) From d19a0ff36bf3cd7c4c1229fb3011357c1463d859 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 00:27:42 -0700 Subject: [PATCH 33/39] strcmp -> std::strcmp (thanks @Skylion007 for catching this) --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index d8750a7a94..743d9f166d 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -75,7 +75,7 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, } const char *obj_tp_name = Py_TYPE(obj)->tp_name; for (const auto *tp_name : tp_names) { - if (strcmp(obj_tp_name, tp_name) == 0) { + if (std::strcmp(obj_tp_name, tp_name) == 0) { return true; } } From 69a0da89ba420904b8f56d38b22c08a32f574d4f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 01:04:20 -0700 Subject: [PATCH 34/39] Add `PyCallable_Check(items)` in `PyObjectTypeIsConvertibleToStdMap()` --- include/pybind11/stl.h | 16 ++++++++++++++-- tests/test_stl.py | 17 ++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 743d9f166d..09b9466ca1 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -96,8 +96,20 @@ inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { } inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { - return (PyDict_Check(obj) != 0) - || ((PyMapping_Check(obj) != 0) && (PyObject_HasAttrString(obj, "items") != 0)); + if (PyDict_Check(obj)) { + return true; + } + if (!PyMapping_Check(obj)) { + return false; + } + PyObject *items = PyObject_GetAttrString(obj, "items"); + if (items == nullptr) { + PyErr_Clear(); + return false; + } + bool is_convertible = (PyCallable_Check(items) != 0); + Py_DECREF(items); + return is_convertible; } // diff --git a/tests/test_stl.py b/tests/test_stl.py index 43615843a4..1c6f093567 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -454,6 +454,17 @@ def items(self): return ((1, 2), (3, "x")) +class FakePyMappingItemsNotCallable(FakePyMappingMissingItems): + @property + def items(self): + return ((1, 2), (3, 4)) + + +class FakePyMappingItemsWithArg(FakePyMappingMissingItems): + def items(self, _): + return ((1, 2), (3, 4)) + + class FakePyMappingGenObj(FakePyMappingMissingItems): def __init__(self, gen_obj): super().__init__() @@ -466,13 +477,17 @@ def items(self): def test_pass_std_map_int(): fn = m.pass_std_map_int assert fn({1: 2, 3: 4}) == 4506 + with pytest.raises(TypeError): + fn([]) assert fn(FakePyMappingWithItems()) == 3507 with pytest.raises(TypeError): fn(FakePyMappingMissingItems()) with pytest.raises(TypeError): fn(FakePyMappingBadItems()) with pytest.raises(TypeError): - fn([]) + fn(FakePyMappingItemsNotCallable()) + with pytest.raises(TypeError): + fn(FakePyMappingItemsWithArg()) @pytest.mark.parametrize( From c86fa9799330c08fc590eb29b101efdb6810f1eb Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 01:29:45 -0700 Subject: [PATCH 35/39] Resolve clang-tidy error --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 09b9466ca1..02e4b7d14b 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -99,7 +99,7 @@ inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { if (PyDict_Check(obj)) { return true; } - if (!PyMapping_Check(obj)) { + if (PyMapping_Check(obj) == 0) { return false; } PyObject *items = PyObject_GetAttrString(obj, "items"); From 940e1906495ed66329fd7e38370ab1067908ff8b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 12:45:39 -0700 Subject: [PATCH 36/39] Use `PyMapping_Items()` instead of `src.attr("items")()`, to be internally consistent with `PyMapping_Check()` --- include/pybind11/stl.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 02e4b7d14b..7129c79c3e 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -99,6 +99,9 @@ inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { if (PyDict_Check(obj)) { return true; } + // Implicit requirement in the conditions below: + // A type with `.__getitem__()` & `.items()` methods must implement these + // to be compatible with https://docs.python.org/3/c-api/mapping.html if (PyMapping_Check(obj) == 0) { return false; } @@ -237,11 +240,10 @@ struct map_caster { if (!convert) { return false; } - // Designed to be behavior-equivalent to passing dict(src.items()) from Python: - // The conversion to a dict will first exhaust the iterator object, to ensure that - // the iterator is not left in an unpredictable (to the caller) partially-consumed - // state. - auto items = src.attr("items")(); + auto items = reinterpret_steal(PyMapping_Items(src.ptr())); + if (!items) { + throw error_already_set(); + } assert(isinstance(items)); return convert_elements(dict(reinterpret_borrow(items)), convert); } From f7725e432cce25626d81861641c889d7e2f4c902 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 14:12:52 -0700 Subject: [PATCH 37/39] Update link to PyCLIF sources. --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 7129c79c3e..7805ad2910 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -38,7 +38,7 @@ PYBIND11_NAMESPACE_BEGIN(detail) // // Begin: Equivalent of -// https://github.com/google/clif/blob/337b2a106c0552ad85d40cfc3797465756840ea0/clif/python/runtime.cc#L388-L424 +// https://github.com/google/clif/blob/ae4eee1de07cdf115c0c9bf9fec9ff28efce6f6c/clif/python/runtime.cc#L388-L438 /* The three `PyObjectTypeIsConvertibleTo*()` functions below are the result of converging the behaviors of pybind11 and PyCLIF From fa729188129cd64e9549c26fd27295fa61bffcd1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 17 Jul 2023 15:02:04 -0700 Subject: [PATCH 38/39] Fix typo (thanks @wangxf123456 for catching this) --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 7805ad2910..ddfc78911d 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -65,7 +65,7 @@ to prevent accidents and improve readability: is that this conversion is not reducing. Implicit conversions of this kind are also fairly commonly used, therefore enforcing explicit conversions would have an unfavorable cost : benefit ratio; more sloppily speaking, - such an enforcement would be more annyoing than helpful. + such an enforcement would be more annoying than helpful. */ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, From 3ae34f3f1b8275efca621ead0bed2d93ebb9d3fb Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 16 Nov 2023 11:06:08 -0800 Subject: [PATCH 39/39] Fix typo discovered by new version of codespell. --- include/pybind11/stl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index df9ade8901..ad10fd4140 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -49,7 +49,7 @@ while pybind11 was very far on the strict side. Originally PyCLIF accepted any Python iterable as input for a C++ `vector`/`set`/`map` argument, as long as the elements were convertible. The obvious (in hindsight) problem was that any empty Python iterable could be passed to any of these C++ types, e.g. `{}` -was accpeted for C++ `vector`/`set` arguments, or `[]` for C++ `map` arguments. +was accepted for C++ `vector`/`set` arguments, or `[]` for C++ `map` arguments. The functions below strike a practical permissive-vs-strict compromise, informed by tens of thousands of use cases in the wild. A main objective is