Skip to content
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

Improve C++11 containers #2207

Merged
merged 9 commits into from Aug 11, 2018
Merged

Conversation

vallsv
Copy link
Contributor

@vallsv vallsv commented Apr 20, 2018

Hi,

As promised, here is some improvement for the C++11 containers.

Unfortunately it is not an exhaustive work, as the changes are subtle and it's much more work to check everything. But it can be already very useful.

For insert with begin/end iterators, i use an iterator class instead of an InputIterator (as it is defined in C++). I guess it avoid a lot of things, but it's better than before cause it allow at least to insert elements from an equivalent container. Let me know if there is better solutions.

I did unittests as the project allow to update and use it very easily. I really appreciate it.

- bucket API
- load_factor API
- erase and insert with begin/end iterators
- bucket API
- load_factor API
- erase and insert with begin/end iterators
- The declaration have to be the reverse of C++
  as the iterator class matches both iterator and const_iterator classes
@vallsv
Copy link
Contributor Author

vallsv commented Apr 23, 2018

BTW, is iterators are real mapped object to C++ or just helper for cython?

For example, could we replace these without problem?

        cppclass iterator:
            pair[const T, U]& operator*()
            const_iterator operator++()
            const_iterator operator--()
            bint operator==(const_iterator)
            bint operator!=(const_iterator)
        cppclass const_iterator(iterator):
            pass

per

        cppclass const_iterator:
            pair[const T, U]& operator*()
            const_iterator operator++()
            const_iterator operator--()
            bint operator==(const_iterator)
            bint operator!=(const_iterator)
        cppclass iterator(const_iterator):
            pass

It looks to match better the sementic.

For example you could write:

cdef set[int].const_iterator a
cdef set[int].iterator b
a = b
#b = a # should be illegal

But i can be wrong.

@scoder
Copy link
Contributor

scoder commented May 1, 2018

The problem I see is this:

pair[const T, U]& operator*()

Here, T is const for the const_iterator, but it shouldn't be for the iterator. This would need to be taken care of in the inheritance case.

@Alexhuszagh
Copy link
Contributor

I like the idea of revamping all STL containers like this, since many functions take a const_iterator argument, but since Cython does not allow automatic conversion of an iterator to const_iterator yet, these functions signatures use iterator currently.

@vallsv
Copy link
Contributor Author

vallsv commented May 12, 2018

I like the idea of revamping all STL containers like this, since many functions take a const_iterator

Exactly. For the first version of this PR i just did copy-paste of the C++ signatures before understanding my mistake.

Anyway this PR do not change any iterators in this way.

@scoder scoder added this to the 0.29 milestone Jul 6, 2018
@scoder
Copy link
Contributor

scoder commented Aug 11, 2018

I think it's an improvement as it is. Thanks!

@scoder scoder merged commit bd0a2c6 into cython:master Aug 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants