Bruce Merry ee0c5ee405 Add make_value_iterator (#3271) ... * Add make_value_iterator This is the counterpart to make_key_iterator, and will allow implementing a `value` method in `bind_map` (although doing so is left for a subsequent PR). I made a few design changes to reduce copy-and-paste boilerplate. Previously detail::iterator_state had a boolean template parameter to indicate whether it was being used for make_iterator or make_key_iterator. I replaced the boolean with a class that determines how to dereference the iterator. This allows for a generic implementation of `__next__`. I also added the ValueType and Extra... parameters to the iterator_state template args, because I think it was a bug that they were missing: if make_iterator is called twice with different values of these, only the first set has effect (because the state class is only registered once). There is still a potential issue in that the *values* of the extra arguments are latched on the first call, but since most policies are empty classes this should be even less common. * Add some remove_cv_t to appease clang-tidy * Make iterator_access and friends take reference For some reason I'd accidentally made it take a const value, which caused some issues with third-party packages. * Another attempt to remove remove_cv_t from iterators Some of the return types were const (non-reference) types because of the pecularities of decltype: `decltype((*it).first)` is the *declared* type of the member of the pair, rather than the type of the expression. So if the reference type of the iterator is `pair<const int, int> &`, then the decltype is `const int`. Wrapping an extra set of parentheses to form `decltype(((*it).first))` would instead give `const int &`. This means that the existing make_key_iterator actually returns by value from `__next__`, rather than by reference. Since for mapping types, keys are always const, this probably hasn't been noticed, but it will affect make_value_iterator if the Python code tries to mutate the returned objects. I've changed things to use double parentheses so that make_iterator, make_key_iterator and make_value_iterator should now all return the reference type of the iterator. I'll still need to add a test for that; for now I'm just checking whether I can keep Clang-Tidy happy. * Add back some NOLINTNEXTLINE to appease Clang-Tidy This is favoured over using remove_cv_t because in some cases a const value return type is deliberate (particularly for Eigen). * Add a unit test for iterator referencing Ensure that make_iterator, make_key_iterator and make_value_iterator return references to the container elements, rather than copies. The test for make_key_iterator fails to compile on master, which gives me confidence that this branch has fixed it. * Make the iterator_access etc operator() const I'm actually a little surprised it compiled at all given that the operator() is called on a temporary, but I don't claim to fully understand all the different value types in C++11. * Attempt to work around compiler bugs https://godbolt.org/ shows an example where ICC gets the wrong result for a decltype used as the default for a template argument, and CI also showed problems with PGI. This is a shot in the dark to see if it fixes things. * Make a test constructor explicit (Clang-Tidy) * Fix unit test on GCC 4.8.5 It seems to require the arguments to the std::pair constructor to be implicitly convertible to the types in the pair, rather than just requiring is_constructible. * Remove DOXYGEN_SHOULD_SKIP_THIS guards Now that a complex decltype expression has been replaced by a simpler nested type, I'm hoping Doxygen will be able to build it without issues. * Add comment to explain iterator_state template params		2021-09-21 13:37:19 -04:00
..
pybind11	Add make_value_iterator (#3271)	2021-09-21 13:37:19 -04:00