Revert "Add make_value_iterator (#3271)"

This reverts commit ee0c5ee405.
This commit is contained in:
Henry Schreiner 2021-09-22 22:50:29 -04:00
parent 1dc9a23cae
commit 2fa3fcfda5
4 changed files with 32 additions and 176 deletions

View File

@ -63,9 +63,6 @@ Convenience functions converting to Python types
.. doxygenfunction:: make_key_iterator(Iterator, Sentinel, Extra &&...)
.. doxygenfunction:: make_key_iterator(Type &, Extra&&...)
.. doxygenfunction:: make_value_iterator(Iterator, Sentinel, Extra &&...)
.. doxygenfunction:: make_value_iterator(Type &, Extra&&...)
.. _extras:
Passing extra arguments to ``def`` or ``class_``

View File

@ -1955,52 +1955,25 @@ inline std::pair<decltype(internals::registered_types_py)::iterator, bool> all_t
return res;
}
/* There are a large number of apparently unused template arguments because
* each combination requires a separate py::class_ registration.
*/
template <typename Access, return_value_policy Policy, typename Iterator, typename Sentinel, typename ValueType, typename... Extra>
template <typename Iterator, typename Sentinel, bool KeyIterator, return_value_policy Policy>
struct iterator_state {
Iterator it;
Sentinel end;
bool first_or_done;
};
// Note: these helpers take the iterator by non-const reference because some
// iterators in the wild can't be dereferenced when const.
template <typename Iterator>
struct iterator_access {
using result_type = decltype((*std::declval<Iterator>()));
// NOLINTNEXTLINE(readability-const-return-type) // PR #3263
result_type operator()(Iterator &it) const {
return *it;
}
};
PYBIND11_NAMESPACE_END(detail)
template <typename Iterator>
struct iterator_key_access {
using result_type = decltype(((*std::declval<Iterator>()).first));
result_type operator()(Iterator &it) const {
return (*it).first;
}
};
template <typename Iterator>
struct iterator_value_access {
using result_type = decltype(((*std::declval<Iterator>()).second));
result_type operator()(Iterator &it) const {
return (*it).second;
}
};
template <typename Access,
return_value_policy Policy,
/// Makes a python iterator from a first and past-the-end C++ InputIterator.
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Iterator,
typename Sentinel,
typename ValueType,
#ifndef DOXYGEN_SHOULD_SKIP_THIS // Issue in breathe 4.26.1
typename ValueType = decltype(*std::declval<Iterator>()),
#endif
typename... Extra>
iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&... extra) {
using state = detail::iterator_state<Access, Policy, Iterator, Sentinel, ValueType, Extra...>;
// TODO: state captures only the types of Extra, not the values
iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
using state = detail::iterator_state<Iterator, Sentinel, false, Policy>;
if (!detail::get_type_info(typeid(state), false)) {
class_<state>(handle(), "iterator", pybind11::module_local())
@ -2014,7 +1987,7 @@ iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&... extra) {
s.first_or_done = true;
throw stop_iteration();
}
return Access()(s.it);
return *s.it;
// NOLINTNEXTLINE(readability-const-return-type) // PR #3263
}, std::forward<Extra>(extra)..., Policy);
}
@ -2022,55 +1995,35 @@ iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&... extra) {
return cast(state{first, last, true});
}
PYBIND11_NAMESPACE_END(detail)
/// Makes a python iterator from a first and past-the-end C++ InputIterator.
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Iterator,
typename Sentinel,
typename ValueType = typename detail::iterator_access<Iterator>::result_type,
typename... Extra>
iterator make_iterator(Iterator first, Sentinel last, Extra &&... extra) {
return detail::make_iterator_impl<
detail::iterator_access<Iterator>,
Policy,
Iterator,
Sentinel,
ValueType,
Extra...>(first, last, std::forward<Extra>(extra)...);
}
/// Makes a python iterator over the keys (`.first`) of a iterator over pairs from a
/// Makes an python iterator over the keys (`.first`) of a iterator over pairs from a
/// first and past-the-end InputIterator.
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Iterator,
typename Sentinel,
typename KeyType = typename detail::iterator_key_access<Iterator>::result_type,
#ifndef DOXYGEN_SHOULD_SKIP_THIS // Issue in breathe 4.26.1
typename KeyType = decltype((*std::declval<Iterator>()).first),
#endif
typename... Extra>
iterator make_key_iterator(Iterator first, Sentinel last, Extra &&...extra) {
return detail::make_iterator_impl<
detail::iterator_key_access<Iterator>,
Policy,
Iterator,
Sentinel,
KeyType,
Extra...>(first, last, std::forward<Extra>(extra)...);
}
using state = detail::iterator_state<Iterator, Sentinel, true, Policy>;
/// Makes a python iterator over the values (`.second`) of a iterator over pairs from a
/// first and past-the-end InputIterator.
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Iterator,
typename Sentinel,
typename ValueType = typename detail::iterator_value_access<Iterator>::result_type,
typename... Extra>
iterator make_value_iterator(Iterator first, Sentinel last, Extra &&...extra) {
return detail::make_iterator_impl<
detail::iterator_value_access<Iterator>,
Policy, Iterator,
Sentinel,
ValueType,
Extra...>(first, last, std::forward<Extra>(extra)...);
if (!detail::get_type_info(typeid(state), false)) {
class_<state>(handle(), "iterator", pybind11::module_local())
.def("__iter__", [](state &s) -> state& { return s; })
.def("__next__", [](state &s) -> detail::remove_cv_t<KeyType> {
if (!s.first_or_done)
++s.it;
else
s.first_or_done = false;
if (s.it == s.end) {
s.first_or_done = true;
throw stop_iteration();
}
return (*s.it).first;
}, std::forward<Extra>(extra)..., Policy);
}
return cast(state{first, last, true});
}
/// Makes an iterator over values of an stl container or other container supporting
@ -2087,13 +2040,6 @@ template <return_value_policy Policy = return_value_policy::reference_internal,
return make_key_iterator<Policy>(std::begin(value), std::end(value), extra...);
}
/// Makes an iterator over the values (`.second`) of a stl map-like container supporting
/// `std::begin()`/`std::end()`
template <return_value_policy Policy = return_value_policy::reference_internal,
typename Type, typename... Extra> iterator make_value_iterator(Type &value, Extra&&... extra) {
return make_value_iterator<Policy>(std::begin(value), std::end(value), extra...);
}
template <typename InputType, typename OutputType> void implicitly_convertible() {
struct set_flag {
bool &flag;

View File

@ -15,7 +15,6 @@
#include <algorithm>
#include <utility>
#include <vector>
#ifdef PYBIND11_HAS_OPTIONAL
#include <optional>
@ -38,29 +37,6 @@ bool operator==(const NonZeroIterator<std::pair<A, B>>& it, const NonZeroSentine
return !(*it).first || !(*it).second;
}
class NonCopyableInt {
public:
explicit NonCopyableInt(int value) : value_(value) {}
NonCopyableInt(const NonCopyableInt &) = delete;
NonCopyableInt(NonCopyableInt &&other) noexcept : value_(other.value_) {
other.value_ = -1; // detect when an unwanted move occurs
}
NonCopyableInt &operator=(const NonCopyableInt &) = delete;
NonCopyableInt &operator=(NonCopyableInt &&other) noexcept {
value_ = other.value_;
other.value_ = -1; // detect when an unwanted move occurs
return *this;
}
int get() const { return value_; }
void set(int value) { value_ = value; }
~NonCopyableInt() = default;
private:
int value_;
};
using NonCopyableIntPair = std::pair<NonCopyableInt, NonCopyableInt>;
PYBIND11_MAKE_OPAQUE(std::vector<NonCopyableInt>);
PYBIND11_MAKE_OPAQUE(std::vector<NonCopyableIntPair>);
template <typename PythonType>
py::list test_random_access_iterator(PythonType x) {
if (x.size() < 5)
@ -312,10 +288,6 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
.def(
"items",
[](const StringMap &map) { return py::make_iterator(map.begin(), map.end()); },
py::keep_alive<0, 1>())
.def(
"values",
[](const StringMap &map) { return py::make_value_iterator(map.begin(), map.end()); },
py::keep_alive<0, 1>());
// test_generalized_iterators
@ -334,38 +306,8 @@ TEST_SUBMODULE(sequences_and_iterators, m) {
.def("nonzero_keys", [](const IntPairs& s) {
return py::make_key_iterator(NonZeroIterator<std::pair<int, int>>(s.begin()), NonZeroSentinel());
}, py::keep_alive<0, 1>())
.def("nonzero_values", [](const IntPairs& s) {
return py::make_value_iterator(NonZeroIterator<std::pair<int, int>>(s.begin()), NonZeroSentinel());
}, py::keep_alive<0, 1>())
;
// test_iterater_referencing
py::class_<NonCopyableInt>(m, "NonCopyableInt")
.def(py::init<int>())
.def("set", &NonCopyableInt::set)
.def("__int__", &NonCopyableInt::get)
;
py::class_<std::vector<NonCopyableInt>>(m, "VectorNonCopyableInt")
.def(py::init<>())
.def("append", [](std::vector<NonCopyableInt> &vec, int value) {
vec.emplace_back(value);
})
.def("__iter__", [](std::vector<NonCopyableInt> &vec) {
return py::make_iterator(vec.begin(), vec.end());
})
;
py::class_<std::vector<NonCopyableIntPair>>(m, "VectorNonCopyableIntPair")
.def(py::init<>())
.def("append", [](std::vector<NonCopyableIntPair> &vec, const std::pair<int, int> &value) {
vec.emplace_back(NonCopyableInt(value.first), NonCopyableInt(value.second));
})
.def("keys", [](std::vector<NonCopyableIntPair> &vec) {
return py::make_key_iterator(vec.begin(), vec.end());
})
.def("values", [](std::vector<NonCopyableIntPair> &vec) {
return py::make_value_iterator(vec.begin(), vec.end());
})
;
#if 0
// Obsolete: special data structure for exposing custom iterator types to python

View File

@ -36,10 +36,6 @@ def test_generalized_iterators():
assert list(m.IntPairs([(1, 2), (2, 0), (0, 3), (4, 5)]).nonzero_keys()) == [1]
assert list(m.IntPairs([(0, 3), (1, 2), (3, 4)]).nonzero_keys()) == []
assert list(m.IntPairs([(1, 2), (3, 4), (0, 5)]).nonzero_values()) == [2, 4]
assert list(m.IntPairs([(1, 2), (2, 0), (0, 3), (4, 5)]).nonzero_values()) == [2]
assert list(m.IntPairs([(0, 3), (1, 2), (3, 4)]).nonzero_values()) == []
# __next__ must continue to raise StopIteration
it = m.IntPairs([(0, 0)]).nonzero()
for _ in range(3):
@ -52,30 +48,6 @@ def test_generalized_iterators():
next(it)
def test_iterator_referencing():
"""Test that iterators reference rather than copy their referents."""
vec = m.VectorNonCopyableInt()
vec.append(3)
vec.append(5)
assert [int(x) for x in vec] == [3, 5]
# Increment everything to make sure the referents can be mutated
for x in vec:
x.set(int(x) + 1)
assert [int(x) for x in vec] == [4, 6]
vec = m.VectorNonCopyableIntPair()
vec.append([3, 4])
vec.append([5, 7])
assert [int(x) for x in vec.keys()] == [3, 5]
assert [int(x) for x in vec.values()] == [4, 7]
for x in vec.keys():
x.set(int(x) + 1)
for x in vec.values():
x.set(int(x) + 10)
assert [int(x) for x in vec.keys()] == [4, 6]
assert [int(x) for x in vec.values()] == [14, 17]
def test_sliceable():
sliceable = m.Sliceable(100)
assert sliceable[::] == (0, 100, 1)
@ -179,7 +151,6 @@ def test_map_iterator():
assert sm[k] == expected[k]
for k, v in sm.items():
assert v == expected[k]
assert list(sm.values()) == [expected[k] for k in sm]
it = iter(m.StringMap({}))
for _ in range(3): # __next__ must continue to raise StopIteration