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

libcpp deque has a random-access iterator. #1867

Closed
Alexhuszagh opened this Issue Sep 11, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@Alexhuszagh
Contributor

Alexhuszagh commented Sep 11, 2017

In libbcpp.vector, you define:

cdef extern from "<vector>" namespace "std" nogil:
    cdef cppclass vector[T,ALLOCATOR=*]:
        ctypedef T value_type
        ctypedef ALLOCATOR allocator_type

        # these should really be allocator_type.size_type and
        # allocator_type.difference_type to be true to the C++ definition
        # but cython doesn't support defered access on template arguments
        ctypedef size_t size_type
        ctypedef ptrdiff_t difference_type

        cppclass iterator:
            T& operator*()
            iterator operator++()
            iterator operator--()
            iterator operator+(size_type)
            iterator operator-(size_type)
            difference_type operator-(iterator)
            bint operator==(iterator)
            bint operator!=(iterator)
            bint operator<(iterator)
            bint operator>(iterator)
            bint operator<=(iterator)
            bint operator>=(iterator)
        cppclass reverse_iterator:
            T& operator*()
            iterator operator++()
            iterator operator--()
            iterator operator+(size_type)
            iterator operator-(size_type)
            bint operator==(reverse_iterator)
            bint operator!=(reverse_iterator)
            bint operator<(reverse_iterator)
            bint operator>(reverse_iterator)
            bint operator<=(reverse_iterator)
            bint operator>=(reverse_iterator)
        cppclass const_iterator(iterator):
            pass
        cppclass const_reverse_iterator(reverse_iterator):
            pass

This defines the most of the operations for a random-access iterator for a C++ iterator.

However, libcpp.deque, which also has a random-access iterator, does not have the same supported methods: in fact, it is treated like a bidirectional iterator.

cdef extern from "<deque>" namespace "std" nogil:
    cdef cppclass deque[T,ALLOCATOR=*]:
        cppclass iterator:
            T& operator*()
            iterator operator++()
            iterator operator--()
            bint operator==(iterator)
            bint operator!=(iterator)
        cppclass reverse_iterator:
            T& operator*()
            iterator operator++()
            iterator operator--()
            bint operator==(reverse_iterator)
            bint operator!=(reverse_iterator)
        cppclass const_iterator(iterator):
            pass

Simply, deque's iterator should support the same methods as vector's iterator does (and ideally, it should also have methods for size_type operator-(iterator). Any standard-compliant implementation of deque such as Clang's should confirm this.

@Alexhuszagh Alexhuszagh changed the title from Deque is a random-access iterator. to libcpp deque has a random-access iterator. Sep 11, 2017

@scoder

This comment has been minimized.

Show comment
Hide comment
@scoder

scoder Sep 11, 2017

Contributor
Contributor

scoder commented Sep 11, 2017

@Alexhuszagh

This comment has been minimized.

Show comment
Hide comment
@Alexhuszagh

Alexhuszagh Sep 11, 2017

Contributor

I'll be submitting a PR very soon then @scoder . Before I do, is there any reason that const_reverse_iterator is commented out for deque? The Github issue tracker does not highlight any bugs reference that issue, but I'd rather not possibly revert any fixes. Next, is there any particular reason why reverse_iterator has the method iterator operator++(), and not reverse_iterator operator++()? And finally, may I implement size_type operator-(iterator) for both deque and vector iterators?

Thank you.

Contributor

Alexhuszagh commented Sep 11, 2017

I'll be submitting a PR very soon then @scoder . Before I do, is there any reason that const_reverse_iterator is commented out for deque? The Github issue tracker does not highlight any bugs reference that issue, but I'd rather not possibly revert any fixes. Next, is there any particular reason why reverse_iterator has the method iterator operator++(), and not reverse_iterator operator++()? And finally, may I implement size_type operator-(iterator) for both deque and vector iterators?

Thank you.

@scoder

This comment has been minimized.

Show comment
Hide comment
@scoder

scoder Sep 11, 2017

Contributor

I'll happily leave these questions to @robertwb.

Contributor

scoder commented Sep 11, 2017

I'll happily leave these questions to @robertwb.

@robertwb

This comment has been minimized.

Show comment
Hide comment
@robertwb

robertwb Sep 12, 2017

Contributor
Contributor

robertwb commented Sep 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment