Skip to content

boost::transform_iterator does not typedef iterator_category #88

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aeubanks opened this issue Oct 29, 2024 · 4 comments
Closed

boost::transform_iterator does not typedef iterator_category #88

aeubanks opened this issue Oct 29, 2024 · 4 comments

Comments

@aeubanks
Copy link

https://www.boost.org/doc/libs/1_85_0/libs/iterator/doc/transform_iterator.html says it typedefs iterator_category based on the underlying iterator. However, nothing in https://github.com/boostorg/iterator/blob/develop/include/boost/iterator/transform_iterator.hpp seems to indicate that it's doing so.

With llvm/llvm-project#112102 disallowing std::prev(non_cpp17_bidi_iterator), we have code that breaks due to transform_iterator not being bidi even though the underlying iterator is.

Example: https://godbolt.org/z/a6TzjGW91

#include <iostream>
#include <boost/iterator/transform_iterator.hpp>

int main() {
    auto d = [](int i) { return i * 2; };
    int a[] = {1, 2, 3};
    auto begin = boost::make_transform_iterator(std::begin(a), d);
    auto end = boost::make_transform_iterator(std::end(a), d);
    static_assert(std::bidirectional_iterator<decltype(std::begin(a))>);
    static_assert(std::bidirectional_iterator<decltype(begin)>); // this fails
    for (auto i = begin; i != end; ++i) {
        std::cout << *i << " ";
    }
}

adding something like using iterator_category = std::iterator_traits<Iterator>::iterator_category; around

seems to fix this for the case we ran into.

@mclow mclow transferred this issue from boostorg/boost Oct 30, 2024
@Lastique
Copy link
Member

Lastique commented Oct 30, 2024

https://www.boost.org/doc/libs/1_85_0/libs/iterator/doc/transform_iterator.html says it typedefs iterator_category based on the underlying iterator. However, nothing in https://github.com/boostorg/iterator/blob/develop/include/boost/iterator/transform_iterator.hpp seems to indicate that it's doing so.

The iterator_category type is provided by iterator_facade_base from which transform_iterator ultimately derives (through iterator_adaptor and iterator_facade).

The problem is that the std::bidirectional_iterator concept requires the iterator to dereference to an actual reference type and your transform_iterator dereferences to an rvalue. This is because the transform function d returns an rvalue. This is expected behavior. If you change d definition to e.g. auto d = [](int& i) -> int& { return i; }; then the code compiles.

@aeubanks
Copy link
Author

@Lastique ah I had misread the class hierarchy. However, someone had pointed out that https://en.cppreference.com/w/cpp/iterator/bidirectional_iterator#Notes says Unlike the LegacyBidirectionalIterator requirements, the bidirectional_iterator concept does not require dereference to return an lvalue.

@Lastique
Copy link
Member

Lastique commented Oct 30, 2024

Yes, I've been inaccurate, but the point still stands. In C++20, the std::bidirectional_iterator concept includes std::input_or_output_iterator requirements, and that concept requires the result of dereferencing the iterator to be bindable to an lvalue reference (the can-reference requirement). So, while it doesn't strictly require a reference type as a result of dereferencing the iterator, an int rvalue is still not allowed.

Though I must say my understanding of concepts is rather limited, so I may be misinterpreting something.

@Lastique
Copy link
Member

Ah, scratch my last reply. The issue is actually that the iterator_category type defined by the iterator is not one of the standard iterator category tags but a synthesized category iterator_category_with_traversal<std::input_iterator_tag, boost::iterators::random_access_traversal_tag> (i.e an input iterator that supports random access traversal). Again, this is because the reference type of the iterator is not an actual reference, which was the requirement for forward iterators prior to C++20.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants